1668672 - Add LifoAlloc to CompilationStencil

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Ted Campbell [:tcampbell]]

Since the ParserAtomsTable keeps all its atoms alive until it is destroyed, it is a great candidate for LifoAlloc. This should improve performance of it, and later we can use a LifoAlloc for the CompilationStencil itself.

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: Bug 1668672 - Store ParserAtomEntry in CompilationStencil::alloc. r?arai!

The lifetime of the ParserAtomEntries in the ParserAtomTable is the same as
the ParserAtomsTable. Use a LifoAlloc to speed up allocation and destruction.

Comment 2

[Ted Campbell [:tcampbell]]

I'm expanding scope of this bug to include putting the LifoAlloc directly in the CompilationStencil.

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1668672 - Add LifoAlloc to CompilationStencil. r?arai!

Later patches will use this to move allocations here to speed up cleanup of
stencils.

Comment 4

[Ted Campbell [:tcampbell]]

Attachment: Bug 1668672 - Use LifoAlloc for ScriptStencil gcThings list. r?arai!

To speed up cleanup of CompilationStencil, we now use the LifoAlloc to hold
the GC things. The frontend accumulates items on the stack before passing the
complete set to the LifoAlloc. This avoids the unique allocations introduced
by the stencil work and is generally better all around. We also use stack
reservation to cover most gcThings lists without ever allocating.

Depends on D95296

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/656da7d03e9f
Add LifoAlloc to CompilationStencil. r=arai
https://hg.mozilla.org/integration/autoland/rev/360ff991b0d3
Store ParserAtomEntry in CompilationStencil::alloc. r=arai

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/656da7d03e9f
https://hg.mozilla.org/mozilla-central/rev/360ff991b0d3

Comment 7

[Tooru Fujisawa [:arai]]

After the above all patches, the score for bug 1674306 bench (with XDR files fixed to follow updated structure), with stencil-mvp, improves from 14165 to 12319, that's 13% improvement.
the score for without-stencil-mvp also improves from 7106 to 6692,
and stencil-mvp is 84% regression from without-stencil-mvp.
I'll check bug 1674351 patch shortly.

Comment 8

[Ted Campbell [:tcampbell]]

TODO: Apply same approach to ScopeStencil::data, BigIntStencil::buf, and RegExprStencil::buf.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/00186be8038a
Use LifoAlloc for ScriptStencil gcThings list. r=arai

Comment 10

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/00186be8038a

Comment 11

[Ted Campbell [:tcampbell]]

Attachment: Bug 1668672 - Move XDRParser*ScopeData methods to StencilXDR interface. r?arai!

The StencilXDR interface has access to internals of ScopeStencil so having
accessing to the explicit methods is helpful. This also uses the stencil as
the argument instead of a UniquePtr& which makes it easier to change types
later.

Comment 12

[Ted Campbell [:tcampbell]]

Attachment: Bug 1668672 - Use the Stencil's LifoAlloc for ParserScopeData in frontend. r?arai!

Instead of allocating temporary ParserScopeData on the JSContext's temporary
LifoAlloc, we use the Stencil's LifoAlloc. Will will allow using this
allocation directly in the stencil in future.

Depends on D95941

Comment 13

[Ted Campbell [:tcampbell]]

Attachment: Bug 1668672 - Use LifoAlloc for Stencil ParserScopeData. r?arai!

Have the ScopeStencil data be owned by the Stencil LifoAlloc. This reduces
cleanup costs for the stencil. Also directly re-use the allocation from the
frontend and removing the extra copies (and code that does the copy).

Depends on D95942

Comment 14

[Ted Campbell [:tcampbell]]

Attachment: Bug 1668672 - Use ParserAtom for RegExpStencil. r?arai!

Instead of using a UniqueTwoByteChars for the strings, switch to using a
ParserAtom. This simplifies code, and makes stencil smaller. Instantiating a
RegExpStencil has to generate a JSAtom already, so this is sensible.

Depends on D95943

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6a8ab1e6c7b2
Move XDRParser*ScopeData methods to StencilXDR interface. r=arai
https://hg.mozilla.org/integration/autoland/rev/19fe77d2b578
Use the Stencil's LifoAlloc for ParserScopeData in frontend. r=arai
https://hg.mozilla.org/integration/autoland/rev/5592ca1320c4
Use LifoAlloc for Stencil ParserScopeData. r=arai

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c6b2d17bda5f
Use ParserAtom for RegExpStencil. r=arai

Comment 17

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/6a8ab1e6c7b2
https://hg.mozilla.org/mozilla-central/rev/19fe77d2b578
https://hg.mozilla.org/mozilla-central/rev/5592ca1320c4
https://hg.mozilla.org/mozilla-central/rev/c6b2d17bda5f

Comment 18

[Ted Campbell [:tcampbell]]

I'll close this bug off now. The BigIntStencil probably could be converted too, but first we should see what happens with ScriptThingVariant. The BigInts will turn into just a ParserAtom, but will need a special tag still so we instantiate correctly.

Comment 19

[Florin Strugariu [:Bebe]]

== Change summary for alert #27539 (as of Fri, 06 Nov 2020 08:01:52 GMT) ==

Improvements:

Ratio	Suite	Test	Platform	Options	Absolute values (old vs new)
4%	facebook-redesign	fcp	linux64-shippable-qr	warm webrender	399.67 -> 383.00
4%	facebook-redesign	fcp	linux64-shippable-qr	warm webrender	399.15 -> 383.00
4%	facebook-redesign	fcp	linux64-shippable	warm	399.62 -> 383.67
3%	facebook	loadtime	linux64-shippable	warm	1,047.08 -> 1,012.92
3%	facebook	loadtime	linux64-shippable-qr	warm webrender	1,046.88 -> 1,015.67
2%	instagram	SpeedIndex	android-hw-g5-7-0-arm7-api-16-shippable	warm	2,415.08 -> 2,358.17
2%	instagram	loadtime	android-hw-g5-7-0-arm7-api-16-shippable	warm	2,544.52 -> 2,485.75
2%	facebook-redesign		linux64-shippable-qr	warm webrender	315.75 -> 308.54
2%	facebook-redesign	loadtime	linux64-shippable-qr	warm webrender	438.15 -> 428.83
2%	facebook-redesign	loadtime	linux64-shippable	warm	437.38 -> 428.12
2%	facebook-redesign		linux64-shippable	warm	315.22 -> 308.61

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