Closed Bug 1785804 Opened 4 years ago Closed 4 years ago

Consider removing DoMarking fast path in tracer dispatch

Categories

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

Product:
Core
Component:
JavaScript: GC
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
106 Branch
Tracking Flags:
Tracking Status
firefox106 --- fixed

People

(Reporter: jonco, Assigned: jonco)

References

Details

Attachments

(7 external files)

Tracing GC things in SpiderMonkey works via virtual method dispatch, except for a special fast path for marking:

https://searchfox.org/mozilla-central/source/js/src/gc/Marking.cpp#724-727

Removing this would simplify the code and make it easier to follow by making marking just another kind of tracing.

It's possible that this will regress performance. However marking also has special cases for eager marking of non-cyclic GC things and for marking objects and these should end up being used most of the time.

Eventually we should be able to get the compiler to devirtualise these calls, which would mitigate any performance impact.

To make things clear, this is about removing the DoMarking fast path, not the GCMarker::eagerlyMarkChildren fast path, which will be investigated in a later bug.

Summary: Consider removing marking fast path in tracing dispatch → Consider removing DoMarking fast path in tracer dispatch

imported patch remove-unused-gc-number-method

This moves all GC thing trace methods (and optimised GCMarker versions that do
eager marking) to their own header. With later patches this should allow
devirtualisation of the tracer dispatch.

A follow on bug will attempt to remove some of the GCMarker versions.

Depends on D156558

One point of difference between the DoMarking fast path and other tracers is
handling the edge name. Passing this through to the concrete tracer and
handling it there removes this differemce.

Eventually we should be able to inline some of these these calls at which point
it should get optimsed away for tracers that ignore it.

Depends on D156559

Similar to the previous patch, this delegates whether to update the edge to the
implementation of the trace method, so we don't need generic tracing code to
handle that possiblity any more.

Depends on D156560

This removes the fast path so we use virtual dispatch for all tracers.

Depends on D156561

All tracers are now GenericTracers so there's no reason for this to be a
separate thing.

Depends on D156562

For non-tagged pointer types, DoCallback is now simple enough that we can
define this inline in Tracing.h and most TraceEdge methods become a virtual
method call.

Depends on D156563

Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/55855c5ad184 Part 0: Remove unused enum values and methods in TracingAPI.h r=sfink https://hg.mozilla.org/integration/autoland/rev/271975a555d0 Part 1: Move GC thing trace methods to an inline header file r=sfink https://hg.mozilla.org/integration/autoland/rev/a115e9eaffee Part 2: Remove tracing name context and pass name through the trace methods r=sfink https://hg.mozilla.org/integration/autoland/rev/f9e864cd34a6 Part 3: Pass edge pointer to trace methods r=sfink https://hg.mozilla.org/integration/autoland/rev/27771c8d9865 Part 4: Remove DoMarking fast path and make GCMarker a GenericTracer r=sfink https://hg.mozilla.org/integration/autoland/rev/361dc2862d74 Part 5: Merge GenericTracer into JSTracer r=sfink https://hg.mozilla.org/integration/autoland/rev/31ef32f519f2 Part 6: Inline TraceEdge methods for non-tagged pointers r=sfink
Pushed by abutkovits@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1e226dbfe8fa Part 0: Remove unused enum values and methods in TracingAPI.h r=sfink https://hg.mozilla.org/integration/autoland/rev/6897324c32c5 Part 1: Move GC thing trace methods to an inline header file r=sfink https://hg.mozilla.org/integration/autoland/rev/88c46af9c2c5 Part 2: Remove tracing name context and pass name through the trace methods r=sfink https://hg.mozilla.org/integration/autoland/rev/1fc5ea164fd7 Part 3: Pass edge pointer to trace methods r=sfink https://hg.mozilla.org/integration/autoland/rev/5aaa5504196b Part 4: Remove DoMarking fast path and make GCMarker a GenericTracer r=sfink https://hg.mozilla.org/integration/autoland/rev/2b2a03b16808 Part 5: Merge GenericTracer into JSTracer r=sfink https://hg.mozilla.org/integration/autoland/rev/81fd7885f181 Part 6: Inline TraceEdge methods for non-tagged pointers r=sfink

Relanded because the real issue was from a different bug.

Flags: needinfo?(jcoppeard)
Blocks: 1790061
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: