Closed Bug 712022 Opened 8 years ago Closed 6 years ago

use templates to unify GC shape marking paths

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED WONTFIX

People

(Reporter: mccr8, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

There are two separate marking paths, one for the GC itself using mark stacks, and a more general one for all other clients.  These differ at the leaves, but there's a fair amount of overlap.  I'd like a third marking path for the cycle collector, which has its own needs (don't trace strings, trace through but don't mark shapes, etc.), but I'd prefer to do that without duplicating the logic all over again.

I've started playing around with using templates to unify the code for the three separate marking paths.  It seems to work out nicely for shapes and base shapes.  Other things seem to be more complex due to optimizations and whatnot.

This probably interferes with the incremental GC patch, so we probably don't want to do it immediately, but I think it could be a nice simplification of the code.

The attached patch is fairly gratuitously on top of my patch for bug 710492.  It compiles (at least jsgcmark.cpp and jsfriendapi.cpp) but I haven't tested it at all.
The CC tracing path in the patch is an improvement over trunk, because when you trace through a shape, the callback is only invoked on gray objects (assuming wantAllTraces is not set).  Strings and black objects are skipped in JS, and all shapes and base shapes are traced through in JS without mentioning them to XPConnect.
fixed minor compilation error in nsXPConnect.
Attachment #582848 - Attachment is obsolete: true
Depends on: 710492
Blocks: 653013
I spent some time today trying to merge JSObject marking, but it is a bit nasty.  When slot marking encounters an object, scanning saves the current slot marking state, then starts scanning the object.  markChildren, on the other hand, marks the object immediately (because it isn't being further traversed) and continues on the slot.  I'm not sure how to merge these without doing something heavy handed like just returning a boolean that says whether to switch to object marking or not.

Is it the case that the only values that need marking in slots are objects and strings?  I guess it would be weird if there were shapes or base shapes, but there's no assertions there that this actually holds.
Added support for objects.  It is a little hacky to try to support the two different traversal orders for fields containing objects.

I also added in the top level changes for a CC-specific MarkChildren, and the simplification for NoteJSChild, but the latter isn't really right until type objects and scripts are dealt with.
Attachment #582855 - Attachment is obsolete: true
I'm going to wait until Terrence lands his GC cleanups before I do this.  It is a lower priority from the CC perspective, thanks to Smaug's CC patches which get rid of a ton of JS from the grpah, but it would still be nice to have.
Depends on: 721463
I'm just going to focus on marking shapes for now.  The conversion can be done piecemeal.
Summary: use templates to unify GC marking paths → use templates to unify GC shape marking paths
Assignee: general → continuation
Attachment #584228 - Attachment is obsolete: true
Looks pretty awesome to me.  The only concern I have is that we need to be very careful to not regress performance: the differences between PushMarkStack and MarkChildren are minimal, but extremely subtle and frequently important.

Also, this is going to conflict pretty heavily with bug 728343, so we should coordinate our landings.
After talking to Bill, I think it is enough for my purposes to merge the CC shape tracer into MarkChildren, so I'll leave this alone for now.
Assignee: continuation → general
Bill wasn't a fan of this, and I don't really care about this any more.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.