Closed Bug 1666853 Opened 2 months ago Closed 2 months ago

Attempt to improve the code generated for GC barriers

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
83 Branch
Tracking Status
firefox83 --- fixed

People

(Reporter: jonco, Assigned: jonco)

Details

(Keywords: perf-alert)

Attachments

(5 files)

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.

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

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

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

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

Attachment #9177683 - Attachment description: Bug 1666853 - Allow inlining preconditions for Value barriers r?sfink → Bug 1666853 - Part 2: Inline preconditions for Value barriers r?sfink
Attachment #9177684 - Attachment description: Bug 1666853 - Move barrier implementations out of derived cell tyes into standalone functions r?sfink → Bug 1666853 - Part 3: Move barrier implementations out of derived cell tyes into standalone functions r?sfink
Attachment #9177685 - Attachment description: Bug 1666853 - Remove use of ApplyGCThingTyped from Value barriers as this doesn't generate good code r?sfink → Bug 1666853 - Part 4: Remove use of ApplyGCThingTyped from Value barriers as this doesn't generate good code r?sfink
Attachment #9177686 - Attachment description: Bug 1666853 - Add a simpler gray unmarking interface that's easier to call from our read barrier r?sfink → Bug 1666853 - Part 5: Add a simpler gray unmarking interface that's easier to call from our read barrier r?sfink
Pushed by jcoppeard@mozilla.com:
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

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

You need to log in before you can comment on or make changes to this bug.