Take advantage of fast properties in ValueSerializer when JSObject has them.

src/value-serializer.cc

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/value-serializer.h"
 
 #include <type_traits>
 
 #include "src/base/logging.h"
 #include "src/conversions.h"
@@ skipping 357 matching lines @@
     case JS_DATA_VIEW_TYPE:
       return WriteJSArrayBufferView(JSArrayBufferView::cast(*receiver));
     default:
       UNIMPLEMENTED();
       break;
   }
   return Nothing<bool>();
 }
 
 Maybe<bool> ValueSerializer::WriteJSObject(Handle<JSObject> object) {
+  DCHECK(object->map()->instance_type() > LAST_CUSTOM_ELEMENTS_RECEIVER);

[Camillo Bruni, 2016/09/01 20:50:32]

nit: Use DCHECK_GT(left..., right..)

[jbroman, 2016/09/02 15:31:42]

Done.

+  const bool can_serialize_fast =
+      object->HasFastProperties() && object->elements()->length() == 0;

[Camillo Bruni, 2016/09/01 20:50:32]

I guess this is WIP, elements() can be easily serialized if they have no
accessors (there's object->GetElementsAccessor()->HasAccessors())
And you may want to offload serialization of elements to the ElementsAccessor
(in elements.cc) as well.

[Jakob Kummerow, 2016/09/02 10:44:23]

It's not that easy. Any element that's an object itself could in turn have
getters with arbitrary side effects. Skipping non-empty elements here is
probably necessary; it would be rather involved to get the behavior correct and
safe otherwise.

[jbroman, 2016/09/02 15:31:42]

+1. And this matches what json-stringifier does (which seems reasonable; I'd
expect that most objects have no own elements and most arrays have no own
properties).

+  if (!can_serialize_fast) return WriteJSObjectSlow(object);
+
+  DCHECK(!object->IsJSGlobalProxy());
+  DCHECK(!object->HasIndexedInterceptor());
+  DCHECK(!object->HasNamedInterceptor());
+  Handle<Map> map(object->map(), isolate_);
+  WriteTag(SerializationTag::kBeginJSObject);
+
+  // Write out fast properties as long as they are only data properties and the
+  // map doesn't change.
+  uint32_t properties_written = 0;
+  for (int i = 0; i < map->NumberOfOwnDescriptors(); i++) {
+    Handle<Name> key(map->instance_descriptors()->GetKey(i), isolate_);
+    if (!key->IsString()) continue;
+    PropertyDetails details = map->instance_descriptors()->GetDetails(i);
+    if (details.IsDontEnum()) continue;
+
+    Handle<Object> value;
+    if (V8_LIKELY(details.type() == DATA && *map == object->map())) {
+      FieldIndex field_index = FieldIndex::ForDescriptor(*map, i);
+      value = JSObject::FastPropertyAt(object, details.representation(),
+                                       field_index);
+    } else {
+      // This logic should essentially match WriteJSObjectPropertiesSlow.
+      bool success;
+      LookupIterator it = LookupIterator::PropertyOrElement(
+          isolate_, object, key, &success, LookupIterator::OWN);

[Camillo Bruni, 2016/09/01 20:50:32]

You can bypass the arrayIndex check in PropertyOrElement and the superfluous
ToName conversion (you can only have internalized strings in the descriptor
array) by directly instantiating the LookupIterator:

LookupIterator(object, key, LookupIterator::OWN)

If you feel courageous maybe add a new constructor to the LookupIterator which
takes the isolate as first argument (currently that only exists for element
lookups).

[jbroman, 2016/09/02 15:31:42]

Done.

+      DCHECK(success);
+      if (!Object::GetProperty(&it).ToHandle(&value)) return Nothing<bool>();
+
+      // If the property is no longer found, do not serialize it.
+      // This could happen if a getter deleted the property.
+      if (!it.IsFound()) continue;

[Camillo Bruni, 2016/09/01 20:50:32]

You can push the IsFound() check above the GetProperty call.

[jbroman, 2016/09/02 15:31:42]

Done.

+    }
+
+    if (!WriteObject(key).FromMaybe(false) ||
+        !WriteObject(value).FromMaybe(false)) {
+      return Nothing<bool>();
+    }
+    properties_written++;
+  }
+
+  WriteTag(SerializationTag::kEndJSObject);
+  WriteVarint<uint32_t>(properties_written);
+  return Just(true);
+}
+
+Maybe<bool> ValueSerializer::WriteJSObjectSlow(Handle<JSObject> object) {
   WriteTag(SerializationTag::kBeginJSObject);
   Handle<FixedArray> keys;
   uint32_t properties_written;
   if (!KeyAccumulator::GetKeys(object, KeyCollectionMode::kOwnOnly,
                                ENUMERABLE_STRINGS)
            .ToHandle(&keys) ||
-      !WriteJSObjectProperties(object, keys).To(&properties_written)) {
+      !WriteJSObjectPropertiesSlow(object, keys).To(&properties_written)) {
     return Nothing<bool>();
   }
   WriteTag(SerializationTag::kEndJSObject);
   WriteVarint<uint32_t>(properties_written);
   return Just(true);
 }
 
 Maybe<bool> ValueSerializer::WriteJSArray(Handle<JSArray> array) {
   uint32_t length = 0;
   bool valid_length = array->length()->ToArrayLength(&length);
@@ skipping 26 matching lines @@
       }
     }
     KeyAccumulator accumulator(isolate_, KeyCollectionMode::kOwnOnly,
                                ENUMERABLE_STRINGS);
     if (!accumulator.CollectOwnPropertyNames(array, array).FromMaybe(false)) {
       return Nothing<bool>();
     }
     Handle<FixedArray> keys =
         accumulator.GetKeys(GetKeysConversion::kConvertToString);
     uint32_t properties_written;
