1301167 - Profiler needs a Wake API

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[(No longer employed by Mozilla) Aaron Klotz]

Bug 1273635 makes it possible for the Windows wait functions to be interrupted by Asynchronous Procedure Calls (APCs). These essentially work like signals.

This impacts the profiler because there is a GeckoProfilerSleepRAII on the stack, thus the profiler will ignore anything that executes in these APCs.

I'd like to add a GeckoProfilerWakeRAII that can be instantiated in the APC so that the profiler is woken up as necessary.

Note that this bug is important in order to be able to profile a11y code under Windows e10s.

Comment 1

[Benoit Girard (:BenWa)]

This seems reasonable. Thanks Aaron.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Add GeckoProfilerWakeRAII

Comment 3

[Benoit Girard (:BenWa)]

Comment on attachment 8789019 [details] [diff] [review]
Add GeckoProfilerWakeRAII

Review of attachment 8789019 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with a few small suggestions to improve the patch.

::: tools/profiler/public/GeckoProfiler.h
@@ +271,5 @@
>      profiler_sleep_end();
>    }
>  };
>  
> +class GeckoProfilerWakeRAII {

Let's add a block comment here. At first glance it seems like this RAII should be used when the profiler is awake in the global state but it's only meant for a special temporary wake ups. Perhaps something like:

/**
 * Temporarily wake up the profiler while servicing events such as
 * Asynchronous Procedure Calls (APCs).
 */

Feel free to adjust.

@@ +272,5 @@
>    }
>  };
>  
> +class GeckoProfilerWakeRAII {
> +public:

Also this class should be MOZ_RAII. Which the other classes don't do properly either. IIRC MOZ_RAII will prevent you accidentally making your RAII a temporary and therefore useless.

@@ +282,5 @@
> +    }
> +  }
> +  ~GeckoProfilerWakeRAII() {
> +    if (mIssuedWake) {
> +      profiler_sleep_start();

MOZ_ASSERT(!profiler_is_sleeping()) before line 286. Let's just make sure we're not in an unexpected state.

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Add GeckoProfilerWakeRAII (r2)

Addressed comments. Carrying forward r+.

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/218a0249ee4fb13af3b620ec83fcf04e02bdf14c
Bug 1301167: Add a GeckoProfilerWakeRAII interface to the profiler; r=BenWa

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/218a0249ee4f
Add a GeckoProfilerWakeRAII interface to the profiler; r=BenWa

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/218a0249ee4f