Switch to crossbeam-queue for events #1091
Conversation
| @@ -91,7 +103,7 @@ impl Backend { | |||
| self.flush(); | |||
| } | |||
|
|
|||
| pub fn run(&mut self, events: impl IntoIterator<Item = Event>) { | |||
| pub fn run(&mut self, events: Arc<ArrayQueue<Event>>) { | |||
| let (lock, cvar) = &*FINISHED; | |||
| let mut finished = lock.lock().unwrap(); | |||
There was a problem hiding this comment.
There's also MainThreadRuntime::send_event, which calls Mutex::lock, too, on a Mutex<Backend>. Also, are we sure allocations can't happen in any of these calls?
There was a problem hiding this comment.
are we sure allocations can't happen in any of these calls?
ArrayQueue docs say:
This queue allocates a fixed-capacity buffer on construction, which is used to store pushed elements.... Having a buffer allocated upfront makes this queue a bit faster than SegQueue.
Which sounds like it doesn't allocate on push.
There's also MainThreadRuntime::send_event, which calls Mutex::lock
@ahomescu, can you clarify specifically which functions should support being called from a signal handler context?
There was a problem hiding this comment.
Which sounds like it doesn't allocate on push.
There's a bunch of other functions here that look like they could potentially called inside a signal handler. I understand this one is specifically designed not to allocate, but I'm not sure about the others. Like bincode serialization or formatting output, etc. They might not, but we need to check, right?
There was a problem hiding this comment.
My vague understanding of the design here is that the instrumentation calls we insert push events onto a queue, and a background thread processes those events. Since we potentially could be instrumenting signal handlers, the instrumentation/push code needs to be lock-free and allocation-free. But the background thread's processing (including bincode/serialization stuff) doesn't run in a signal handler and so isn't subject to those constraints.
There was a problem hiding this comment.
That makes sense. So then only ExistingRuntime::send_event needs to be async-signal-safe, meaning fn run and fn write_all here don't need to be, right?
There was a problem hiding this comment.
Right, it's exactly what @spernsteiner said: the push code needs to be signal-safe, but the receiver doesn't because it runs in its own thread.
Edit: Should I go audit all the functions in the runtime and check exactly which ones need to be signal-safe? Shouldn't be too hard.
| @@ -91,7 +103,7 @@ impl Backend { | |||
| self.flush(); | |||
| } | |||
|
|
|||
| pub fn run(&mut self, events: impl IntoIterator<Item = Event>) { | |||
| pub fn run(&mut self, events: Arc<ArrayQueue<Event>>) { | |||
| let (lock, cvar) = &*FINISHED; | |||
| let mut finished = lock.lock().unwrap(); | |||
There was a problem hiding this comment.
That makes sense. So then only ExistingRuntime::send_event needs to be async-signal-safe, meaning fn run and fn write_all here don't need to be, right?
Use a lock-free queue to store the event list because the event callbacks might be called from inside a signal handler. Most system calls are not safe in that context, so we just spin if the queue is full.
ahomescu
force-pushed
the
ahomescu/lock_free_event_queue
branch
from
64faf2f
to
d83f983
Compare
Use a lock-free queue to store the event list because the event callbacks might be called from inside a signal handler. Most system calls are not safe in that context, so we just spin if the queue is full.