Thanks for the review.
https://codereview.chromium.org/768633002/diff/1/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/768633002/diff/1/src/objects.cc#newcode8112
src/objects.cc:8112: Handle<FixedArray>::cast(array)->set(index +
kReservedFieldCount, *cell);
On 2014/12/01 15:23:54, Toon Verwaest wrote:
So kReservedFieldCount is just an internal header? What about naming
it
kFirstIndex like in the other arrays?
Done.
https://codereview.chromium.org/768633002/diff/1/src/objects.cc#newcode8158
src/objects.cc:8158: int first_index = array->last_used_index();
On 2014/12/01 15:23:54, Toon Verwaest wrote:
Seems like you would want to start at last_used_index+1?
No; the last element may have been added and then removed again, so it's
best to start searching at its index.
https://codereview.chromium.org/768633002/diff/1/src/objects.cc#newcode8168
src/objects.cc:8168: if (i == array->length()) i = 0;
On 2014/12/01 15:23:54, Toon Verwaest wrote:
i = (i + 1) % array->length();
Done.
https://codereview.chromium.org/768633002/diff/1/src/objects.cc#newcode9791
src/objects.cc:9791: if (!WeakFixedArray::StoreAnywhere(array, user)) {
On 2014/12/01 15:23:54, Toon Verwaest wrote:
Handle<WFA> new_array = WFA::Add(array, user);
must_store = !new_array.is_identical_to(array);
Done.
https://codereview.chromium.org/768633002/diff/1/src/objects.cc#newcode9808
src/objects.cc:9808: Handle<Object> maybe_array =
JSObject::GetProperty(&it).ToHandleChecked();
On 2014/12/01 15:23:54, Toon Verwaest wrote:
JSObject::GetProperty(prototype, symbol);
Done.
https://codereview.chromium.org/768633002/diff/1/src/objects.cc#newcode9837
src/objects.cc:9837: if (!back->IsMap()) return true;
On 2014/12/01 15:23:54, Toon Verwaest wrote:
That would include maps that aren't part of a transition tree right?
Including
slow-mode maps.
Done -- blacklisted dictionary maps. On the other hand, these rules
obviously depend on what we need the information for, so we anyway might
have to adapt them over time as necessary.
https://codereview.chromium.org/768633002/diff/1/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/768633002/diff/1/src/objects.h#newcode2601
src/objects.h:2601: static bool StoreAnywhere(Handle<WeakFixedArray>
array,
On 2014/12/01 15:23:54, Toon Verwaest wrote:
What about combining StoreAnywhere with GrowAndStore into
Handle<WFA> Add(Handle<WFA>, ...)?
Done.
https://codereview.chromium.org/768633002/diff/1/src/objects.h#newcode2607
src/objects.h:2607: void Delete(Handle<HeapObject> value);
On 2014/12/01 15:23:54, Toon Verwaest wrote:
And then rename to Remove to match Add.
Done.
https://codereview.chromium.org/768633002/diff/1/src/objects.h#newcode2627
src/objects.h:2627: friend class Factory; // For kReservedFieldCount.
On 2014/12/01 15:23:54, Toon Verwaest wrote:
Just move the method onto WeakFixedArray rather than having this
indirection
over the factory. Just like DescriptorArray::Allocate. In that case,
you don't
need to friend anything.
Done. I've merged CopyFrom into it, to avoid redundant initialization.
https://codereview.chromium.org/768633002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
