589172 - Allow GCObjects to enlist and delist as GCRoots

Status: RESOLVED FIXED

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

Description

[Tommy Reilly]

The player has a few places where things are placed into a GCRoot (or a GCRoot is created) in order to temporarily prevent an object from being GC'd.  It would be nice to have an API so the GC can do this efficiently on behalf of the player instead forcing each instance into a GCRoot allocation or adding new fields to existing GC roots that don't belong there.  Proposal:

void GC::LockObject(GCObject *);
void GC::UnlockObject(GCObject *);

"pinned" is frequently used for this but that has a specific meaning relating to DRC so I thought something else would be better, open to other ideas.

Comment 1

[Felix S. Klock II [:pnkfelix, :fklock]]

I guess I need to see the use-cases you are trying to cover.

My initial reaction was "why doesn't keeping a local reference from the stack handle this?"

The only situation where a local stack reference would not work is when the control flow is complicated enough that you end up returning place where you want to initiate forced retention -- but in those cases, you need to have a reference to the object in question *somewhere* if you're going to eventually pass it to UnlockObject; so again, why doesn't the reference you presumably already have suffice?  Is the object-reference in question being passed to code that is not managed by MMgc?

I don't really object to the API except for a fear of potential for it to be abused.  Instead of adding these sorts of work-arounds that we'll have to support in the future, I would prefer we make the gc itself more efficient in its support of the api it current offers ...

Comment 2

[Tommy Reilly]

Real world use case:

function f()
{
var cam:CameraUI = new CameraUI();
cam.launch(MediaType.IMAGE);
}

when f returns cam will be GC'd and the semantics we're looking for are that CameraUI is locked in memory until the launch comes back (either successfully or with a failure).   Right now due to the undefined nature of GC sometimes the events would come back and sometimes they wouldn't.

The current solution for this is to make a GCRoot, stuff a weak ref and a strong ref into it and make the strong ref exposed while the asynchronous launch command is in flight.

Comment 3

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #2)
> Real world use case:
> 
> function f()
> {
> var cam:CameraUI = new CameraUI();
> cam.launch(MediaType.IMAGE);
> }
> 
> when f returns cam will be GC'd and the semantics we're looking for are that
> CameraUI is locked in memory until the launch comes back (either successfully
> or with a failure).   Right now due to the undefined nature of GC sometimes the
> events would come back and sometimes they wouldn't.
> 
> The current solution for this is to make a GCRoot, stuff a weak ref and a
> strong ref into it and make the strong ref exposed while the asynchronous
> launch command is in flight.


In your example, what party is responsible for invoking GC::UnlockObject(cam), which presumably happens at some point after f returns and alos after the launch comes back?

From where is the party listed in answer going to get the reference to cam ?  I am still a newbie when it comes to, among other things, real-world AS programming and the event infrastructure; would a launch failure invoke some callback with a reference to cam?

Comment 4

[Tommy Reilly]

Essentially there's two event systems being mated, the underlying IMPL of launch will send an event off to the OS to do the thing and register a callback that includes a pointer to CameraUI (as an opaque data pointer).

Comment 5

[Felix S. Klock II [:pnkfelix, :fklock]]

(Tommy explained enough to me over lunch that I am going to retract my already non-existent objection.)

Note that assuming we do shift to a partially moving collector, this API will need to work with it.  Which is doable (there are a number of strategies for it), but we just should make sure we consider that up-front.  E.g., do we want any arbitrary object to be usable with LockObject?  Or should we make it a property that is associated with an object at its construction time (something that would probably ease supporting a moving collector.)

Comment 6

[Lars T Hansen]

