1666853 - Attempt to improve the code generated for GC barriers

Status: RESOLVED FIXED

(Core :: JavaScript: GC, enhancement, P3)

Description

[Jon Coppeard (:jonco)]

I looked at the code clang generates for GC barriers and it doesn't seem optimal. The read barrier for JS::Value is 1.5KB for example. With some adjustments the source it should be possible to do much better.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1666853 - Part 1: Remove a redundant heap state check r?sfink

We've already asserted that we're not collecting if we're on the main thread in this method.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1666853 - Part 2: Inline preconditions for Value barriers r?sfink

Currently our Value barriers are all out-of-line. We can at least avoid a call
if the value isn't a GC thing.

Depends on D91308

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1666853 - Part 3: Move barrier implementations out of derived cell tyes into standalone functions r?sfink

This moves barrier implementations out of derived cell types, with the types
supplying necessary information to a central implementation (e.g. with the
isPermanentAndMayBeShared() method).

Depends on D91309

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1666853 - Part 4: Remove use of ApplyGCThingTyped from Value barriers as this doesn't generate good code r?sfink

Depends on D91310

Comment 5

[Jon Coppeard (:jonco)]

Attachment: Bug 1666853 - Part 5: Add a simpler gray unmarking interface that's easier to call from our read barrier r?sfink

Constructing a GCCellPtr means getting the trace kind from a lookup table based
on the alloc kind. We can move this out of line so that code's not included in
the barrier.

Depends on D91311

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/737cf10628b2
Part 1: Remove a redundant heap state check r=sfink
https://hg.mozilla.org/integration/autoland/rev/4caed033bc07
Part 2: Inline preconditions for Value barriers r=sfink
https://hg.mozilla.org/integration/autoland/rev/48cec06d1360
Part 3: Move barrier implementations out of derived cell tyes into standalone functions r=sfink
https://hg.mozilla.org/integration/autoland/rev/be4870668a89
Part 4: Remove use of ApplyGCThingTyped from Value barriers as this doesn't generate good code r=sfink
https://hg.mozilla.org/integration/autoland/rev/1258696a62ee
Part 5: Add a simpler gray unmarking interface that's easier to call from our read barrier r=sfink

Comment 7

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/737cf10628b2
https://hg.mozilla.org/mozilla-central/rev/4caed033bc07
https://hg.mozilla.org/mozilla-central/rev/48cec06d1360
https://hg.mozilla.org/mozilla-central/rev/be4870668a89
https://hg.mozilla.org/mozilla-central/rev/1258696a62ee

Comment 8

[Jon Coppeard (:jonco)]

This resulted in the following improvement:

== Change summary for alert #27079 (as of Mon, 28 Sep 2020 14:25:28 GMT) ==

Improvements:

8% six-speed-sm linux64-shippable opt 10,201.75 -> 9,403.75

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