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

2021-07-15 Thread Ben Pfaff 
On Mon, Jul 12, 2021 at 06:15:29PM +0100, anton.iva...@cambridgegreys.com wrote:
> From: Anton Ivanov 
> 
> 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 

I'd like another look at that after patch 1 is revised.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

2021-07-15 Thread Anton Ivanov 

On 15/07/2021 20:49, Mark Michelson wrote:

Hi Anton,

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


I can't pick it up on OVN heater.

That is not unexpected. It is "noise" compared to the compute time and 
the number of syscalls used in one iteration in northd or ovsdb.


I suspect it will be more useful in places like the vswitch itself where 
the loops are tighter, processing per loop is less and lower latency is 
the key. There, one syscall less per iteration may show up as a 
noticeable difference.


I do not have a benchmark set up for those.

A.


On 7/12/21 1:15 PM, anton.iva...@cambridgegreys.com wrote:

From: Anton Ivanov 

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 
---
  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;






--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

2021-07-15 Thread Mark Michelson 

Hi Anton,

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

On 7/12/21 1:15 PM, anton.iva...@cambridgegreys.com wrote:

From: Anton Ivanov 

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 
---
  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;



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

2021-07-12 Thread anton . ivanov 
From: Anton Ivanov 

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 
---
 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;
-- 
2.20.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev