Closed Bug 1580091 Opened 11 months ago Closed 11 months ago

BlocksRingBuffer's mutex should be optional

Categories

(Core :: Gecko Profiler, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: gerald, Assigned: gerald)

References

Details

Attachments

(2 files)

Inside the critical section of the sampler, we need to store stack data in BlocksRingBuffer. But no mutex can be used inside the critical section, not even one that is solely used here!

So we a BlocksRingBuffer that doesn't use its mutex.
This task will wrap the existing mutex in a Maybe, and whether the mutex is created and used will be chosen at construction time.

(Ideally, we should have a separate type for buffers with and without mutex, but that would be too-big a change right now; it could be done in a later bug.)

BaseProfilerMaybeMutex wraps a BaseProfilerMutex inside a Maybe.
The decision to use a mutex or not is set at construction time.
If there is no mutex, all operations do nothing (at the small cost of checking
if the mutex is present.)

BaseProfilerMaybeAutoLock is the recommented RAII object to lock and
automatically unlock a BaseProfilerMaybeMutex until the end of a block scope.

In some situations we will need to use a BlocksRingBuffer that absolutely
does not use a mutex -- In particular in the critical section of
SamplerThread::Run(), when a thread is suspended, because any action on any
mutex (even a private one that no-one else interacts with) can result in a hang.

As a bonus, BlocksRingBuffers that are known not to be used in multi-thread
situations (e.g., backtraces, extracted stacks for duplication, etc.) will waste
less resources trying to lock/unlock their mutex.

Depends on D45304

(Ideally, we should have a separate type for buffers with and without mutex, but that would be too-big a change right now; it could be done in a later bug.)

Sounds like a good decision to me.

Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6f34c2e57ccf
BaseProfilerMaybeMutex and BaseProfilerMaybeAutoLock - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/79b29286f906
Use BaseProfilerMaybeMutex in BlocksRingBuffer - r=gregtatum
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3ba4d78b186a
BaseProfilerMaybeMutex and BaseProfilerMaybeAutoLock - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/5f0d86d44a8a
Use BaseProfilerMaybeMutex in BlocksRingBuffer - r=gregtatum
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.