(In reply to comment #5)

> Note that assuming we do shift to a partially moving collector, this API will
> need to work with it.  Which is doable (there are a number of strategies for
> it), but we just should make sure we consider that up-front.

IMO there are going to be so many problems with supporting a partly moving collector that I'm not sure this API will make much difference at all.  Consider that LockObject could be implemented on the Player side by means of dynamically allocated GCRoots and a hash table.

Comment 7

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #6)
> (In reply to comment #5)
> 
> > Note that assuming we do shift to a partially moving collector, this API will
> > need to work with it.  Which is doable (there are a number of strategies for
> > it), but we just should make sure we consider that up-front.
> 
> IMO there are going to be so many problems with supporting a partly moving
> collector that I'm not sure this API will make much difference at all. 
> Consider that LockObject could be implemented on the Player side by means of
> dynamically allocated GCRoots and a hash table.

For the use-cases that Tommy is talking about, I was assuming that LockObject acted as a signal that its argument object-reference could not be moved in memory until UnlockObject is invoked.  (That's why Tommy mentioned the more typical term "pinned" for this feature.)  The idea being that the object-reference could be passed into unmanaged native code.  The moving collector needs to know that this portion of memory dedicated to this object is fixed: the object cannot be moved, and even if the collector thinks that there are no references to the object, the memory cannot be overwritten.

Maybe I'm misunderstanding your hypothetical implementation, but I do not see how it addresses this.  Can you provide more detail?

----

I am now wondering if a slightly different API would be more flexible... e.g. something like 

  GCObject* GC::GetLockedObject(GCObject *);
  void GC::UnlockObject(GCObject *);

where the GC, at its option, may choose to lock the actual argument to GetLockedObject (and return that object-reference), or may instead choose to make a copy of the object in a distinct memory region, and return a reference to that.  This seems like it would open up more implementation strategies -- but it also probably puts too much of a burden on the end-developer (who will need to juggle their references since they cannot assume that GetLockedObject locks its argument, but also cannot assume that it always returns a copy).

Comment 8

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #7)
> The moving
> collector needs to know that this portion of memory dedicated to this object is
> fixed: the object cannot be moved, and even if the collector thinks that there
> are no references to the object, the memory cannot be overwritten.

Then again, I do not know enough about how Lars and Tommy have been planning to fold in a partly moving collector.

Reflecting on what I have seen in the code base so far, I suspect that we are going to have to support object pinning anyway, whether or not an API for it is exposed, and therefore Lars is probably right in saying that Tommy's proposed API would make much difference.

Comment 9

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #8)
> Lars is probably right in saying that Tommy's proposed
> API would make much difference.

edit: would _not_ make much difference.

Comment 10

[Tommy Reilly]

We use "pin" to describe RCObject's we found on the stack so I didn't use that term.   LockObject could be implemented in a moving collector by just moving the object to a non-moving generation (I think java does this).    An API like GetLockedObject as you spec it makes sense but I doubt it would be used correctly by the player.   I think we have re-think the entire GC/Player GC/AVM boundaries to go to a moving collector.

Comment 11

[Lars T Hansen]

References to GCObjects from arbitrary C++ locations are endemic at present.  Thus it's my assumption that an object will only be moveable by a partly-moving collector if the collector can prove that all references to the object are from objects whose layout we control exactly (similar to the exact marking invariant).  With probably fairly major changes to the collector API we may be able to support a more flexible API, which I think is what you're getting at, where we can move anything that's not explicitly pinned by the mutator.  That may be more efficient - even significantly more efficient - and will almost certainly allow more objects to be moved, but I think that it would require such large changes that (a) it won't be the first thing we do and (b) at that point, the cost of changing GetLockedObject will be a rounding error on the total cost of API changes.

Comment 12

[Lars T Hansen]

Attachment: Quick and dirty

Needs a selftest obviously but if you think it's acceptable to return a lock object that we later unlock then this is all we need.

Comment 13

[Lars T Hansen]

Attachment: Patch, v2

Slightly richer API.  Better documentation.  Fixed a bug in gcTrace.  Added a selftest, which should be reviewed too.

Comment 14

[Tamarin Bot]

changeset: 6005:e5427e8b81be
user:      Lars T Hansen <[e-mail]>
summary:   Fix 589172 - Allow GCObjects to enlist and delist as GCRoots (r=treilly)

http://hg.mozilla.org/tamarin-redux/rev/e5427e8b81be

Comment 15

[Felix S. Klock II [:pnkfelix, :fklock]]

Selftests having problem: Bug 637695