Closed Bug 1598992 Opened 2 years ago Closed 2 years ago

On Linux the starting Gecko Profiler tries to handle Base Profiler signals too early

Categories

(Core :: Gecko Profiler, defect, P1)

69 Branch
Unspecified
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr68 --- unaffected
firefox71 --- wontfix
firefox72 --- fixed

People

(Reporter: gerald, Assigned: gerald)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files)

When the Gecko Profiler (GP) starts, GP sets up its own signal handler but it's not ready yet to handle signals until the GP sampler starts.
If the Base Profiler (BP) is running, it may send a signal to stop threads to be profiled, which the GP handler may intercept before it is ready.

So the BP (and its sampler) needs to be stopped before the GP can be started.

This ensures that no more Base Profiler (BP) activity can happen when Gecko
Profiler (GP) starts.
In particular on Linux this is needed because the BP sampler sends signals to
stop threads, and the just-starting GP could receive this signal before it is
fully ready to handle it with its own sampler.

Depends on D54442

The Base Profiler originally named the main thread "Main Thread", which is
friendlier than "GeckoMain". However this makes it more difficult to combine the
controls for both profilers if they use different names.

So now both profilers use "GeckoMain", so filters can be exactly the same.
Base Profiler adds "(pre-xul)" to the name to distinguish tracks in the
frontend -- This distinction is actually necessary so the frontend doesn't get
confused by threads with the exact same name, but eventually tracks will get
combined in the frontend as well.

Depends on D54443

In practice the previous test that was deleting Base Profiler data when the
index became greater that 1 was correct for Firefox, because the Base Profiler
always starts before the very first Gecko Profiler active instance.

However in tests (like the one in the following patch) this may not be true,
because each test may start and stop the profiler, and the recent storage update
means that the index doesn't go back to 1. So when a test (apart from the first
test to use the profiler) attemps to use the Base Profiler, that Base Profiler
data will be immediately discarded because the index is already greater than 1
(from previous tests).

This change is more future-proof as well, in case we later want to use the Base
Profiler more than once in a Firefox instance.

Depends on D54444

Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b3880cc8dc23
Stop Base Profiler before starting Gecko Profiler - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/a7931318ce6c
Use same thread names in Base Profiler filter as in Gecko, and suffix with "(pre-xul)" in JSON - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/53bc356053b0
More precise way to decide when to delete expired Base Profiler data - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/40903d7428fb
Add test for profile hand-off from Base to Gecko Profiler - r=gregtatum
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/909fd0369609
Stop Base Profiler before starting Gecko Profiler - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/bf4148a5bc47
Use same thread names in Base Profiler filter as in Gecko, and suffix with "(pre-xul)" in JSON - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/98015236fc64
More precise way to decide when to delete expired Base Profiler data - r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/0e6c7dbb2abb
Add test for profile hand-off from Base to Gecko Profiler - r=gregtatum
Flags: needinfo?(gsquelart)
You need to log in before you can comment on or make changes to this bug.