Patina PR #410
Patina PR #410
Conversation
Signed-off-by: msdx321 <[email protected]>
Signed-off-by: msdx321 <[email protected]>
Signed-off-by: msdx321 <[email protected]>
Signed-off-by: msdx321 <[email protected]>
Signed-off-by: msdx321 <[email protected]>
Signed-off-by: msdx321 <[email protected]>
Signed-off-by: msdx321 <[email protected]>
Signed-off-by: msdx321 <[email protected]>
Signed-off-by: msdx321 <[email protected]>
Signed-off-by: msdx321 <[email protected]>
Signed-off-by: msdx321 <[email protected]>
Signed-off-by: msdx321 <[email protected]>
Signed-off-by: msdx321 <[email protected]>
There was a problem hiding this comment.
This is really great, and very close to being done, as I read it.
The biggest change I'd like to see is 1. remove the rampant debug printouts from the previous PR (sorry to ask this of you!), and 2. make sure that the printouts well-explain what is being tested. When the tests are done, they should print out that they are done in a consistent manner. Imagine a python script parsing their output to make sure that they still spit out the correct output.
| while (1) { | ||
| cycles_t end; | ||
|
|
||
| while (count < ITERATION) { | ||
| debug("h1,"); |
There was a problem hiding this comment.
I know you didn't add this code, but there is no reason to keep meaningless printouts in the code. Feel free to remove them.
| @@ -0,0 +1,9 @@ | |||
| ## tests | |||
|
|
|||
| This is the skeleton interface used by the `mkcomponent.sh` script to aid in the creation of a new component. | |||
There was a problem hiding this comment.
Just a brief note here about what it actually does, or remove this file. The file has no value as it stands. (I know it was automatically created).
| } | ||
|
|
||
| /*** | ||
| * The two threads reciprocally sends and receives. |
There was a problem hiding this comment.
"send" and "receive" should be singular.
| { | ||
| /* Never stops running; writer controls how many iters to run. */ | ||
| while (1) { | ||
| debug("r1,"); |
There was a problem hiding this comment.
Even worse, the debug statements make this hard to read! I know you didn't write them originally.
| { | ||
| int i; | ||
|
|
||
| for (int i = 0; i < ITERATION + COLD_OFFSET; i++) { |
There was a problem hiding this comment.
double definition of i. Remove the int on this line.
|
|
||
| assert(!crt_sem_init(s, (unsigned long)init_value)) | ||
|
|
||
| ss_sem_activate(s); |
There was a problem hiding this comment.
seems like the spacing is uniformly off for this style of function.
| assert(a & b); | ||
| assert(!(a->sec + b->sec < a->sec || a->usec + b->usec < a->usec)) | ||
|
|
||
| a->sec = a->sec + b->sec; |
| patina_time_add(struct time *a, struct time *b) | ||
| { | ||
| assert(a & b); | ||
| assert(!(a->sec + b->sec < a->sec || a->usec + b->usec < a->usec)) |
There was a problem hiding this comment.
Great job checking for the overflow.
| assert(a & b); | ||
| assert(!(a->sec - b->sec > a->sec || a->usec - b->usec > a->usec)) | ||
|
|
||
| a->sec = a->sec - b->sec; |
| @@ -229,7 +229,7 @@ perfdata_99ptile(struct perfdata *pd) | |||
| static void | |||
| perfdata_print(struct perfdata *pd) | |||
| { | |||
| printc("PD:%s -sz:%d,SD:%llu,Mean:%llu,99%%:%llu, Max: %llu\n", | |||
| printc("#PD:%s -sz:%d,SD:%llu,Mean:%llu,99%%:%llu, Max: %llu\n", | |||
Summary of this Pull Request (PR)
Implement Patina library and benchkit
Intent for your PR
Choose one (Mandatory):
Reviewers (Mandatory):
(Specify @<github.com username(s)> of the reviewers. Ex: @user1, @user2)
Code Quality
As part of this pull request, I've considered the following:
Style:
Code Craftsmanship:
Testing
I've tested the code using the following test programs (provide list here):