envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.85k stars 4.77k forks source link

Possible optimization opportunity: set EVENT_BASE_FLAG_NOLOCK #5703

Open oschaaf opened 5 years ago

oschaaf commented 5 years ago

Possible optimization opportunity: set EVENT_BASE_FLAG_NOLOCK

Description: While looking for potential accuracy and throughput improvements for Nighthawk [1] (a load generator/benchmarking tool based on Envoy's libraries), observational data suggested that configuring libevent with EVENT_BASE_FLAG_NOLOCK would yield around 5% added troughput and about 1us lower reported latencies in the benchmarking client in max rps test runs.

Subsequently maxing out rps on an Envoy instance with the same patch applied [2] (which probably breaks a few things in its current form!) yields another improvement compared to a non-patched Envoy, which while not as pronounced as it was in Nighthawk, may still be worth exploring further, hence me filing this issue. If an audit of event usage would show this is low hanging fruit, incorporating a proper version of this patch may be little work and yield a quick win.

Links:

mattklein123 commented 5 years ago

FWIW, I tried to do this a while ago (1-1.5 years), but it's not easy unless I missed something back then. Basically, the only case in which we require the lock is when we post events between event loops. I tried to get rid of the lock and provide my own locking for posting, but unfortunately there was no way to get libevent to safely wake up the event loop during a post. So then there would have to be some code like, "every 10ms wake up no matter what and check the post queue" but there is no hook in libevent to do this. cc @htuch since he has been looking at similar stuff when considering other event loops and I discussed this case with him briefly.

oschaaf commented 5 years ago

I'm curious, would listening on a socket and periodically writing a byte to that be a viable way to wake up an event loop with locking disabled in a safe way?

dnoe commented 5 years ago

In a past project we used a pipe for this. Libevent has it in the watched set, and the poster writes something to the pipe to trigger a wake up.

On Thu, Jan 24, 2019 at 12:17 PM Otto van der Schaaf < notifications@github.com> wrote:

I'm curious, would listening on a socket and periodically writing a byte to that be a viable way to wake up an event loop with locking disabled in a safe way?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/envoyproxy/envoy/issues/5703#issuecomment-457280230, or mute the thread https://github.com/notifications/unsubscribe-auth/ABIPmb8i2EaZ30kL9e5MwAWKjGDxDO3Nks5vGeqjgaJpZM4aQVpO .

mattklein123 commented 5 years ago

Yeah a pipe/UDS would definitely work.

htuch commented 5 years ago

libev also has support for doing this directly; https://metacpan.org/pod/distribution/EV/libev/ev.pod#ev_async-how-to-wake-up-an-event-loop.

I think it would be interesting to consider how libuv/libev would perform as well as functional distinctions in https://github.com/envoyproxy/envoy/issues/4952.

dnoe commented 5 years ago

Check out "eventfd" for a specialized abstraction layer on top of the underlying pipelike mechanism that can do this. But there might be portability concerns, I've only used it on Linux.

On Thu, Jan 24, 2019 at 12:50 PM Matt Klein notifications@github.com wrote:

Yeah a pipe/UDS would definitely work.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/envoyproxy/envoy/issues/5703#issuecomment-457291721, or mute the thread https://github.com/notifications/unsubscribe-auth/ABIPmVY7VGSr4koAvHPZ_vc6lJc0nHkyks5vGfJVgaJpZM4aQVpO .

htuch commented 5 years ago

Also see http://libev.schmorp.de/bench.html for performance comparisons of libevent vs. libev, you need a lot of FDs to make things noticeable, but I wonder how the event loops differ on other dimensions, e.g. contention or raw speed of various common operations we make use of in Envoy.

keesspoelstra commented 5 years ago

Is this mostly applicable to cross thread calls to the Dispatcher.post?

I've been looking at the libevent implementation of event_activate (enableTimer(0) use case). A pipe or eventfd is already used by libevent to signal another thread when activating an event. We would have no extra performance hit of extra kernel calls if we used a pipe/eventfd, because that is already happening under the hood.

repokitteh-read-only[bot] commented 5 years ago

:scream_cat: Error while processing event:

evaluation error
error: load("github.com/repokitteh/modules/review.star") error: module load error
:cat: Caused by: a https://github.com/envoyproxy/envoy/issues/5703#issuecomment-458136745 was created by @keesspoelstra. see: [more](https://github.com/envoyproxy/envoy/issues/5703#issuecomment-458136745), [trace](https://prod.repokitteh.app/traces/ui/envoyproxy/envoy/a25cd390-2302-11e9-8924-fb19f9898b27).