Revisit doAtLeastOnePeriodicSample
Categories
(Core :: Gecko Profiler, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox72 | --- | fixed |
People
(Reporter: mozbugz, Assigned: mozbugz)
References
Details
Attachments
(3 files)
doAtLeastOnePeriodicSample() is used in some tests to wait until at least one more full "periodic sample" is done, by capturing the whole profile then capturing more until 'profile.threads[0].samples.data' changes.
This seems a bit fragile: It relies on the profiler actually collecting samples.
And after bug 1576555, markers won't be stored during a sampling run, which some tests used to rely upon (and maybe there are more I haven't noticed yet).
So I think we should review how doAtLeastOnePeriodicSample() works (maybe there could be a cheaper way to be notified directly by the SamplerThread::Run() loop), and maybe even if it's the right thing to do in all tests that use it.
|
Assignee | |
Comment 1•5 years ago
|
||
Register a callback to be invoked the next time the Sampler thread ends its
periodic loop.
Registration may fail (e.g., if the profiler is not active), in which case the
callback is never called.
After successful registration the callback is guaranteed to be invoked after the
sampler runs, or stops running.
|
|
Assignee | |
Comment 2•5 years ago
|
||
nsIProfiler::WaitOnePeriodicSampling() returns a promise that gets resolved
after the next full periodic sampling. The promise is rejected if the sampler is
not running.
Implementation detail: Promise uses single-threaded ref-counting, so special
care is needed to AddRef() before storing the raw reference to be kept by the
sampler thread, and then dispatching back to the main thread before using and
Release()'ing the Promise there.
Depends on D50781
|
|
Assignee | |
Comment 3•5 years ago
|
||
Instead of requesting profiles until it "seems" to have collected something,
use nsIProfiler::WaitOnePeriodicSampling() to really wait for a sample to be
taken -- which is what the function name doAtLeastOnePeriodicSample() implies.
Note that this means some test could theoretically fail if they were in fact
waiting for stack samples to appear in the first registered thread. If that
happens, these tests should be udpated to do that extra wait-for-data.
Depends on D50782
|
|
Assignee | |
Updated•5 years ago
|
|
||
Updated•5 years ago
|
|
|
||
Updated•5 years ago
|
Pushed by gsquelart@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4cf580c4f693 profiler_callback_after_sampling() - r=gregtatum https://hg.mozilla.org/integration/autoland/rev/89d901dbd481 nsIProfiler.waitOnePeriodicSampling() - r=gregtatum,froydnj https://hg.mozilla.org/integration/autoland/rev/ae8cfefad5db Replace doAtLeastOnePeriodicSample() with nsIProfiler.waitOnePeriodicSampling() - r=gregtatum
|
||
Comment 5•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/4cf580c4f693
https://hg.mozilla.org/mozilla-central/rev/89d901dbd481
https://hg.mozilla.org/mozilla-central/rev/ae8cfefad5db
|
|
||
Comment 6•5 years ago
|
||
| bugherder landing | ||
Description
•