1438232 - Can CanReuseScriptForClone be optimized more?

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jan de Mooij [:jandem]]

CanReuseScriptForClone returns false when we have a lazy script:

    // We need to clone the script if we're not already marked as having a
    // non-syntactic scope. If we're lazy, go ahead and clone the script; see
    // the big comment at the end of CopyScriptInternal for the explanation of
    // what's going on there.
    return fun->hasScript() && fun->nonLazyScript()->hasNonSyntacticScope();

This will make the caller clone the function + script (after delazifying).

What if we delazify in CanReuseScriptForClone (since we will delazify anyway) and then check hasNonSyntacticScope()? It seems like that could help avoid script clones.

I don't know how common this case is though; should instrument a browser build to get some data.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Patch

So here's a much nicer patch, thanks to help from Shu.

Let's see if this is also green on Try.

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #0)
> I don't know how common this case is though; should instrument a browser
> build to get some data.

For posterity: I instrumented a browser build and this definitely hits many times when starting the browser, even more when opening devtools. So I hope this will improve both perf and memory usage.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/784c1815c3c4
Handle lazy scripts in CanReuseScriptForClone without forcing a delazify/clone. r=shu

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/784c1815c3c4

Comment 6

[Jan de Mooij [:jandem]]

AWSY report below. This was a 2-3% JS memory usage win.

mccr8, I think lazy parsing for non-syntactic scopes (bug 1364566) should be a lot more effective now. We no longer unnecessarily delazify + clone scripts when cloning such (lazy) functions.

---

== Change summary for alert #11619 (as of Thu, 15 Feb 2018 23:06:33 GMT) ==

Improvements:

  3%  JS windows7-32 pgo stylo     87,252,061.99 -> 84,582,935.05
  3%  JS windows10-64 opt stylo    115,966,089.39 -> 112,636,003.66
  3%  JS windows7-32 opt stylo     86,772,136.57 -> 84,458,974.43
  3%  JS linux64-stylo-sequential opt stylo-sequential104,128,567.46 -> 101,451,809.46
  2%  JS windows10-64 pgo stylo    115,672,796.91 -> 112,963,230.23
  2%  JS linux64 opt stylo         104,154,544.19 -> 101,914,488.72
  2%  JS osx-10-10 opt stylo       103,851,712.72 -> 101,649,556.17

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11619

Comment 7

[Jan de Mooij [:jandem]]

And some perf improvements:

== Change summary for alert #11610 (as of Thu, 15 Feb 2018 23:06:33 GMT) ==

Improvements:

  5%  cpstartup content-process-startup windows7-32 opt e10s stylo     295.67 -> 280.33
  4%  cpstartup content-process-startup windows10-64 opt e10s stylo    301.54 -> 288.92
  4%  cpstartup content-process-startup windows7-32 pgo e10s stylo     263.79 -> 253.33
  3%  cpstartup content-process-startup linux64 pgo e10s stylo         200.38 -> 194.83

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11610