-    if (!WriteJSObjectProperties(array, keys).To(&properties_written)) {
+    if (!WriteJSObjectPropertiesSlow(array, keys).To(&properties_written)) {
       return Nothing<bool>();
     }
     WriteTag(SerializationTag::kEndDenseJSArray);
     WriteVarint<uint32_t>(properties_written);
     WriteVarint<uint32_t>(length);
   } else {
     WriteTag(SerializationTag::kBeginSparseJSArray);
     WriteVarint<uint32_t>(length);
     Handle<FixedArray> keys;
     uint32_t properties_written;
     if (!KeyAccumulator::GetKeys(array, KeyCollectionMode::kOwnOnly,
                                  ENUMERABLE_STRINGS)
              .ToHandle(&keys) ||
-        !WriteJSObjectProperties(array, keys).To(&properties_written)) {
+        !WriteJSObjectPropertiesSlow(array, keys).To(&properties_written)) {
       return Nothing<bool>();
     }
     WriteTag(SerializationTag::kEndSparseJSArray);
     WriteVarint<uint32_t>(properties_written);
     WriteVarint<uint32_t>(length);
   }
   return Just(true);
 }
 
 void ValueSerializer::WriteJSDate(JSDate* date) {
@@ skipping 138 matching lines @@
   } else {
     DCHECK(view->IsJSDataView());
     tag = ArrayBufferViewTag::kDataView;
   }
   WriteVarint(static_cast<uint8_t>(tag));
   WriteVarint(NumberToUint32(view->byte_offset()));
   WriteVarint(NumberToUint32(view->byte_length()));
   return Just(true);
 }
 
-Maybe<uint32_t> ValueSerializer::WriteJSObjectProperties(
+Maybe<uint32_t> ValueSerializer::WriteJSObjectPropertiesSlow(
     Handle<JSObject> object, Handle<FixedArray> keys) {
   uint32_t properties_written = 0;
   int length = keys->length();
   for (int i = 0; i < length; i++) {
     Handle<Object> key(keys->get(i), isolate_);
 
     bool success;
     LookupIterator it = LookupIterator::PropertyOrElement(
         isolate_, object, key, &success, LookupIterator::OWN);
     DCHECK(success);
@@ skipping 733 matching lines @@
   }
 #endif
   position_ = end_;
 
   if (stack.size() != 1) return MaybeHandle<Object>();
   return scope.CloseAndEscape(stack[0]);
 }
 
 }  // namespace internal
 }  // namespace v8