Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Await for the gecko profiler start instead of adding a synthetic delay #1857

Open
canova opened this issue Oct 17, 2022 · 2 comments
Open

Await for the gecko profiler start instead of adding a synthetic delay #1857

canova opened this issue Oct 17, 2022 · 2 comments

Comments

@canova
Copy link
Contributor

canova commented Oct 17, 2022

Feature/improvement

Currently browsertime adds a delay after the gecko profiler start command:

await runner.runPrivilegedScript(script, 'Start GeckoProfiler');
return delay(firefoxConfig.geckoProfilerSettleTime || 3000);

This was because previously we didn't have a way to properly await for the profiler start since all the processes start the profiler individually. But this year, with the Bug 1668867, we've started to return a Promise instead from the Services.profiler.StartProfiler API and it resolves when all the processes start profiling. This way we can start the profiler and await until it's ready without needing an additional settle delay.

Here's the profiler start command in browsertime:

script = `Services.profiler.StartProfiler(${bufferSize},${interval},${featureString},
${chosenFeatures.length},${threadString},${chosenThreads.length});`;
} else if (parameterLength === 5 || parameterLength === 6) {
script = `Services.profiler.StartProfiler(${bufferSize},${interval},${featureString},${threadString});`;

Here's an example in our tests: https://searchfox.org/mozilla-central/rev/0a2eba79c24300ce0539f91c1bebac2e75264e58/tools/profiler/tests/xpcshell/test_active_configuration.js#24-30

@soulgalore
Copy link
Member

Hi @canova ah cool, so then I can just remove the extra wait and it will work as it should right?

@canova
Copy link
Contributor Author

canova commented Oct 18, 2022

Yes, it should work as expected without the extra wait if we properly await the Services.profiler.StartProfiler.
Not sure if we are doing it already? I see an await in front of the runner.runPrivilegedScript call but there is no await inside the script that's passed to runPrivilegedScript as an argument. Probably it's not awaiting for that one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants