New GC related APIs: Implicit references.

New GC related APIs: Implicit references.

When using these APIs, the embedder doesn't need to copy Persistent handles around.

BUG=NONE

Committed: http://code.google.com/p/v8/source/detail?r=14283

[marja, 2013-04-11 16:50:52]

This is the next step after the GC API changes for object groups.

mstarzinger, svenpanne: wdyt?

Previously, the implicit references are connected to an object, and for that
WebKit needed to keep track of a "representative object" for each group, since
it really wanted to connect implicit references to a group. After this patch,
implicit references are connected to a group.

*But*: for this to work, I had to make the following assumption: the implicit
references are always connected to the first object of an object group. Afaics
this is true for V8GCController. Do other embedders use implicit references?

[Michael Starzinger, 2013-04-16 10:42:16]

LGTM for the implementation with a few comments.

https://codereview.chromium.org/14175005/diff/3001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/14175005/diff/3001/include/v8.h#newcode3546
include/v8.h:3546: static void SetObjectGroupRepresentativeObject(
Can we add a short comment to this method that mentions this is used in
conjunction with implicit references? Also how about naming this just
"SetObjectGroupRepresentative()", I'll leave that up to you and Sven?

https://codereview.chromium.org/14175005/diff/3001/include/v8.h#newcode3569
include/v8.h:3569: * before-garbage-collection callback function. */
nit: Close comment on next line.

https://codereview.chromium.org/14175005/diff/3001/src/global-handles.h
File src/global-handles.h (right):

https://codereview.chromium.org/14175005/diff/3001/src/global-handles.h#newco...
src/global-handles.h:89: struct ObjectGroupRepresentativeObject {
Let's just call this structure "ObjectGroupRepresentative", the additional
"Object" suffix doesn't provide much additional benefit (especially since these
Objects are called HeapObjects internally).

https://codereview.chromium.org/14175005/diff/3001/src/heap-snapshot-generato...
File src/heap-snapshot-generator.cc (right):

https://codereview.chromium.org/14175005/diff/3001/src/heap-snapshot-generato...
src/heap-snapshot-generator.cc:2038: // This should not happen: representative
object for a group was not
Add an "UNREACHABLE();" here.

https://codereview.chromium.org/14175005/diff/3001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/14175005/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:1988: // This should not happen: representative object for a
group was not
Add an "UNREACHABLE();" here.

[Michael Starzinger, 2013-04-16 10:56:08]

One more comment ...

https://codereview.chromium.org/14175005/diff/3001/src/global-handles.cc
File src/global-handles.cc (right):

https://codereview.chromium.org/14175005/diff/3001/src/global-handles.cc#newc...
src/global-handles.cc:908: void GlobalHandles::AddImplicitReference(UniqueId id,
Object** child) {
Can we add an assertion here to check that the child is not marked independent?

ASSERT(!Node::FromLocation(child)->is_independent());

[marja, 2013-04-16 12:28:37]

Thanks for review!

In addition to the done's here, I also updated the tests (test-mark-compact was
mixing old and new api -> didn't pass, and I updated test-heap-profiler to use
the new API).

https://codereview.chromium.org/14175005/diff/3001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/14175005/diff/3001/include/v8.h#newcode3546
include/v8.h:3546: static void SetObjectGroupRepresentativeObject(
On 2013/04/16 10:42:16, Michael Starzinger wrote:
> Can we add a short comment to this method that mentions this is used in
> conjunction with implicit references? Also how about naming this just
> "SetObjectGroupRepresentative()", I'll leave that up to you and Sven?

Done.

https://codereview.chromium.org/14175005/diff/3001/include/v8.h#newcode3569
include/v8.h:3569: * before-garbage-collection callback function. */
On 2013/04/16 10:42:16, Michael Starzinger wrote:
> nit: Close comment on next line.

Done.

https://codereview.chromium.org/14175005/diff/3001/src/global-handles.cc
File src/global-handles.cc (right):

https://codereview.chromium.org/14175005/diff/3001/src/global-handles.cc#newc...
src/global-handles.cc:908: void GlobalHandles::AddImplicitReference(UniqueId id,
Object** child) {
On 2013/04/16 10:56:08, Michael Starzinger wrote:
> Can we add an assertion here to check that the child is not marked
independent?
> 
> ASSERT(!Node::FromLocation(child)->is_independent());

Done.

https://codereview.chromium.org/14175005/diff/3001/src/global-handles.h
File src/global-handles.h (right):

https://codereview.chromium.org/14175005/diff/3001/src/global-handles.h#newco...
src/global-handles.h:89: struct ObjectGroupRepresentativeObject {
On 2013/04/16 10:42:16, Michael Starzinger wrote:
> Let's just call this structure "ObjectGroupRepresentative", the additional
> "Object" suffix doesn't provide much additional benefit (especially since
these
> Objects are called HeapObjects internally).

Done.

https://codereview.chromium.org/14175005/diff/3001/src/heap-snapshot-generato...
File src/heap-snapshot-generator.cc (right):

https://codereview.chromium.org/14175005/diff/3001/src/heap-snapshot-generato...
src/heap-snapshot-generator.cc:2038: // This should not happen: representative
object for a group was not
On 2013/04/16 10:42:16, Michael Starzinger wrote:
> Add an "UNREACHABLE();" here.

Done.

https://codereview.chromium.org/14175005/diff/3001/src/mark-compact.cc
File src/mark-compact.cc (right):

https://codereview.chromium.org/14175005/diff/3001/src/mark-compact.cc#newcod...
src/mark-compact.cc:1988: // This should not happen: representative object for a
group was not
On 2013/04/16 10:42:16, Michael Starzinger wrote:
> Add an "UNREACHABLE();" here.

Done.

[Michael Starzinger, 2013-04-16 12:41:32]

LGTM. I'll land this.

[Michael Starzinger, 2013-04-16 12:57:15]

Committed patchset #3 manually as r14283 (presubmit successful).