Open Bug 1773286 Opened 2 years ago Updated 2 years ago

Add an API to unregister a thread from another thread, by its thread id

Categories

(Core :: Gecko Profiler, enhancement, P2)

enhancement

Tracking

()

People

(Reporter: padenot, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

This is necessary to properly unregister threads that are not stopped by Firefox, from themselves (e.g. OS callbacks).

This sounds like a good idea. The current implementation assumes unregistration happens on the thread, so this won't be trivial (but simpler than registration, I think).

For my curiosity: How will you know when to unregister? Will it be when you stop your work and don't expect more callbacks? Or your callback runs on a different thread id and therefore assumes the previous one is obsolete?
Does it really give useful information when you sample those OS threads, apart from the times your callbacks run?

And another possible solution, would be to automatically unregister threads when the sampler can't find them anymore. (At the risk of sampling a new thread with the same recycled thread id).

Severity: -- → N/A
Priority: -- → P2

(In reply to Gerald Squelart [:gerald] (he/him) from comment #1)

For my curiosity: How will you know when to unregister? Will it be when you stop your work and don't expect more callbacks? Or your callback runs on a different thread id and therefore assumes the previous one is obsolete?

Speaking for media only (this is where I need this, and I know how it works):

  • Real-time Audio callback threads aren't created explicitely by Gecko, they are created by the OS and calling into Gecko. As such we can only register them (because we find a new thread id that we don't know), but for unregistering, there are two ways:
    • either the end of the stream is caused because we've run out of data (end of the media file). This we can detect, and unregister the thread already
    • or we destroy the system-level audio stream (via cubeb_stream_destroy), because e.g. we mute the audio, we navigate the page, etc.. We have the guarantee that when the function has returned, the thread won't be called again, and for all intents and purposes we can unregister it (if not in use by other audio streams, this ref counting is implemented already). However at this point we can only do so from another thread.
    • finally, it can also be that the thread id changes mi-stream (one common occurence is to switch the audio output device you're listening on mid-mistream). This we can detect, but at this point, we're on the new thread, so we can't run the unregistering function on the old one (that has probably exited).
  • For media decoders, it's more or less the same thing, except a bit less important: when destroying the decoder and no other decoders run, we can unregister them. Lifecycle is extremely clear as well.
  • For various listeners the OS calls, it's also reasonnably clear. We don't care too much about CPU there, but could use marker. There is the problem where threads must be registered to insert markers, last I checked. We should think about relaxing this requirement, but this is a separate concern.

Does it really give useful information when you sample those OS threads, apart from the times your callbacks run?

Yes, all the DSP code runs on those threads (for latency reasons), they are the thread that are important to profile to optimize audio processing.

And another possible solution, would be to automatically unregister threads when the sampler can't find them anymore. (At the risk of sampling a new thread with the same recycled thread id).

Because we know precisely when to register/unregister, because the API guarantees a certain behaviour, this is less appealing, if only because doing this explicitely is so easy.

In bug 1369222, Aaron wrote a prototype, highlighting some changes that may be needed in the sampling code: https://hg.mozilla.org/try/rev/210c788f4f38980de18c3a38d16ffc89e37ead36

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

Attachment

General

Created:
Updated:
Size: