[ovs-dev] [PATCH v2 2/2] Minimize the number of time calls in time_poll()

[Mark Michelson]

Hi Anton,

Out of curiosity, has this change made a noticeable impact on performance?

On 7/12/21 1:15 PM, anton.ivanov at cambridgegreys.com wrote:
> From: Anton Ivanov <anton.ivanov at cambridgegreys.com>
> 
> time_poll() makes an excessive number of time_msec() calls
> which incur a performance penalty.
> 
> 1. Avoid time_msec() call for timeout calculation when time_poll()
> is asked to skip poll()
> 
> 2. Reuse the time_msec() result from deadline calculation for
> last_wakeup and timeout calculation.
> 
> Signed-off-by: Anton Ivanov <anton.ivanov at cambridgegreys.com>
> ---
>   lib/timeval.c | 36 +++++++++++++++++++++---------------
>   1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/timeval.c b/lib/timeval.c
> index c6ac87376..64ab22e05 100644
> --- a/lib/timeval.c
> +++ b/lib/timeval.c
> @@ -287,7 +287,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE *handles OVS_UNUSED,
>             long long int timeout_when, int *elapsed)
>   {
>       long long int *last_wakeup = last_wakeup_get();
> -    long long int start;
> +    long long int start, now;
>       bool quiescent;
>       int retval = 0;
>   
> @@ -297,28 +297,31 @@ time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE *handles OVS_UNUSED,
>       if (*last_wakeup && !thread_is_pmd()) {
>           log_poll_interval(*last_wakeup);
>       }
> -    start = time_msec();
> +    now = start = time_msec();
>   
>       timeout_when = MIN(timeout_when, deadline);
>       quiescent = ovsrcu_is_quiescent();
>   
>       for (;;) {
> -        long long int now = time_msec();
>           int time_left;
>   
> -        if (now >= timeout_when) {
> +        if (n_pollfds == 0) {
>               time_left = 0;
> -        } else if ((unsigned long long int) timeout_when - now > INT_MAX) {
> -            time_left = INT_MAX;
>           } else {
> -            time_left = timeout_when - now;
> -        }
> -
> -        if (!quiescent) {
> -            if (!time_left) {
> -                ovsrcu_quiesce();
> +            if (now >= timeout_when) {
> +                time_left = 0;
> +            } else if ((unsigned long long int) timeout_when - now > INT_MAX) {
> +                time_left = INT_MAX;
>               } else {
> -                ovsrcu_quiesce_start();
> +                time_left = timeout_when - now;
> +            }
> +
> +            if (!quiescent) {
> +                if (!time_left) {
> +                    ovsrcu_quiesce();
> +                } else {
> +                    ovsrcu_quiesce_start();
> +                }
>               }
>           }
>   
> @@ -329,6 +332,8 @@ time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE *handles OVS_UNUSED,
>            */
>           if (n_pollfds != 0) {
>               retval = poll(pollfds, n_pollfds, time_left);
> +        } else {
> +            retval = 0;
>           }
>           if (retval < 0) {
>               retval = -errno;
> @@ -355,7 +360,8 @@ time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE *handles OVS_UNUSED,
>               ovsrcu_quiesce_end();
>           }
>   
> -        if (deadline <= time_msec()) {
> +        now = time_msec();
> +        if (deadline <= now) {
>   #ifndef _WIN32
>               fatal_signal_handler(SIGALRM);
>   #else
> @@ -372,7 +378,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE *handles OVS_UNUSED,
>               break;
>           }
>       }
> -    *last_wakeup = time_msec();
> +    *last_wakeup = now;
>       refresh_rusage();
>       *elapsed = *last_wakeup - start;
>       return retval;
>