1918139 - Turn on --enable-perf by default for nightly builds

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P4)

Description

[Denis Palmeiro [:denispal]]

Turning this feature on by default will be a big help when diagnosing problems in CI and Nightly. Currently we have to either push a separate build with --enable-perf so we get PGO training, or re-build this locally which may not always match the performance of a proper build.

There is some perf assessment that needs to be done to make sure enabling this doesn't regress anything.

Comment 1

[Nicolas B. Pierron [:nbp]]

Is there any privacy concerns that should be addressed with a data-review?

Comment 2

[Denis Palmeiro [:denispal]]

(In reply to Nicolas B. Pierron [:nbp] from comment #1)

Is there any privacy concerns that should be addressed with a data-review?

Hmm, were you thinking of anything specific? I don't think there should be any privacy concerns. The intention is to build with --enable-perf by default on nightly so we can toggle jitdump output through an environment variable.

Comment 3

[Nicolas B. Pierron [:nbp]]

(In reply to Denis Palmeiro [:denispal] from comment #2)

(In reply to Nicolas B. Pierron [:nbp] from comment #1)

Is there any privacy concerns that should be addressed with a data-review?

Hmm, were you thinking of anything specific? I don't think there should be any privacy concerns. The intention is to build with --enable-perf by default on nightly so we can toggle jitdump output through an environment variable.

I do not have anything specific in mind, just asking in case there is any probe which does any reporting without user interaction, such as adding information to the crash-dumps or telemetry.

Comment 4

[Denis Palmeiro [:denispal]]

Ah, this is purely for local profiling so no client information will be collected by telemetry or crash dumps.

Comment 5

[Denis Palmeiro [:denispal]]

Attachment: Bug 1918139: Use SprintfBuf instead of snprintf to fix build errors. r=iain!

Comment 6

[Denis Palmeiro [:denispal]]

Attachment: Bug 1918139: Fix plain build errors due to undeclared identifier JS_smprintf. r=iain!

Depends on D226641

Comment 7

[Denis Palmeiro [:denispal]]

Attachment: Bug 1918139: Do not leak ExecutablePools when perfspewer is active. r=iain!

We currently leak ExecutablePools if the perf spewer is enabled since perf maps cannot properly understand if an address region was reused.
Using jitdump, we now have load records so this should no longer be an issue.

Depends on D226642

Comment 8

[Denis Palmeiro [:denispal]]

Attachment: Bug 1918139: Enable perf jitdump integration by default for nightly. r=iain!

Depends on D226643

Comment 9

[Denis Palmeiro [:denispal]]

Attachment: Bug 1918139: Remove JitCodeAPI since it's unused. r=iain!

The JitCodeAPI was initially planned to surface some PerfSpewer information to the Gecko Profiler, but it's currently unused and likely unneeded.

Depends on D226644

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/994185187bb0
Use SprintfBuf instead of snprintf to fix build errors. r=iain
https://hg.mozilla.org/integration/autoland/rev/6ac63b456ee4
Fix plain build errors due to undeclared identifier JS_smprintf.  r=iain
https://hg.mozilla.org/integration/autoland/rev/41cb4171d24a
Do not leak ExecutablePools when perfspewer is active. r=iain
https://hg.mozilla.org/integration/autoland/rev/eb055c6db954
Enable perf jitdump integration by default for nightly.  r=iain
https://hg.mozilla.org/integration/autoland/rev/91dc4a926783
Remove JitCodeAPI since it's unused.  r=iain

Comment 11

[agoloman]

Backed out for causing build bustages @bug1875795.js.

• Backout link
• Push with failures
• Failure Log

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/014306ce6604
Use SprintfBuf instead of snprintf to fix build errors. r=iain
https://hg.mozilla.org/integration/autoland/rev/5e5d4534f1e0
Fix plain build errors due to undeclared identifier JS_smprintf.  r=iain
https://hg.mozilla.org/integration/autoland/rev/82e8a7d7fce6
Do not leak ExecutablePools when perfspewer is active. r=iain
https://hg.mozilla.org/integration/autoland/rev/ff8540bd68df
Enable perf jitdump integration by default for nightly.  r=iain
https://hg.mozilla.org/integration/autoland/rev/e81aa9a01c77
Remove JitCodeAPI since it's unused.  r=iain

Comment 13

[amarc]

https://hg.mozilla.org/mozilla-central/rev/014306ce6604
https://hg.mozilla.org/mozilla-central/rev/5e5d4534f1e0
https://hg.mozilla.org/mozilla-central/rev/82e8a7d7fce6
https://hg.mozilla.org/mozilla-central/rev/ff8540bd68df
https://hg.mozilla.org/mozilla-central/rev/e81aa9a01c77