Spawned from bug 1651102 comment 3:

Do we really need to lock the profiler mutex from profiler_get_backtrace?
Naively, I would hope that it's not needed when walking the stack from the stack-owning thread.
We already have other stack walkers that don't use the profiler lock.
And the periodic sampler should still be able to do its work because it's suspending the target thread and just reading from its stack.

I asked Markus his throughts, if there was any strong reason for this lock to exist?

I see quite a few uses of the LockRef inside profiler_get_backtrace. For example, DoSharedSample checks ActivePS::FeatureStackWalk, which could be mutated on the profiler controller thread.

This sounds like some technical reasons why it may be difficult... So I'm filing this bug to explore this further!
(In the case of ActivePS::FeatureStackWalk, maybe we could use the atomic RacyFeatures instead, like we do for markers; and/or pass features as arguments into sub-functions? It doesn't seem like a hard blocker to me; TBC...)

This profile (thanks :jesup) shows a few markers caught by the sampler: https://share.firefox.dev/3bzXvk2
It's quite a busy time, there are hundreds of markers during this 26ms window!
Hopefully this bug should alleviate some of the overhead due to markers using the main profiler mutex.

Tech detail for this bug: One thing the mutex is used for, is accessing a shared pre-allocated buffer to store JIT frames, because it could cause stack overflow if stored on the stack.

Some possible solutions:

• At least have a separate buffer for markers only, so it wouldn't clash with the sampler.
• Avoid locking the mutex when we don't actually have JIT frames. (Only possible when sampling the current thread. Otherwise we've suspended another thread, and it's not possible to touch mutexes during that time.)
• One buffer per thread.
• Where platforms permit, gauge the amount of stack space left, and if possible use a stack-based lockless buffer.

This has been fixed and we don't have a mutex anymore inside profiler_get_backtrace anymore since Bug 1716959. Closing.