Closed Bug 960718 Opened 10 years ago Closed 6 years ago

A thread that shuts down while it's being profiled shouldn't discard its profiling data

Categories

(Core :: Gecko Profiler, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: mstange, Unassigned)

References

(Blocks 1 open bug)

Details

Currently, when a thread shuts down, we unregister it from the profiler and throw away its thread profile. That's bad; for example when you get a profile during browser shutdown with the MOZ_SHUTDOWN_PROFILE env variable, it will only contain the main thread profile because all the other threads have already shut down. (I've run into this because I want to profile the Compositor thread on Talos.)

Instead, when a thread unregisters itself while it is being profiled, we should move its ThreadInfo and PseudoStack to a temporary location where they can survive. When the profiler is stopped, we can delete them. We also need to make sure that we don't attempt to get any more samples from the unregistered threads.
We should bound how long they survive to prevent workers for starting/stopping many threads and accumulating thread buffers. We could use the generation number of the main thread to expire them. Once the main thread has fully wrapped over then we can discard them.
Blocks: 1337558
This has changed a lot since I filed this bug.

When a thread gets unregistered, we move its ThreadInfo from CorePS::mLiveThreads to mDeadThreads. Its data is in the buffer that's shared between all threads, and that data doesn't get discarded. It's only evicted from the buffer at the regular rate, as the buffer is overwritten.

So this means that we hold on to the data of dead threads for just as long as any other thread's data. The only exception to this is the data that we store in mSavedStreamedSamples and mSavedStreamedMarkers. We should probably discard those once the time range they cover no longer overlaps the buffer contents.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
(In reply to Markus Stange [:mstange] from comment #2)
> We should probably
> discard those once the time range they cover no longer overlaps the buffer
> contents.

I've filed bug 1433583 about this.
You need to log in before you can comment on or make changes to this bug.