I'm impressed by the improvement. I didn't really expect any.
On Tue, Jul 13, 2021 at 09:19:34AM +0100, Anton Ivanov wrote: > I ran the revised patch series (v2) which addresses your comments on the > ovn-heater benchmark. > > With 9 fake nodes, 50 fake pods per node I get ~ 2% reproducible improvement. > IMHO it should be even more noticeable at high scale. > > Best regards, > > A > > On 09/07/2021 19:45, Ben Pfaff wrote: > > On Fri, Jul 09, 2021 at 06:19:06PM +0100, anton.iva...@cambridgegreys.com > > wrote: > > > From: Anton Ivanov <anton.iva...@cambridgegreys.com> > > > > > > If we are not obtaining any useful information out of the poll(), > > > such as is a fd busy or not, we do not need to do a poll() if > > > an immediate_wake() has been requested. > > > > > > This cuts out all the pollfd hash additions, forming the poll > > > arguments and the actual poll() after a call to > > > poll_immediate_wake() > > > > > > Signed-off-by: Anton Ivanov <anton.iva...@cambridgegreys.com> > > Thanks for the patch. > > > > I think that this will have some undesirable side effects because it > > avoids calling time_poll() if the wakeup should happen immediately, and > > time_poll() does some thing that we always want to happen between one > > main loop and another. In particular the following calls from > > time_poll() are important: > > > > coverage_clear(); > > coverage_run(); > > if (*last_wakeup && !thread_is_pmd()) { > > log_poll_interval(*last_wakeup); > > } > > > > ... > > > > if (!time_left) { > > ovsrcu_quiesce(); > > } else { > > ovsrcu_quiesce_start(); > > } > > > > ... > > > > if (deadline <= time_msec()) { > > #ifndef _WIN32 > > fatal_signal_handler(SIGALRM); > > #else > > VLOG_ERR("wake up from WaitForMultipleObjects after deadline"); > > fatal_signal_handler(SIGTERM); > > #endif > > if (retval < 0) { > > retval = 0; > > } > > break; > > } > > > > ... > > > > *last_wakeup = time_msec(); > > refresh_rusage(); > > > > Instead of this change, I'd suggest something more like the following, > > along with the changes you made to suppress the file descriptors if the > > timeout is already zero: > > > > diff --git a/lib/timeval.c b/lib/timeval.c > > index 193c7bab1781..f080a9999742 100644 > > --- a/lib/timeval.c > > +++ b/lib/timeval.c > > @@ -323,7 +323,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE > > *handles OVS_UNUSED, > > } > > #ifndef _WIN32 > > - retval = poll(pollfds, n_pollfds, time_left); > > + retval = time_left ? poll(pollfds, n_pollfds, time_left) : 0; > > if (retval < 0) { > > retval = -errno; > > } > > > > > -- > 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