Reviewers: Toon Verwaest,
Message:
Hi Toon,
Thanks for the great comments, I've addressed all but one where I have a
question (JSObject::ResetElements() change).
thx,
--Michael
https://codereview.chromium.org/228333003/diff/1/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/228333003/diff/1/src/objects.cc#newcode3153
src/objects.cc:3153: static void Insert(Name* key,
On 2014/04/08 12:58:55, Toon Verwaest wrote:
Can't call from unhandlified into handlified code!
Fixed.
https://codereview.chromium.org/228333003/diff/1/src/objects.cc#newcode3329
src/objects.cc:3329:
On 2014/04/08 12:58:55, Toon Verwaest wrote:
DisallowHeapAllocation?
Done.
https://codereview.chromium.org/228333003/diff/1/src/objects.cc#newcode4711
src/objects.cc:4711: object->set_map(*map);
On 2014/04/08 12:58:55, Toon Verwaest wrote:
JSObject::SetMapAndElements
But initialize_elements() contains detailed logic to determine the
elements given a map. Why should I use SetMapAndElements()?
https://codereview.chromium.org/228333003/diff/1/src/objects.cc#newcode6793
src/objects.cc:6793: new_descriptors->Append(descriptor);
On 2014/04/08 12:58:55, Toon Verwaest wrote:
We cannot GC anymore after this has happened. Otherwise it may be that
the newly
appended descriptor isn't properly marked as it is weak until
result->InitializeDescriptors(*new_descriptors);
Okay, I re-ordered things for the desired effect.
https://codereview.chromium.org/228333003/diff/1/src/objects.cc#newcode6818
src/objects.cc:6818: Map* walk_map;
On 2014/04/08 12:58:55, Toon Verwaest wrote:
DisallowHeapAllocation no_gc from here down.
I've got it above now, I think that is fine?
https://codereview.chromium.org/228333003/diff/1/src/objects.cc#newcode6856
src/objects.cc:6856: MaybeObject*
Map::CopyReplaceDescriptorsFull(DescriptorArray* descriptors,
On 2014/04/08 12:58:55, Toon Verwaest wrote:
This name doesn't really make sense.
Already gone.
https://codereview.chromium.org/228333003/diff/1/src/objects.cc#newcode6882
src/objects.cc:6882: Handle<Map>
Map::CopyReplaceDescriptorsFull(Handle<Map> map,
On 2014/04/08 12:58:55, Toon Verwaest wrote:
Can we get rid of this version of the method? Kinda ugly to have both
versions;
and this name doesn't really make sense.
Already gone.
https://codereview.chromium.org/228333003/diff/1/src/objects.cc#newcode7198
src/objects.cc:7198: new_descriptors->Set(i, descriptor, witness);
On 2014/04/08 12:58:55, Toon Verwaest wrote:
In this Set case, the "pointer" will be wrong.
Can we just use CopyUpTo rather than this local copy; and afterwards
do a "set"
which guarantees that the "pointer" value is maintained?
I followed your suggestion to just add
descriptor->SetSortedKeyIndex(descriptors->GetSortedKeyIndex(i));
before the Set() call.
https://codereview.chromium.org/228333003/diff/1/src/transitions.cc
File src/transitions.cc (right):
https://codereview.chromium.org/228333003/diff/1/src/transitions.cc#newcode209
src/transitions.cc:209: for (int i = 0; i < number_of_transitions; ++i)
{
On 2014/04/08 12:58:55, Toon Verwaest wrote:
Wrong. number_of_transitions can change because of
TransitionArray::AllocateHandle; since it's a weak array.
Done.
Description:
Handlefy Descriptor and other code in objects.cc
Please review this at https://codereview.chromium.org/228333003/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+480, -535 lines):
M src/bootstrapper.cc
M src/elements.cc
M src/factory.h
M src/factory.cc
M src/objects.h
M src/objects.cc
M src/objects-inl.h
M src/property.h
M src/runtime.cc
M src/transitions.h
M src/transitions.cc
M test/cctest/test-alloc.cc
--
--
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.
