On 2014/03/10 14:15:34, Toon Verwaest wrote:
I added some more superficial comments inline.
My main comment is though: are typed elements kinds at all related to
normal
elements transitions?
It seems rather weird to mix up the "IsMoreGeneralElementsTransition"
related
code (that puts them in sequence etc) with typed arrays. Typed arrays, as
far
as
I know, can't transition elements kind.
But they do transition elements kind. UINT8_ELEMENTS transitions to
EXTERNAL_UINT8_ELEMENTS as soon as you call .buffer on it.
Let's chat offline.
So they don't really belong in sequence
as we don't transition between them. Your changes makes that code more
complex
(and slower) without providing any benefit to that part (If I'm not
mistaken...).
We may want to have dedicated transitions for them though, to ensure that
we
can
store them all. Maybe we should have internal symbols for each specific
transition so we can add all of them to a same root map; rather than
putting
them in sequence?
https://codereview.chromium.org/150813004/diff/200001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/150813004/diff/200001/src/objects.cc#newcode3260
src/objects.cc:3260: IsFastElementsKind(to_kind) ||
IsExternalArrayElementsKind(to_kind)
IsTransitionElementsKind(to_kind)?
https://codereview.chromium.org/150813004/diff/200001/src/objects.cc#newcode3277
src/objects.cc:3277: if (to_kind != kind &&
current_map->HasElementsTransition()) {
Can this only happen for DICTIONARY elements now? If so, can we ASSERT?
https://codereview.chromium.org/150813004/diff/200001/src/objects.cc#newcode3282
src/objects.cc:3282: ASSERT(current_map->elements_kind() == target_kind);
This ASSERT is wrong afaict. If we didnt' have an elements transition
above,
and
the target kind is DICTIONARY_ELEMENTS, the closest transition will still
be
whatever fast kind came right before that.
https://codereview.chromium.org/150813004/diff/200001/src/objects.cc#newcode3324
src/objects.cc:3324: if (kind != to_kind) {
Can this only happen for DICTIONARY elements now? If so, can we ASSERT?
https://codereview.chromium.org/150813004/
--
--
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.
