Closed
Bug 1364120
Opened 8 years ago
Closed 8 years ago
Remove FINISH_LARGE_EVALUATE gc.
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: nbp, Assigned: nbp)
Details
Attachments
(1 file)
6.19 KB,
patch
|
jonco
:
review+
|
Details | Diff | Splinter Review |
Returning the JSScript from JS::Evaluate is necessary for triggering the encoding of the bytecode. The JSScript is used by the StartIncrementalEncoding and FinishIncrementalEncoding which have to be called by the ScriptLoader in order to save the bytecode used during the execution.
Assignee | ||
Comment 1•8 years ago
|
||
This change reverts the modification added in Bug 852331. It is outdated for
or multiple reasons:
- We no longer have the bytecode analysis straight after the full parse.
- We have Lazy parsing now.
- We replaced the analysis by using IonBuilder at the time of the first Baseline
compilation which uses a LifoAlloc, and which are discarded right
away. (almost as mentioned in Bug 852331 comment 1)
- LARGE_SCRIPT_LENGTH is above the HUGE_LENGTH heuristics of the
CanCompileOffThread function, which takes another path for executing the
code compiled off-main-thread.
Attachment #8866845 -
Flags: review?(jcoppeard)
Comment 2•8 years ago
|
||
Comment on attachment 8866845 [details] [diff] [review]
Remove FINISH_LARGE_EVALUATE gc.
Review of attachment 8866845 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good. There's always a small chance that changes like this will cause some kind of regression though so be prepared.
::: js/public/GCAPI.h
@@ -107,5 @@
> D(INTER_SLICE_GC) \
> D(REFRESH_FRAME) \
> D(FULL_GC_TIMER) \
> D(SHUTDOWN_CC) \
> - D(FINISH_LARGE_EVALUATE) \
Please replace this with UNUSED2 so that existing telemetry data retains the same meaning.
Attachment #8866845 -
Flags: review?(jcoppeard) → review+
Assignee | ||
Comment 3•8 years ago
|
||
For Bug 1364117, I will also need to insert a call to StartIncrementalEncoding between the compilation and the execution. Doing it in the Evaluate function might add much more API changes, and way to ignore them in existing functions.
So I will keep the current patch as-is, and split the Evaluate function call into CompileScript & JS_Execute in the dom/base/nsJSUtils.cpp file.
Summary: Add a JS::Evaluate variant which returns the computed JSScript → Remove FINISH_LARGE_EVALUATE gc.
Pushed by npierron@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9414aca02ae0
Remove FINISH_LARGE_EVALUATE gc. r=jonco
Comment 5•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•