1600705 - Stop generating closed-over-binding information for leaf functions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Ted Campbell [:tcampbell]]

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.

Comment 1

[Nicolas B. Pierron [:nbp]]

(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

Comment 2

[Ted Campbell [:tcampbell]]

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.

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1600705 - Avoid storing nullptr closed-over-bindings data for LazyScripts. r?jorendorff

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.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70ee029bd1f5
Avoid storing nullptr closed-over-bindings data for LazyScripts. r=jorendorff

Comment 5

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/70ee029bd1f5