Change trail from GrowableObjectArray to ZoneGrowableArray.

Change trail from GrowableObjectArray to ZoneGrowableArray.

BUG=
R=regis@google.com

Committed: https://github.com/dart-lang/sdk/commit/42f32322cb30cb24fb098e54db61b8f3617c7799

[srdjan, 2015-08-26 22:55:23]

srdjan@google.com changed reviewers:
+ regis@google.com

[regis, 2015-08-27 02:30:26]

LGTM if you can use AbstractType in trail.

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/class_finali...
File runtime/vm/class_finalizer.h (right):

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/class_finali...
runtime/vm/class_finalizer.h:152: GrowableObjectArray* pending_types,
Will you do it next for pending_types?

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:14903: RawInstance* AbstractType::OnlyBuddyInTrail(TrailPtr
trail) const {
see comment in header

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:14905: return Instance::null();
ditto

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:14915: return Instance::null();
ditto

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:14924: ASSERT(OnlyBuddyInTrail(*trail) == Object::null());
AbstractType:null()

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:15992: const Instance& buddy) const {
AbstractType instead of Instance.

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.h#new...
runtime/vm/object.h:1530: // TODO(srdjan): Ideally this should be 'const
AbstractType*'.
Why not just do it?

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.h#new...
runtime/vm/object.h:5015: // Object::null() if the receiver is not contained in
the trail.
It could be AbstractType::null() instead of Object::null().

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.h#new...
runtime/vm/object.h:5016: RawInstance* OnlyBuddyInTrail(TrailPtr trail) const;
Could be RawAbstracType instead of RawInstance.

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.h#new...
runtime/vm/object.h:5303: bool TestAndAddBuddyToTrail(TrailPtr* trail, const
Instance& buddy) const;
AbstractType instead of Instance

[srdjan, 2015-08-27 14:54:00]

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.h#new...
runtime/vm/object.h:1530: // TODO(srdjan): Ideally this should be 'const
AbstractType*'.
On 2015/08/27 02:30:26, regis wrote:
> Why not just do it?

The size of this change exploded because of changes required for
TestAndAddBuddyToTrail to accept AbstractType instead of Instance. I will try it
again.

[srdjan, 2015-08-27 16:09:49]

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/class_finali...
File runtime/vm/class_finalizer.h (right):

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/class_finali...
runtime/vm/class_finalizer.h:152: GrowableObjectArray* pending_types,
On 2015/08/27 02:30:25, regis wrote:
> Will you do it next for pending_types?

Maybe. The priority is to fix issues for background compilation first
(allocating in new space while optimizing). I would also like to give it more
thought how to make A ZoneGrowableArray of handles safer to use (elements must
be unique zone handles).

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:14903: RawInstance* AbstractType::OnlyBuddyInTrail(TrailPtr
trail) const {
On 2015/08/27 02:30:26, regis wrote:
> see comment in header

Done.

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:14905: return Instance::null();
On 2015/08/27 02:30:26, regis wrote:
> ditto

Done.

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:14915: return Instance::null();
On 2015/08/27 02:30:25, regis wrote:
> ditto

Done.

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:14924: ASSERT(OnlyBuddyInTrail(*trail) == Object::null());
On 2015/08/27 02:30:26, regis wrote:
> AbstractType:null()

Done.

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:15992: const Instance& buddy) const {
On 2015/08/27 02:30:26, regis wrote:
> AbstractType instead of Instance.

Done.

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.h#new...
runtime/vm/object.h:1530: // TODO(srdjan): Ideally this should be 'const
AbstractType*'.
On 2015/08/27 02:30:26, regis wrote:
> Why not just do it?

Done.

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.h#new...
runtime/vm/object.h:5015: // Object::null() if the receiver is not contained in
the trail.
On 2015/08/27 02:30:26, regis wrote:
> It could be AbstractType::null() instead of Object::null().

Done.

https://codereview.chromium.org/1309113004/diff/20001/runtime/vm/object.h#new...
runtime/vm/object.h:5016: RawInstance* OnlyBuddyInTrail(TrailPtr trail) const;
On 2015/08/27 02:30:26, regis wrote:
> Could be RawAbstracType instead of RawInstance.

Done.

[regis, 2015-08-27 16:24:54]

LGTM

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:14930: *trail = new Trail();
new Trail(4) as elsewhere?

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:15904: if (other.IsAbstractType() &&
It's correct, but I would return false faster:

if (!other.IsAbstractType()) {
  return false;
}
if (TestAndAddBuddyToTrail(&trail, AbstractType::Cast(other))) {
  return true;
}
...

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:15985: *trail = new Trail();
new Trail(4)?

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:16003: *trail = new Trail();
new Trail(4)?

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.h#new...
runtime/vm/object.h:1530: // TODO(srdjan): Ideally this should be 'const
AbstractType*'.
Looks pretty ideal to me.

[srdjan, 2015-08-27 16:36:20]

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:14930: *trail = new Trail();
On 2015/08/27 16:24:53, regis wrote:
> new Trail(4) as elsewhere?

Done.

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:15904: if (other.IsAbstractType() &&
On 2015/08/27 16:24:53, regis wrote:
> It's correct, but I would return false faster:
> 
> if (!other.IsAbstractType()) {
>   return false;
> }
> if (TestAndAddBuddyToTrail(&trail, AbstractType::Cast(other))) {
>   return true;
> }
> ...

Done.

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:15985: *trail = new Trail();
On 2015/08/27 16:24:53, regis wrote:
> new Trail(4)?

Done.

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:16003: *trail = new Trail();
On 2015/08/27 16:24:53, regis wrote:
> new Trail(4)?

Done.

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/1309113004/diff/60001/runtime/vm/object.h#new...
runtime/vm/object.h:1530: // TODO(srdjan): Ideally this should be 'const
AbstractType*'.
On 2015/08/27 16:24:53, regis wrote:
> Looks pretty ideal to me.

It's Puuurrrfect ... thus removed.

[srdjan, 2015-08-27 16:57:01]

Committed patchset #5 (id:80001) manually as
42f32322cb30cb24fb098e54db61b8f3617c7799 (presubmit successful).