Getting there... could you please add relevant JSON tests and double
transition
tests of normal objects.
https://codereview.chromium.org/14850006/diff/42001/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/14850006/diff/42001/src/hydrogen-instructions.cc#newcode2525
src/hydrogen-instructions.cc:2525: if (map->is_deprecated()) continue;
Shouldn't be hitting this case? Maybe this should be an assert.
https://codereview.chromium.org/14850006/diff/42001/src/hydrogen-instructions.cc#newcode2538
src/hydrogen-instructions.cc:2538:
lookup.GetPropertyDetails().representation().IsDouble()) {
Do lookup.representation directly.
https://codereview.chromium.org/14850006/diff/42001/src/ia32/stub-cache-ia32.cc
File src/ia32/stub-cache-ia32.cc (right):
https://codereview.chromium.org/14850006/diff/42001/src/ia32/stub-cache-ia32.cc#newcode910
src/ia32/stub-cache-ia32.cc:910: __ mov(FieldOperand(receiver_reg,
offset), name_reg);
Give this name_reg guy another name?
https://codereview.chromium.org/14850006/diff/42001/src/ia32/stub-cache-ia32.cc#newcode1078
src/ia32/stub-cache-ia32.cc:1078: __ RecordWriteField(scratch1,
Remove the write barrier in the double case
https://codereview.chromium.org/14850006/diff/42001/src/json-parser.h
File src/json-parser.h (right):
https://codereview.chromium.org/14850006/diff/42001/src/json-parser.h#newcode432
src/json-parser.h:432: JSObject::TransitionToMap(json_object, map);
AllocateStorageForMap... better name?
https://codereview.chromium.org/14850006/diff/42001/src/objects-inl.h
File src/objects-inl.h (right):
https://codereview.chromium.org/14850006/diff/42001/src/objects-inl.h#newcode290
src/objects-inl.h:290: MaybeObject* Object::StorageFor(Heap* heap,
Representation representation) {
AllocateNewStorageFor
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode824
src/objects.cc:824: return heap->AllocateHeapNumber(value->Number());
If this happens too frequently (i.e. having to allocate the HeapNumber
after calling FastPropertyAt, then shouldn't it actually be inside
FastPropertyAt? And have a FastPropertyStoreAtRaw access the unboxed
value (it would be used less frequently).
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode1719
src/objects.cc:1719: Representation representation) {
Perhaps a comment that this is a transition case. How about using the
AllocateNewStorageFor method?
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode1721
src/objects.cc:1721: if (FLAG_track_double_fields &&
representation.IsDouble()) {
Don't you just have to write into the existing box? This doesn't look
like a transition case.
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode1800
src/objects.cc:1800: if (map()->unused_property_fields() == 0) {
Add a TODO to merge the second half of this method with
AddFastPropertyUsingMap (if possible). At least think about it.
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode2291
src/objects.cc:2291: heap->AllocateHeapNumber(Smi::cast(value)->value(),
TENURED);
AllocateNewStorageFor
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode3921
src/objects.cc:3921: result = lookup.holder()->AddFastPropertyUsingMap(
Can you please revert to self?
https://codereview.chromium.org/14850006/diff/42001/src/objects.cc#newcode5308
src/objects.cc:5308: MaybeObject* maybe_number =
heap->AllocateHeapNumber(value->Number());
NewStorageForValue
https://codereview.chromium.org/14850006/diff/42001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/14850006/diff/42001/src/parser.cc#newcode4022
src/parser.cc:4022: }
*COUGH* Any other better way to do this? *COUGH*
Boolean would be better.
https://codereview.chromium.org/14850006/diff/42001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/14850006/diff/42001/src/runtime.cc#newcode237
src/runtime.cc:237: Factory* factory = isolate->factory();
Don't need this change.
https://codereview.chromium.org/14850006/diff/42001/src/stub-cache.cc
File src/stub-cache.cc (right):
https://codereview.chromium.org/14850006/diff/42001/src/stub-cache.cc#newcode228
src/stub-cache.cc:228: field.translate(receiver),
Extraneous change?
https://codereview.chromium.org/14850006/diff/42001/src/stub-cache.cc#newcode1510
src/stub-cache.cc:1510: object, receiver(), holder, name, &miss);
Extraneous whitespace change
https://codereview.chromium.org/14850006/
--
--
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/groups/opt_out.
