Closed
Bug 1156533
Opened 9 years ago
Closed 9 years ago
Simplify how we trace Shapes for marking
Categories
(Core :: JavaScript: GC, defect)
Core
JavaScript: GC
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: terrence, Assigned: terrence)
References
Details
Attachments
(1 file)
16.20 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
Because why use a while loop if a goto will suffice. Considering this particular over-use, I'd be tempted to say that someone thought goto was magic fast sauce. A couple of things in this patch deserve comment. First, the ascii-art diagram is still a bit of a lie -- a few edges need to be trimmed to fit that view still. I hope to make it not a lie before closing bug 1154959. In particular, I've added markAndTraceChildren and /removed/ the default implementation of traverse. This makes using traverse on a non-layout type a linking error. Hence, I've added several static_cast to manually coerce derived types into their base types in the eager and mark-stack paths. This seems like a clearer solution to me than ConvertToBase, given that this is all in the optimized path and I don't want to have an additional internal shim layer. The new test was to verify for myself that the getter/setter isTenured() checks are actually required. Sadly, they are. The pre-barrier does a standard DoMarking, which for Shapes eagerly traverses children. Unfortunate. We are very close to being able to easily add a custom pre-barrier marker that uses a mark stack for all marking instead.
Assignee | ||
Comment 1•9 years ago
|
||
Comment 2•9 years ago
|
||
Comment on attachment 8595005 [details] [diff] [review] 10.1_simplify_shape_tracing-v0.diff Review of attachment 8595005 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/gc/Marking.cpp @@ +55,5 @@ > +// Callback > +// -------- > +// > +// The secondary JSTracer is the CallbackTracer. This simply invokes a callback > +// on each edge in a child. Ok, *now* that all makes sense. I was hung up on the distinction between visiting a node vs tracing its children, but the distinction the naming is based on is just marking vs generic callbacks. Thanks! @@ +92,5 @@ > +// | '----------------------' '----------------' || // > +// | | || || // > +// | | || || // > +// | | || || // > +// o<-----------------o<========================OO============Oo // Nice diagram! @@ +567,1 @@ > // to visit the child edges. In the absense of other traversal mechanisms, this *absence "fully generic traceChild virtual method", maybe? Or perhaps "fully generic" is redundant with "virtual method"? @@ +1051,2 @@ > > + // When triggered onbetween slices on belhalf of a barrier, these "When triggered between slices on behalf..."
Attachment #8595005 -
Flags: review?(sphink) → review+
Assignee | ||
Comment 3•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9d4c64043ab2
Comment 5•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/333aaffb8ad7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in
before you can comment on or make changes to this bug.
Description
•