Re: [Iperf-users] [Patch 1/1]: (a) timer utilities, (b) use nanosleep(2) for delay_loop()

[Jon D. <jd...@x1...>]

Gerrit,

Excellent!

My preference for handling patches would be to submit them to the Patch 
tracker on Sourceforge:

https://sourceforge.net/tracker/?group_id=128336&atid=711373

And then send a note to the list mentioning that the patch has been uploaded 
and what it does.  I'll get an email when the patches are submitted, but 
others on the list may be interested.

I don't have time to look at this patch in depth right now, but I will get to 
it early next week.  This along with several other patches should hit trunk 
next week.

Cheers,

Jon

Gerrit Renker wrote:
> Dear Jon,
> 
> further to earlier email discussion, I am now in the process
> of updating the DCCP patches from
>  
>   http://www.erg.abdn.ac.uk/users/gerrit/dccp/apps/#iperf
> 
> to match the current SVN revision (#37). I have copied this
> to the iperf-users list -- can you please advise re submission
> format and to whom to copy?
> 
> I will be sending the other patches shortly. The first patch
> in the series has been omitted (replacing sched_yield() with
> condition variables), since per announcement this is in the
> works anyway.
> 
> Gerrit
> 
> The attached patch: 
> ------------------
>  * replaces delay_loop() with nanosleep
>    - this has advantages over the gettimeofday() spin-loop
>      since among other things it blocks signals and was found
>      to be more accurate over the long term than the current
>      implementation
>    - if people feel they would like a different delay_loop,
>      the old implementation has been kept via #ifdefs
> 
>  * implements several timer utilities
>    - a "timer-stop" delta_usec() time measurement method
>    - an overloaded form of "before()"
>    - a corresponding "after()"
> 
> 
> The University of Aberdeen is a charity registered in Scotland, No SC013683.
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Timer Utilities
> ===============
> 
> Apart from extending the Timestamp class (support for measuring elapsed
> time and an `after' test), the main point of this patch is to update
> the delay_loop() function. 
> 
> The present implementation used a tight loop of continually calling
> gettimeofday(2) until the specified amount of time has passed. This 
> is resource-intensive. In addition, while the loop worked reasonably
> in many cases, it sometimes caused unwanted delays of 50 milliseconds
> or more (process preempted?).
> 
> This has been replaced with nanosleep(2) as default now. The advantage
> of nanosleep over the old solution is that signals are blocked and it
> seems a much more robust solution.
> 
> Also, nanosleep uses the hrtimers interface internally 
> (/usr/src/davem-2.6/Documentation/hrtimers/hrtimers.txt).
> 
> For people who would like to keep the old behaviour, #ifdefs with
> alternatives have been added.
> 
> Signed-off-by: Gerrit Renker <ge...@er...>
> ---
>  compat/delay.cpp      |   38 ++++++++++++++++++++++++++++++--------
>  include/Timestamp.hpp |   29 ++++++++++++++++-------------
>  2 files changed, 46 insertions(+), 21 deletions(-)
> 
> --- a/compat/delay.cpp
> +++ b/compat/delay.cpp
> @@ -51,19 +51,31 @@
>   * ------------------------------------------------------------------- */
>  
>  #include "Timestamp.hpp"
> -
> +#include "util.h"
>  #include "delay.hpp"
>  
>  /* -------------------------------------------------------------------
> - * A micro-second delay function. This uses gettimeofday (underneith
> - * the Timestamp) which has a resolution of upto microseconds. I've
> - * found it's good to within about 10 usecs.
> - * I used to do calibration, but iperf automatically adjusts itself
> - * so that isn't necesary, and it causes some problems if the
> - * calibration adjustment is larger than your sleep time.
> + * A micro-second delay function.
>   * ------------------------------------------------------------------- */
> +void delay_loop(unsigned long usec)
> +{
> +#ifdef SLEEP_VIA_SELECT
> +     // This is a hack but works apparently quite well
> +     // I found that it sometimes under-estimates the timeout
> +     struct timeval tv;
> +
> +     tv.tv_sec = 0;
> +     tv.tv_usec = usec;
>  
> -void delay_loop( unsigned long usec ) {
> +     select(1, (fd_set *) 0, (fd_set *) 0, (fd_set *) 0, &tv);
> +#elif defined(SLEEP_OLD_METHOD)
> +    /* Tis uses gettimeofday (underneath the Timestamp), which has a
> +     * resolution of upto microseconds. I've found it's good to within
> +     * about 10 usecs. I used to do calibration, but iperf automatically
> +     * adjusts itself so that isn't necesary, and it causes some problems
> +     * if the calibration adjustment is larger than your sleep time.
> +     * Note: I found this has problems - if the loop gets interrupted,
> +     * long sleeps can result. I found up to 50..60msec. -- Gerrit */
>      Timestamp end;
>      end.add( usec * 1e-6 );
>  
> @@ -71,4 +83,14 @@ void delay_loop( unsigned long usec ) {
>      while ( now.before( end ) ) {
>          now.setnow();
>      }
> +#else
> +    // This is preferred; it is even better than usleep (see manpage).
> +    struct timespec ts;
> +
> +    ts.tv_sec = 0;
> +    ts.tv_nsec = usec * 1000L;
> +
> +    if (nanosleep(&ts, NULL) < 0)
> +        WARN_errno(1, "nanosleep");
> +#endif
>  }
> --- a/include/Timestamp.hpp
> +++ b/include/Timestamp.hpp
> @@ -152,6 +152,16 @@ public:
>      }
>  
>      /* -------------------------------------------------------------------
> +     * Return the number of microseconds from now to last time of setting.
> +     * ------------------------------------------------------------------- */
> +    long delta_usec(void) {
> +        struct timeval previous = mTime;
> +
> +        setnow();
> +        return subUsec(previous);
> +    }
> +
> +    /* -------------------------------------------------------------------
>       * subtract the right timestamp from my timestamp.
>       * return the difference in seconds as a floating point.
>       * ------------------------------------------------------------------- */
> @@ -202,29 +212,22 @@ public:
>      /* -------------------------------------------------------------------
>       * return true if my timestamp is before the right timestamp.
>       * ------------------------------------------------------------------- */
> -    bool before( Timestamp right ) {
> -        return mTime.tv_sec < right.mTime.tv_sec  ||
> -        (mTime.tv_sec == right.mTime.tv_sec &&
> -         mTime.tv_usec < right.mTime.tv_usec);
> -    }
> -    
> -    /* -------------------------------------------------------------------
> -     * return true if my timestamp is before the right timestamp.
> -     * ------------------------------------------------------------------- */
>      bool before( timeval right ) {
>          return mTime.tv_sec < right.tv_sec  ||
>          (mTime.tv_sec == right.tv_sec &&
>           mTime.tv_usec < right.tv_usec);
>      }
> +    bool before( Timestamp right ) { return before(right.mTime); }
>  
>      /* -------------------------------------------------------------------
>       * return true if my timestamp is after the right timestamp.
>       * ------------------------------------------------------------------- */
> -    bool after( Timestamp right ) {
> -        return mTime.tv_sec > right.mTime.tv_sec  ||
> -        (mTime.tv_sec == right.mTime.tv_sec &&
> -         mTime.tv_usec > right.mTime.tv_usec);
> +    bool after( timeval right ) {
> +        return mTime.tv_sec > right.tv_sec  ||
> +        (mTime.tv_sec == right.tv_sec &&
> +         mTime.tv_usec > right.tv_usec);
>      }
> +    bool after( Timestamp right ) { return after(right.mTime); }
>  
>      /**
>       * This function returns the fraction of time elapsed after the beginning