1690277 - Snapshot delazification enclosing scope

Status: REOPENED

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

separated from bug 1660275.

to snapshot enclosing scope for delazification's usage, we need a list of free names in the lazy function,
and that requires storing the list in the lazy function itself (atoms in gcthings list).
that results in 2% awsy regression.

we should revisit this once we introduce delazify from stencil.

https://phabricator.services.mozilla.com/D103579 and https://phabricator.services.mozilla.com/D103580 contains previous attempt.

Comment 1

[Tooru Fujisawa [:arai]]

maybe it's better just snapshot the entire scope chain, instead of limiting to free names?
so that we don't have to keep the list in gcthings, and awsy regression won't happen.
If there's not too much variables, snapshotting the entire scope chain won't be much problematic.

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

When I had originally been thinking about this, many moons ago, I had seen us using a parallel data structure that merely pointed to pre-existing Scope::Data blocks (now renamed to Scope::RuntimeData I think)

If that's feasible (once ownership issues are solved), then you'd not have very much extra space at all.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690277 - Snapshot delazification enclosing scope.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/72fcd56bd169
Snapshot delazification enclosing scope. r=mgaudet

Comment 5

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/72fcd56bd169

Comment 6

[Dorel Luca [:dluca]]

Backed out changeset 72fcd56bd169 (bug 1690277) on Dev's request

Backout:
https://hg.mozilla.org/integration/autoland/rev/c5a7e3ee7a6307a61d4fa7f2bda6ca5345d284ea

Comment 7

[Tooru Fujisawa [:arai]]

thanks!

I'll look into other way to solve the issue.

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backout merged: https://hg.mozilla.org/mozilla-central/rev/c5a7e3ee7a63

Comment 9

[Ionuț Goldan [:igoldan]]

When this landed in comment #5, we noticed these perf regressions (the backout cancelled all of them):

== Change summary for alert #28740 (as of Thu, 11 Feb 2021 10:37:26 GMT) ==

Regressions:

Ratio	Suite	Test	Platform	Options	Absolute values (old vs new)
10%	raptor-tp6-wikipedia-firefox-cold		windows10-64-shippable-qr	nocondprof webrender	1,522.54 -> 1,680.01
7%	raptor-tp6-twitch-firefox-cold	loadtime	windows10-64-shippable-qr	nocondprof webrender	939.38 -> 1,003.08
6%	raptor-tp6-twitch-firefox-cold		windows10-64-shippable-qr	nocondprof webrender	291.48 -> 310.29

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=28740

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:arai, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 11

[Tooru Fujisawa [:arai]]

The patch didn't work, in term of performance, and we need another solution.
We can keep the current behavior for now.