Closed Bug 1600705 Opened 11 months ago Closed 10 months ago

Stop generating closed-over-binding information for leaf functions

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla73
Tracking Status
firefox73 --- fixed

People

(Reporter: tcampbell, Assigned: tcampbell)

References

(Blocks 1 open bug)

Details

(Whiteboard: [overhead:60k])

Attachments

(1 file)

When we parse into a LazyScript we generate a variable-length allocation (previously called LazyScriptData until Bug 1600439) to hold inner-functions and closed-over-bindings. We currently always allocate this (except for BinAST) even if it is trivial.

A leaf function has no inner functions, and should also have no closed-over-bindings. Unfortunately, we record the closed-over-bindings for each scope with a nullptr delimiter between each scope's corresponding data. This results in generating script-data that only contains nullptrs for leaf functions which is a waste.

One caveat is that class-constructors require this data-structure to store fieldInitializers, but this is a rare case. We should still try to save memory in general.

Another motivation is that we only relazify leaf functions and as a result do not need to keep track of this trivial script-data in order to relazify. This will be useful for Bug 1529456.

(In reply to Ted Campbell [:tcampbell] from comment #0)

A leaf function has no inner functions, and should also have no closed-over-bindings. […]

How is the following case handled?

var h;
function f(s) { var i = 3; eval(s); }
f("h = function g() { return i++; }");
print(h()); // 3
print(h()); // 4

Great question. In general, for the purposes of relazification we would consider HasDirectEval the same way as HasInnerFunctions and still generate the script-data.

I checked for your example what c-o-b flow into the LazyScript::Create and got the following results.

f:  [nullptr]
g:  [nullptr, nullptr]

(The two entries for g are apparently because the scope-chain has a FunctionScope -> NamedLambdaScope -> ...)

This leaves the question of how the frontend does handle this. The variable i is not marked as closed-over. Instead, when we encounter an eval we set allBindingsClosedOver [1], and then when creating the Scope we mark entire scope as closed-over [2].

[1] https://searchfox.org/mozilla-central/rev/04d8e7629354bab9e6a285183e763410860c5006/js/src/frontend/Parser.cpp#9356
[2] https://searchfox.org/mozilla-central/rev/04d8e7629354bab9e6a285183e763410860c5006/js/src/frontend/Parser.cpp#1187,1203

The experiment I ran was to assert during LazyScript::Create that the c-o-b were all null if innerFunctions.empty(). I ran that over the jit-tests and didn't find any counter-examples.

Depends on: 1605207

The closed-over-binding data is generated for each scope with a nullptr
delimiter per scope. This is wasteful when there are no closed-over bindings.
This patch removes trailing nullptr entries and for leaf-functions may result
in the script-data allocation being avoided altogether.

The FullParseHandler::nextLazyClosedOverBinding() will return nullptr after
the end of the gcthings data so that the delazification process is otherwise
unchanged.

Pushed by tcampbell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/70ee029bd1f5
Avoid storing nullptr closed-over-bindings data for LazyScripts. r=jorendorff
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
Blocks: 1541353
You need to log in before you can comment on or make changes to this bug.