From 36985859e421a9fbbdfc56379506b2a367c7d4e9 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Fri, 30 Aug 2019 17:49:28 +0000 Subject: [PATCH] [vm] Don't copy when reading typed data in messages sent to native ports. Bug: https://github.com/dart-lang/sdk/issues/37833 Change-Id: Iac906e58320e003365c4c0624b4587579f352db1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/114970 Reviewed-by: Alexander Aprelev Commit-Queue: Ryan Macnak --- runtime/vm/dart_api_message.cc | 55 ++++++++++++++++++++----------- runtime/vm/datastream.h | 2 +- runtime/vm/finalizable_data.h | 23 +++++++++---- runtime/vm/raw_object_snapshot.cc | 2 ++ runtime/vm/snapshot.h | 3 ++ runtime/vm/zone.h | 6 ++-- 6 files changed, 62 insertions(+), 29 deletions(-) diff --git a/runtime/vm/dart_api_message.cc b/runtime/vm/dart_api_message.cc index ee4aa1da5ee..e598ded5834 100644 --- a/runtime/vm/dart_api_message.cc +++ b/runtime/vm/dart_api_message.cc @@ -2,6 +2,8 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +#include + #include "vm/dart_api_message.h" #include "platform/unicode.h" #include "vm/object.h" @@ -557,29 +559,43 @@ Dart_CObject* ApiMessageReader::ReadInternalVMObject(intptr_t class_id, return object; } -#define READ_TYPED_DATA_HEADER(type) \ - intptr_t len = ReadSmiValue(); \ - Dart_CObject* object = \ - AllocateDartCObjectTypedData(Dart_TypedData_k##type, len); \ - AddBackRef(object_id, object, kIsDeserialized); - -#define READ_TYPED_DATA(type, ctype) \ +#define READ_TYPED_DATA(tname, ctype) \ { \ - READ_TYPED_DATA_HEADER(type); \ - uint8_t* p = \ - reinterpret_cast(object->value.as_typed_data.values); \ - ReadBytes(p, len * sizeof(ctype)); \ + intptr_t len = ReadSmiValue(); \ + auto type = Dart_TypedData_k##tname; \ + intptr_t length_in_bytes = GetTypedDataSizeInBytes(type) * len; \ + Dart_CObject* object = \ + reinterpret_cast(allocator(sizeof(Dart_CObject))); \ + ASSERT(object != NULL); \ + object->type = Dart_CObject_kTypedData; \ + object->value.as_typed_data.type = type; \ + object->value.as_typed_data.length = length_in_bytes; \ + if (len > 0) { \ + Align(Zone::kAlignment); \ + object->value.as_typed_data.values = \ + const_cast(CurrentBufferAddress()); \ + Advance(length_in_bytes); \ + } else { \ + object->value.as_typed_data.values = NULL; \ + } \ + AddBackRef(object_id, object, kIsDeserialized); \ return object; \ } -#define READ_EXTERNAL_TYPED_DATA(type, ctype) \ +#define READ_EXTERNAL_TYPED_DATA(tname, ctype) \ { \ - READ_TYPED_DATA_HEADER(type); \ - uint8_t* p = \ - reinterpret_cast(object->value.as_typed_data.values); \ - FinalizableData finalizable_data = finalizable_data_->Take(); \ - memmove(p, finalizable_data.data, len * sizeof(ctype)); \ - finalizable_data.callback(NULL, NULL, finalizable_data.peer); \ + intptr_t len = ReadSmiValue(); \ + auto type = Dart_TypedData_k##tname; \ + intptr_t length_in_bytes = GetTypedDataSizeInBytes(type) * len; \ + Dart_CObject* object = \ + reinterpret_cast(allocator(sizeof(Dart_CObject))); \ + ASSERT(object != NULL); \ + object->type = Dart_CObject_kTypedData; \ + object->value.as_typed_data.type = type; \ + object->value.as_typed_data.length = length_in_bytes; \ + object->value.as_typed_data.values = \ + reinterpret_cast(finalizable_data_->Get().data); \ + AddBackRef(object_id, object, kIsDeserialized); \ return object; \ } @@ -635,7 +651,6 @@ Dart_CObject* ApiMessageReader::ReadInternalVMObject(intptr_t class_id, #undef READ_TYPED_DATA #undef READ_EXTERNAL_TYPED_DATA #undef READ_TYPED_DATA_VIEW -#undef READ_TYPED_DATA_HEADER case kGrowableObjectArrayCid: { // A GrowableObjectArray is serialized as its type arguments and @@ -1069,11 +1084,13 @@ bool ApiMessageWriter::WriteCObjectInlined(Dart_CObject* object, case kTypedDataInt8ArrayCid: case kTypedDataUint8ArrayCid: { uint8_t* bytes = object->value.as_typed_data.values; + Align(Zone::kAlignment); WriteBytes(bytes, len); break; } case kTypedDataUint32ArrayCid: { uint8_t* bytes = object->value.as_typed_data.values; + Align(Zone::kAlignment); WriteBytes(bytes, len * sizeof(uint32_t)); break; } diff --git a/runtime/vm/datastream.h b/runtime/vm/datastream.h index 13bd393cc75..da4c450145c 100644 --- a/runtime/vm/datastream.h +++ b/runtime/vm/datastream.h @@ -89,7 +89,7 @@ class ReadStream : public ValueObject { const uint8_t* AddressOfCurrentPosition() const { return current_; } void Advance(intptr_t value) { - ASSERT((end_ - current_) > value); + ASSERT((end_ - current_) >= value); current_ = current_ + value; } diff --git a/runtime/vm/finalizable_data.h b/runtime/vm/finalizable_data.h index df383d20dc7..acea96cf459 100644 --- a/runtime/vm/finalizable_data.h +++ b/runtime/vm/finalizable_data.h @@ -20,10 +20,11 @@ struct FinalizableData { class MessageFinalizableData { public: - MessageFinalizableData() : records_(0), position_(0), external_size_(0) {} + MessageFinalizableData() + : records_(0), get_position_(0), take_position_(0), external_size_(0) {} ~MessageFinalizableData() { - for (intptr_t i = position_; i < records_.length(); i++) { + for (intptr_t i = take_position_; i < records_.length(); i++) { records_[i].callback(nullptr, nullptr, records_[i].peer); } } @@ -46,13 +47,22 @@ class MessageFinalizableData { external_size_ += external_size; } + // Retrieve the next FinalizableData, but still run its finalizer when |this| + // is destroyed. + FinalizableData Get() { + ASSERT(get_position_ < records_.length()); + return records_[get_position_++]; + } + + // Retrieve the next FinalizableData, and skip its finalizer when |this| is + // destroyed. FinalizableData Take() { - ASSERT(position_ < records_.length()); - return records_[position_++]; + ASSERT(take_position_ < records_.length()); + return records_[take_position_++]; } void SerializationSucceeded() { - for (intptr_t i = position_; i < records_.length(); i++) { + for (intptr_t i = 0; i < records_.length(); i++) { if (records_[i].successful_write_callback != nullptr) { records_[i].successful_write_callback(nullptr, nullptr, records_[i].peer); @@ -64,7 +74,8 @@ class MessageFinalizableData { private: MallocGrowableArray records_; - intptr_t position_; + intptr_t get_position_; + intptr_t take_position_; intptr_t external_size_; DISALLOW_COPY_AND_ASSIGN(MessageFinalizableData); diff --git a/runtime/vm/raw_object_snapshot.cc b/runtime/vm/raw_object_snapshot.cc index 22a70169685..67731410f95 100644 --- a/runtime/vm/raw_object_snapshot.cc +++ b/runtime/vm/raw_object_snapshot.cc @@ -1311,6 +1311,7 @@ RawTypedData* TypedData::ReadFrom(SnapshotReader* reader, intptr_t length_in_bytes = len * element_size; NoSafepointScope no_safepoint; uint8_t* data = reinterpret_cast(result.DataAddr(0)); + reader->Align(Zone::kAlignment); reader->ReadBytes(data, length_in_bytes); // If it is a canonical constant make it one. @@ -1456,6 +1457,7 @@ void RawTypedData::WriteTo(SnapshotWriter* writer, writer->WriteTags(writer->GetObjectTags(this)); writer->Write(ptr()->length_); uint8_t* data = reinterpret_cast(ptr()->data()); + writer->Align(Zone::kAlignment); writer->WriteBytes(data, bytes); } } diff --git a/runtime/vm/snapshot.h b/runtime/vm/snapshot.h index 69d9493caf0..1f7beaa843f 100644 --- a/runtime/vm/snapshot.h +++ b/runtime/vm/snapshot.h @@ -255,6 +255,7 @@ class BaseReader { return stream_.AddressOfCurrentPosition(); } + void Align(intptr_t value) { stream_.Align(value); } void Advance(intptr_t value) { stream_.Advance(value); } intptr_t PendingBytes() const { return stream_.PendingBytes(); } @@ -517,6 +518,8 @@ class BaseWriter : public StackResource { Write(static_cast(flags)); } + void Align(intptr_t value) { stream_.Align(value); } + // Write out a buffer of bytes. void WriteBytes(const uint8_t* addr, intptr_t len) { stream_.WriteBytes(addr, len); diff --git a/runtime/vm/zone.h b/runtime/vm/zone.h index 88e1ef99d7f..fff19b3e003 100644 --- a/runtime/vm/zone.h +++ b/runtime/vm/zone.h @@ -82,13 +82,13 @@ class Zone { return false; } + // All pointers returned from AllocateUnsafe() and New() have this alignment. + static const intptr_t kAlignment = kDoubleSize; + private: Zone(); ~Zone(); // Delete all memory associated with the zone. - // All pointers returned from AllocateUnsafe() and New() have this alignment. - static const intptr_t kAlignment = kDoubleSize; - // Default initial chunk size. static const intptr_t kInitialChunkSize = 1 * KB;