Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(603)

Unified Diff: src/value-serializer.cc

Issue 2259633002: Blink-compatible serialization of arrays, both dense and sparse. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Micro-tweak: let NewJSArray construct the elements. Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: src/value-serializer.cc
diff --git a/src/value-serializer.cc b/src/value-serializer.cc
index ad7492a2a60f32c03ca30b4808fb8954c3f95fb1..6061da58f065940fc89fc43d0e6cf26b4c916a18 100644
--- a/src/value-serializer.cc
+++ b/src/value-serializer.cc
@@ -60,6 +60,16 @@ enum class SerializationTag : uint8_t {
kBeginJSObject = 'o',
// End of a JS object. numProperties:uint32_t
kEndJSObject = '{',
+ // Beginning of a sparse JS array. length:uint32_t
+ // Elements and properties are written as key/value pairs, like objects.
+ kBeginSparseJSArray = 'a',
+ // End of a sparse JS array. numProperties:uint32_t length:uint32_t
+ kEndSparseJSArray = '@',
+ // Beginning of a dense JS array. length:uint32_t
+ // |length| elements, followed by properties as key/value pairs
+ kBeginDenseJSArray = 'A',
+ // End of a dense JS array. numProperties:uint32_t length:uint32_t
+ kEndDenseJSArray = '$',
};
ValueSerializer::ValueSerializer(Isolate* isolate)
@@ -253,6 +263,8 @@ Maybe<bool> ValueSerializer::WriteJSReceiver(Handle<JSReceiver> receiver) {
HandleScope scope(isolate_);
switch (instance_type) {
+ case JS_ARRAY_TYPE:
+ return WriteJSArray(Handle<JSArray>::cast(receiver));
case JS_OBJECT_TYPE:
case JS_API_OBJECT_TYPE:
return WriteJSObject(Handle<JSObject>::cast(receiver));
@@ -278,6 +290,65 @@ Maybe<bool> ValueSerializer::WriteJSObject(Handle<JSObject> object) {
return Just(true);
}
+Maybe<bool> ValueSerializer::WriteJSArray(Handle<JSArray> array) {
+ uint32_t length;
+ array->length()->ToArrayLength(&length);
+
+ // This currently does not use dense serialization for holey elements, since
+ // the format currently does not provide for properly distinguishing between
Jakob Kummerow 2016/08/18 13:49:56 The deserializer implementation below treats every
jbroman 2016/08/18 15:03:03 That's fair enough. I'd originally had the deseria
Jakob Kummerow 2016/08/18 17:42:28 Yes, I know. I meant my review comment as conside
jbroman 2016/08/18 18:10:11 Reworded the comment to that effect, and expanded
+ // undefined and the hole. Additionally, if holey elements were used, it's
+ // possible that an element which began non-enumerable (by being a hole)
+ // became enumerable (by being assigned to).
+ const bool should_serialize_densely =
+ array->HasFastElements() && !array->HasFastHoleyElements();
Jakob Kummerow 2016/08/18 13:49:56 Given that the array could transition to slow or h
jbroman 2016/08/18 15:03:04 A more principled heuristic would be used_elements
Jakob Kummerow 2016/08/18 17:42:28 Yes, something like that. Doing that later (if at
jbroman 2016/08/18 18:10:11 I've reworded the comment to (hopefully) be cleare
+
+ if (should_serialize_densely) {
+ // TODO(jbroman): Take fuller advantage of fast elements.
+ // TODO(jbroman): Distinguish between undefined and a hole (this can happen
+ // if serializing one of the elements deletes another). This requires wire
+ // format changes.
+ WriteTag(SerializationTag::kBeginDenseJSArray);
+ WriteVarint<uint32_t>(length);
+ for (uint32_t i = 0; i < length; i++) {
+ Handle<Object> element;
+ LookupIterator it(isolate_, array, i, array, LookupIterator::OWN);
Jakob Kummerow 2016/08/18 13:49:56 Please add a comment: // Recursively serializing t
jbroman 2016/08/18 15:03:04 Done. That's why I left taking advantage of the el
Jakob Kummerow 2016/08/18 17:42:28 Yes.
+ if (!Object::GetProperty(&it).ToHandle(&element) ||
+ !WriteObject(element).FromMaybe(false)) {
+ return Nothing<bool>();
+ }
+ }
+ 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)) {
+ return Nothing<bool>();
+ }
+ WriteTag(SerializationTag::kEndDenseJSArray);
+ WriteVarint<uint32_t>(properties_written);
+ WriteVarint<uint32_t>(length);
Jakob Kummerow 2016/08/18 13:49:56 Any particular reason why |length| is written twic
jbroman 2016/08/18 15:03:03 Compatibility with the existing format. Some of th
+ } 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)) {
+ return Nothing<bool>();
+ }
+ WriteTag(SerializationTag::kEndSparseJSArray);
+ WriteVarint<uint32_t>(properties_written);
+ WriteVarint<uint32_t>(length);
+ }
+ return Just(true);
+}
+
Maybe<uint32_t> ValueSerializer::WriteJSObjectProperties(
Handle<JSObject> object, Handle<FixedArray> keys) {
uint32_t properties_written = 0;
@@ -454,6 +525,10 @@ MaybeHandle<Object> ValueDeserializer::ReadObject() {
}
case SerializationTag::kBeginJSObject:
return ReadJSObject();
+ case SerializationTag::kBeginSparseJSArray:
+ return ReadSparseJSArray();
+ case SerializationTag::kBeginDenseJSArray:
+ return ReadDenseJSArray();
default:
return MaybeHandle<Object>();
}
@@ -517,6 +592,71 @@ MaybeHandle<JSObject> ValueDeserializer::ReadJSObject() {
return scope.CloseAndEscape(object);
}
+MaybeHandle<JSArray> ValueDeserializer::ReadSparseJSArray() {
+ // If we are at the end of the stack, abort. This function may recurse.
+ if (StackLimitCheck(isolate_).HasOverflowed()) return MaybeHandle<JSArray>();
+
+ uint32_t length;
+ if (!ReadVarint<uint32_t>().To(&length)) return MaybeHandle<JSArray>();
+
+ uint32_t id = next_id_++;
+ HandleScope scope(isolate_);
+ Handle<JSArray> array = isolate_->factory()->NewJSArray(0);
+ JSArray::SetLength(array, length);
+ AddObjectWithID(id, array);
+
+ uint32_t num_properties;
+ uint32_t expected_num_properties;
+ uint32_t expected_length;
+ if (!ReadJSObjectProperties(array, SerializationTag::kEndSparseJSArray)
+ .To(&num_properties) ||
+ !ReadVarint<uint32_t>().To(&expected_num_properties) ||
+ !ReadVarint<uint32_t>().To(&expected_length) ||
+ num_properties != expected_num_properties || length != expected_length) {
+ return MaybeHandle<JSArray>();
+ }
+
+ DCHECK(HasObjectWithID(id));
+ return scope.CloseAndEscape(array);
+}
+
+MaybeHandle<JSArray> ValueDeserializer::ReadDenseJSArray() {
+ // If we are at the end of the stack, abort. This function may recurse.
+ if (StackLimitCheck(isolate_).HasOverflowed()) return MaybeHandle<JSArray>();
+
+ uint32_t length;
+ if (!ReadVarint<uint32_t>().To(&length)) return MaybeHandle<JSArray>();
+
+ uint32_t id = next_id_++;
+ HandleScope scope(isolate_);
+ Handle<JSArray> array = isolate_->factory()->NewJSArray(
+ FAST_HOLEY_ELEMENTS, length, length, INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE);
Jakob Kummerow 2016/08/18 13:49:56 It's a bit sad that deserialized arrays will alway
jbroman 2016/08/18 15:03:03 Probably logic to use FAST_ELEMENTS if there are n
+ AddObjectWithID(id, array);
+
+ Handle<FixedArray> elements(FixedArray::cast(array->elements()), isolate_);
+ for (uint32_t i = 0; i < length; i++) {
+ Handle<Object> element;
+ if (!ReadObject().ToHandle(&element)) return MaybeHandle<JSArray>();
+ // TODO(jbroman): Distinguish between undefined and a hole.
+ if (element->IsUndefined(isolate_)) continue;
Jakob Kummerow 2016/08/18 13:49:56 This is inconsistent with the serializer. Are you
jbroman 2016/08/18 15:03:03 In fact I don't want to, but the existing wire for
Jakob Kummerow 2016/08/18 17:42:28 I'm aware of the wire format's limitation. I was j
jbroman 2016/08/18 18:10:11 It's unfortunate but intentional, yes.
+ elements->set(i, *element);
+ }
+
+ uint32_t num_properties;
+ uint32_t expected_num_properties;
+ uint32_t expected_length;
+ if (!ReadJSObjectProperties(array, SerializationTag::kEndDenseJSArray)
+ .To(&num_properties) ||
+ !ReadVarint<uint32_t>().To(&expected_num_properties) ||
+ !ReadVarint<uint32_t>().To(&expected_length) ||
+ num_properties != expected_num_properties || length != expected_length) {
+ return MaybeHandle<JSArray>();
+ }
+
+ DCHECK(HasObjectWithID(id));
+ return scope.CloseAndEscape(array);
+}
+
Maybe<uint32_t> ValueDeserializer::ReadJSObjectProperties(
Handle<JSObject> object, SerializationTag end_tag) {
for (uint32_t num_properties = 0;; num_properties++) {

Powered by Google App Engine
This is Rietveld 408576698