Improve mark-compact object grouping interface.

Improve mark-compact object grouping interface.

The main goal was to improve O(n^2) behavior when there are many object groups.  The old API required the grouping to be done on the v8 side, along with a linear search.  The new interface requires the caller to do the grouping, passing V8 entire groups at a time.  This removes the group id concept on the v8 side.

  - Changed AddObjectToGroup to AddObjectGroup.
  - Removed the group id concept from the V8 side.
  - Remove a static constructor while I'm here, lazily initialize
    the object groups list.
  - Cleaned up return by non-const references to return pointers.

[Dean McNamee, 2008-12-10 14:23:42]

I tried a handful of approaches, and this ended up being the cleanest.  Keeping
a HashMap on V8's side was very cumbersome.  There is still some confusion
between who owns the global list (between global-handles and mark-compact), but
I'm not going to address that now.

I think pushing the most of this to the caller is the best thing we can do. 
This will make it easier to tune things on the bindings side.

This makes an API change, but I suspect we're the only users of this API.

V8 bindings change review coming up right now.

[Christian Plesner Hansen, 2008-12-10 14:38:45]

My $.02.  Lgtm overall.

http://codereview.chromium.org/13341/diff/1/2
File include/v8.h (right):

http://codereview.chromium.org/13341/diff/1/2#newcode1973
Line 1973: static void AddObjectGroup(Persistent<Object>* objects, size_t
length);
This might as well be Persistent<Value>* since the first thing you do is cast it
to a i::Object***.  Confusingly, v8::Value is equivalent i::Object** whereas
v8::Object is equivalent to i::JSObject.

http://codereview.chromium.org/13341/diff/1/6
File src/mark-compact.cc (right):

http://codereview.chromium.org/13341/diff/1/6#newcode570
Line 570: ObjectGroup* entry = (*object_groups)[i];
You would usually use object_groups->at(i).

[Mads Ager (chromium), 2008-12-10 15:58:35]

LGTM

[Feng Qian, 2008-12-10 16:31:28]

LGTM,

my only comment is that, in the future, we might want to expose a
collection-like interface (or V8 understand a collection-like interface) so the
API can use the collection interface rather than an array of pointers.

On 2008/12/10 14:23:42, Dean McNamee wrote:
> I tried a handful of approaches, and this ended up being the cleanest. 
Keeping
> a HashMap on V8's side was very cumbersome.  There is still some confusion
> between who owns the global list (between global-handles and mark-compact),
but
> I'm not going to address that now.
> 
> I think pushing the most of this to the caller is the best thing we can do. 
> This will make it easier to tune things on the bindings side.
> 
> This makes an API change, but I suspect we're the only users of this API.
> 
> V8 bindings change review coming up right now.

[Dean McNamee, 2008-12-11 11:04:53]

I added the changes to test/cctest/test-mark-compact.cc, tests pass.  If it
looks ok I'll commit.  Thanks.

On 2008/12/10 16:31:28, Feng Qian wrote:
> LGTM,
> 
> my only comment is that, in the future, we might want to expose a
> collection-like interface (or V8 understand a collection-like interface) so
the
> API can use the collection interface rather than an array of pointers.
> 
> On 2008/12/10 14:23:42, Dean McNamee wrote:
> > I tried a handful of approaches, and this ended up being the cleanest. 
> Keeping
> > a HashMap on V8's side was very cumbersome.  There is still some confusion
> > between who owns the global list (between global-handles and mark-compact),
> but
> > I'm not going to address that now.
> > 
> > I think pushing the most of this to the caller is the best thing we can do. 
> > This will make it easier to tune things on the bindings side.
> > 
> > This makes an API change, but I suspect we're the only users of this API.
> > 
> > V8 bindings change review coming up right now.

[Christian Plesner Hansen, 2008-12-11 11:10:36]

Lgtm!