Enabling branch pruning will prevent relazification.

RESOLVED FIXED in Firefox 49

Status

()

Core
JavaScript Engine
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: nbp, Unassigned)

Tracking

Trunk
mozilla49
Points:
---

Firefox Tracking Flags

(firefox48 affected, firefox49 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
While investigating LazyScriptCache potential out-come, I noticed that the way we use to check isRelazifiable() [1] implies that we would prevent relazification when branch pruning is enabled.

We should not disable relazification when branch pruning is enabled, but we should ensure that we disable it when we use either the Debugger, or the environment variable to spew the Code Coverage information.

[1] https://dxr.mozilla.org/mozilla-central/source/js/src/jsscript.h#1626
(Reporter)

Updated

2 years ago
Flags: needinfo?(nicolas.b.pierron)
(Reporter)

Comment 1

a year ago
Created attachment 8752927 [details] [diff] [review]
Distinguish code coverage usages, and disable it by default in the interpreter.

This patch change the code coverage function from the JSCompartment to
gather the result of 2 different reasons for enable code coverage.

The first reason is the Debugger, profiling option ( -D ), and the LCov
output enabled via an environment variable.  In such case we want the code
coverage result to be monotonic, and to not loose any of the profiled
information, thus disabling relazification.

The second reason is the Branch Pruning optimization, which uses code
coverage counters as a hint for the number of executions that we might
expect.  In such case, we want to limit the memory impact and only monitor
functions which would later be optimized with the branch pruning
optimization (or any others based on approximating code coverage)

This patch adds 2 functions to account for these different goals, and
respectively call codeCoverageForDebug when there is no concern about the
optimizing case.
Attachment #8752927 - Flags: review?(jdemooij)
Comment on attachment 8752927 [details] [diff] [review]
Distinguish code coverage usages, and disable it by default in the interpreter.

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

Makes sense.

::: js/src/jit/BaselineCompiler.cpp
@@ +96,5 @@
>      if (!script->ensureHasTypes(cx) || !script->ensureHasAnalyzedArgsUsage(cx))
>          return Method_Error;
>  
> +    // When code coverage is only enabled for optimizations, or when a Debugger
> +    // set the collcetCoverageInfo flag, we have to create the ScriptCounts if

Nit: s/collcet/collect/
Attachment #8752927 - Flags: review?(jdemooij) → review+

Comment 3

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8550398c99fb

Comment 4

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8550398c99fb
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
(Reporter)

Updated

a year ago
Flags: needinfo?(nicolas.b.pierron)
You need to log in before you can comment on or make changes to this bug.