Open Bug 1697953 Opened 3 years ago Updated 8 months ago

Consider switching profiler buffer locking to atomics

Categories

(Core :: Gecko Profiler, enhancement, P2)

enhancement

Tracking

()

People

(Reporter: mozbugz, Unassigned)

References

Details

(Whiteboard: [fxp])

The current profiler buffer (ProfileChunkedBuffer) uses a mutex when writing each entry. Writing should be quick, but this can still cause contentions, e.g., there are a few markers blocking on it in this profile: https://share.firefox.dev/3bzXvk2 (Thanks :jesup)
Also, mutexes are definitely a problem when encountered in real-time operations, like audio callback tracing (which currently uses its own buffering with off-thread processing to generate markers.)

I believe it should be possible to use atomics in the buffer in most situations.
The APIs were designed to allow this, and I wrote an early prototype. (My old MultiWriterQueue is somewhat similar.)
But its average throughput looked the same as the mutexed buffer, so I went with that much-simpler solution to start with.

Even with atomics there would still be some potentially-blocking operations, e.g., when a new chunk needs to be allocated right now, but in practice this should always happen in advance in the sampler thread, so most users should never experience blocking there.

Special care will be needed when a chunk gets filled, we'll need to ensure that everybody is done writing, before releasing it.
And we'd need to correctly handle back-readers when destroying/recycling chunks. E.g., to duplicate stacks, but bug 1633572 may remove this usage; I'm not sure if there are any other back-readers?

You need to log in before you can comment on or make changes to this bug.