> On Jul 26, 2024, at 3:27 AM, Bertrand Drouvot <bertranddrouvot...@gmail.com> 
> wrote:
> 
> Hi,
> 
> On Thu, Jul 25, 2024 at 05:27:15PM -0500, Sami Imseih wrote:
>> I am attaching v3 of the patch which addresses the comments made
>> earlier by Bertrand about the comment in the patch [1].
> 
> Thanks!
> 
> Looking at it:
> 
> 1 ===
> 
> +       struct instr_time start_time;
> 
> I think we can get rid of the "struct" keyword here.
> 
> 2 ===
> 
> +           struct instr_time current_time;
> +           struct instr_time elapsed_time;
> 
> Same as above.

Will fix those 2.

> 
> 3 ===
> 
> I gave more thoughts and I think it can be simplified a bit to reduce the
> number of operations in the while loop.
> 
> What about relying on a "absolute" time that way:
> 
>       instr_time absolute;
>    absolute.ticks = start_time.ticks + msec * 1000000;
> 
> and then in the while loop:
> 
>    while (nanosleep(&delay, &remain) == -1 && errno == EINTR)
>    {
>        instr_time current_time;
>        INSTR_TIME_SET_CURRENT(current_time);
> 
>        if (current_time.ticks > absolute.ticks)
>        {
>            break;

While I agree this code is cleaner, myy hesitation there is we don’t 
have any other place in which we access .ticks directly and the 
common practice is to use the intsr_time.h APIs.


What do you think?


Regards,

Sami 




Reply via email to