Closed Bug 634651 Opened 14 years ago Closed 14 years ago

Need API to distinguish "roots" from "stacks"

Categories

(Tamarin Graveyard :: Garbage Collection (mmGC), defect, P3)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Q3 11 - Serrano

People

(Reporter: lhansen, Assigned: lhansen)

References

Details

(Whiteboard: has-patch)

Attachments

(1 file, 1 obsolete file)

Currently the GCRoot API is used for at least two things: - garbage collection roots, ie, objects that are roots for GC and for which write barriers are not required for assignment. These need to be scanned at the beginning and end of a GC cycle: at the beginning to get the collection going, at the end for correctness. - stack-like memory -- AS1 stack, AVM+ "alloca" system, ie, objects that are roots for the GC in a sense and which do not require write barriers, but which need be scanned only at the end of the GC cycle for correctness. Currently we are allowing GC roots to be split by the marker to limit the height of the mark stack, but that is mainly to accomodate the "stack" use cases; normal GC root objects are quite small and should generally not be numerous, and need not be split for incrementality nor for keeping the mark stack small. The "stack" use case will need to split the objects, but those split objects will never be deleted while marking is ongoing (stack marking is atomic), so the splitting protocol we're currently using for roots, and which is complicated, would not need to be used.
I've found only two uses of the "general" GCRoot constructor in the entire system: to register the pointer to emptyWeakRef (this is gratuitous, since the GC knows about this and could just push it onto the mark stack in MarkAllRoots) and in the construction of ActionScriptStack in the AS1 interpreter. In addition VMPI_alloca comes into play but that's a special case. In all of the Flash Player outside tamarin only 32 classes subclass GCRoot, from what I can tell, and there's no indication anything stack-like should be going on with any of them. Strawman proposal: I propose that we remove the three-argument constructor from GCRoot and move it into a new type: namespace MMgc { class StackMemory : public GCRoot { public: StackMemory(GC *gc, const void *object, size_t size); ... } } This is exactly like GCRoot except that its memory will be scanned only at the end of a collection cycle, and for the moment: - It will be scanned conservatively always. - Pointers from the memory into the middle of other objects will always be recognized as pointers to those objects. That last bullet is debatable but is what we do for "normal" stack memory. We don't currently do it for roots (ergo for the AS1 stack or for alloca'd memory) and there's a risk of performance regressions if we start doing it. We could make it configurable (AVMTWEAK?) but not clear to me what the benefits are. Longer term it's obviously beneficial for the user of alloca (at least) to be able to say "this memory does not contain pointers" (stack-allocated buffers, strings) but I believe there's a separate alloca bug that tracks that issue.
StackMemory would also need the Set() method, which will be removed from GCRoot.
Attached patch Patch (obsolete) (deleted) — Splinter Review
I've yet to test this in the Player which is the main reason why I'm only asking for feedback; so far as I'm concerned this is roughly right. (Sits on top of the mark stack rewrite; there aren't a lot of tie-ins but you can't apply this to today's TR.)
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Attachment #513441 - Flags: feedback?(treilly)
Comment on attachment 513441 [details] [diff] [review] Patch Big +1 to scanning Stack's only at finish time, I always hated that we scanned stacks in start. Interior pointers for ActionScriptStack could result in int atoms pinning stuff they didn't before but we're gonna have to have the ability to exactly scan array's of ScriptAtom's anyways (based on profiling we've done so far) so we might as well use that on ActionScriptStack too so I wouldn't sweat it for now. ActionScriptStack lives in a GCRoot already so it could be served by the notion of a GCObject that's always scanned at finish time if such a thing made sense generally, not sure. Would that make exact gc tie in easier?
Attachment #513441 - Flags: feedback?(treilly) → feedback+
So far as I can tell the only change needed in the Player for ActionScriptStack is this: === flash/core/stack.h === 130c130 < MMgc::GCRoot root; --- > MMgc::StackMemory root; I think we'll hold off on further decisions about ActionScriptStack until I've done more work on the exact tracing API for roots. On thing that would probably make sense for these kinds of stacks is an API that returns the stack pointer for the item, so that the marker does not mark dead memory.
Attached patch Patch (deleted) — Splinter Review
Pretty much the same as the previous one, with the addition of a virtual method based API on StackMemory to ask for the active size of the memory segment. The AS1 stack should be able to use that API without much hassle; the alloca stack needs a little restructuring probably.
Attachment #513441 - Attachment is obsolete: true
Attachment #513955 - Flags: review?(treilly)
Whiteboard: has-patch
Priority: -- → P3
Target Milestone: --- → flash10.x-Serrano
Attachment #513955 - Flags: review?(treilly) → review+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
changeset: 5953:fb4760cc5b5f user: Lars T Hansen <lhansen@adobe.com> summary: Fix 634651 - Need API to distinguish "roots" from "stacks" (r=treilly) http://hg.mozilla.org/tamarin-redux/rev/fb4760cc5b5f
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: