maxikrie commented on code in PR #3929:
URL: https://github.com/apache/nuttx-apps/pull/3929#discussion_r4383776339


##########
testing/sched/timerjitter/timerjitter.c:
##########
@@ -192,8 +194,8 @@ static FAR void *timerjitter(FAR void *arg)
     }
 
   param->avg = 0;
-  param->max = 0;
-  param->min = (unsigned long)-1;
+  param->max = INT64_MIN;

Review Comment:
   Thanks for the review!
   
   max = INT64_MIN, min = INT64_MAX: these are start values that every sample 
beats. The first sample sets both min and max, whatever its sign. The old start 
values were max = 0 and min = (unsigned long)-1, which only work while 
latencies are never negative. With a signed latency, max = 0 would be wrong 
whenever every sample is early: if all wakeups come 50 us early, max would 
print 0, a value that was never measured. INT64_MIN/INT64_MAX are the signed 
versions of "lowest/highest possible", so the first diff always replaces them.
   
   The start values are never printed. The loop sets min and max on the first 
sample, and when there are no samples (cur_cnt == 0) the output is "no samples" 
instead.
   
   avg = 0 as a double: during the loop avg is a running sum, so it starts at 
0. After the loop it is divided by cur_cnt, so it becomes the mean. It is a 
double so the mean can have a fractional part, and it was a double before this 
PR too, so I left it alone. The value is still printed rounded (%.0lf).
   
   Negative average: a double is signed, so nothing special happens. avg += 
diff adds the signed int64_t value, and the result prints as a negative number. 
For example, samples -100 and 10 give avg -45 and print min: -100, avg: -45, 
max 10. The average was already correct for negative latencies before this PR, 
because diff was always int64_t. Only min and max were broken: they were stored 
as unsigned long, so -851 showed up as 4294966445.
   
   Looking at this I also found that cur_cnt was incremented before sigwait(), 
so a failed first wait would have printed the start values as min/max. Fixed in 
the updated commit: an iteration is counted only after sigwait() returns.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to