Refactor public tracing interface to remove details of heap storage
Categories
(Core :: JavaScript: GC, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox83 | --- | fixed |
People
(Reporter: jonco, Assigned: jonco)
References
Details
Attachments
(13 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
Right now the heap storage is part of the tracing interface because our |onFooEdge| virtual methods take a Foo**. If we want to try and make heap storage atomic (for concurrent marking) then that will have to change this interface. It would be preferable to keep these details out of public interface by only exposing the |onChild| interface, which is the only part that gets used outside SpiderMonkey anyway.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
The instances of this are called 'actions' everywhere and I think it makes
sense that the type should be called that. Also make it an enum class and move
it into the JS namespace.
Assignee | ||
Comment 2•4 years ago
|
||
Currently we have two different enums that determine the tracer kind. Combine
this into a single enum and move it to the JS namespace.
Depends on D92252
Assignee | ||
Comment 3•4 years ago
|
||
This splits out a GenericTracer class (with a virtual method per edge kind)
from CallbackTracer (with a single virtual method for all edges). GenericTracer
is used interally whereas all external uses use CallbackTracer.
Depends on D92253
Assignee | ||
Comment 4•4 years ago
|
||
Depends on D92254
Assignee | ||
Comment 5•4 years ago
|
||
This gives JSTracer a nullable pointer to a JS::TracingContext. This will only be set for CallbackTracers.
Depends on D92255
Assignee | ||
Comment 6•4 years ago
|
||
This isn't used anywhere so we can make onChild void.
Depends on D92256
Assignee | ||
Comment 7•4 years ago
|
||
Existing uses changed to construct and GCCellPtr and call JS::TraceChildren instead.
Depends on D92257
Assignee | ||
Comment 8•4 years ago
|
||
Depends on D92258
Updated•4 years ago
|
Updated•4 years ago
|
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ac952bad0e4e Rename WeakMapTraceKind to WeakMapTraceAction and make it an enum class r=sfink https://hg.mozilla.org/integration/autoland/rev/862bae6bac69 Combine TracerKindTag and TracerKind r=sfink https://hg.mozilla.org/integration/autoland/rev/7cfb76e5d6ba Split CallbackTracer into GenericTracer and CallbackTracer r=sfink https://hg.mozilla.org/integration/autoland/rev/940c9b71f178 Move canSkipJsid to JSTracer so it's together with the other tracing options r=sfink https://hg.mozilla.org/integration/autoland/rev/52e9a1f9b9c1 Split out tracing context information into a separate class r=sfink https://hg.mozilla.org/integration/autoland/rev/ea033bf01dd0 Remove unused onChild return value r=sfink https://hg.mozilla.org/integration/autoland/rev/fef8db29083e Remove deprectated js::TraceChildren function that takes separate pointer and trace kind r=sfink https://hg.mozilla.org/integration/autoland/rev/6ae7a5b34ffb Remove JS_GetTraceThingInfo from the public API since it's only used internally r=sfink
Comment 10•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ac952bad0e4e
https://hg.mozilla.org/mozilla-central/rev/862bae6bac69
https://hg.mozilla.org/mozilla-central/rev/7cfb76e5d6ba
https://hg.mozilla.org/mozilla-central/rev/940c9b71f178
https://hg.mozilla.org/mozilla-central/rev/52e9a1f9b9c1
https://hg.mozilla.org/mozilla-central/rev/ea033bf01dd0
https://hg.mozilla.org/mozilla-central/rev/fef8db29083e
https://hg.mozilla.org/mozilla-central/rev/6ae7a5b34ffb
Assignee | ||
Comment 11•4 years ago
|
||
Assignee | ||
Comment 12•4 years ago
|
||
I gave TraceOptions some implicit consturctors to make it easier to set a
single option by passing an enum value.
Depends on D92595
Comment 13•4 years ago
|
||
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0e6a88c27779 Remove JSTracer's checkEdges option which doesn't seem to be necessary any more r=sfink https://hg.mozilla.org/integration/autoland/rev/5bc8cb307c61 Move trace options into a separate options class passed when the tracer is created r=sfink
Comment 14•4 years ago
|
||
Backed out for build bustages.
Log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=317868860&repo=autoland&lineNumber=80220
Backout: https://hg.mozilla.org/integration/autoland/rev/1dc072fcc9e81e8a93341034bd601a2b03c3bd12
Comment 15•4 years ago
|
||
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/118d9d444b76 Remove JSTracer's checkEdges option which doesn't seem to be necessary any more r=sfink https://hg.mozilla.org/integration/autoland/rev/a6331448cef2 Move trace options into a separate options class passed when the tracer is created r=sfink
Comment 16•4 years ago
|
||
bugherder |
Assignee | ||
Comment 17•4 years ago
|
||
These don't need to be in the public header. We can do this with static functions.
Assignee | ||
Comment 18•4 years ago
|
||
This API is only used in one place, with a callback that calls into a tracer,
so we can make this take a tracer in the first place and cut out one level of
indirection.
Depends on D93152
Comment 19•4 years ago
|
||
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ac6536591c1c Move disptachToOnEdge methods out of class definition r=sfink https://hg.mozilla.org/integration/autoland/rev/a90ba60415cc Change VisitGrayWrapperTargets API to take a tracer rather than a callback r=sfink
Assignee | ||
Comment 20•4 years ago
|
||
The final patch in this series. This changes the GenericTracer APIs to take a
thing pointer and return a possibly updated versions, rather than taking a
double pointer to the thing. This means that we can change the details of how
pointers are stored in the heap without chaning this interface.
Comment 21•4 years ago
|
||
bugherder |
Comment 22•4 years ago
|
||
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1547b1a71895 Change GenericTracer API so that it doesn't include details of heap storage r=sfink
Assignee | ||
Updated•4 years ago
|
Comment 23•4 years ago
|
||
bugherder |
Description
•