Assign external sizes to external strings and to external typed data created by the VM.

runtime/vm/object.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // 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 "vm/object.h"
 
 #include "include/dart_api.h"
 #include "platform/assert.h"
 #include "vm/assembler.h"
 #include "vm/become.h"
@@ skipping 8295 matching lines @@
     // This should be caught before we reach here.
     FATAL1("Fatal error in TokenStream::New: invalid len %" Pd "\n", len);
   }
   uint8_t* data = reinterpret_cast<uint8_t*>(::malloc(len));
   ASSERT(data != NULL);
   Zone* zone = Thread::Current()->zone();
   const ExternalTypedData& stream = ExternalTypedData::Handle(
       zone,
       ExternalTypedData::New(kExternalTypedDataUint8ArrayCid,
                              data, len, Heap::kOld));
-  stream.AddFinalizer(data, DataFinalizer);
+  stream.AddFinalizer(data, DataFinalizer, len);
   const TokenStream& result = TokenStream::Handle(zone, TokenStream::New());
   result.SetStream(stream);
   return result.raw();
 }
 
 
 // CompressedTokenMap maps String and LiteralToken keys to Smi values.
 // It also supports lookup by TokenDescriptor.
 class CompressedTokenTraits {
  public:
@@ skipping 172 matching lines @@
   CompressedTokenStreamData data(token_objects, &map);
   Scanner scanner(source, private_key);
   scanner.ScanAll(&data);
   INC_STAT(thread, num_tokens_scanned, data.NumTokens());
 
   // Create and setup the token stream object.
   const ExternalTypedData& stream = ExternalTypedData::Handle(
       zone,
       ExternalTypedData::New(kExternalTypedDataUint8ArrayCid,
                              data.GetStream(), data.Length(), Heap::kOld));
-  stream.AddFinalizer(data.GetStream(), DataFinalizer);
+  intptr_t external_size = data.Length();
+  stream.AddFinalizer(data.GetStream(), DataFinalizer, external_size);

[siva, 2016/08/29 23:03:45]

In this case we are not allocating a new buffer but instead just pointing to the
stream in the snapshot, shouldn't external_size be 0 in this case.

[rmacnak, 2016/08/30 20:49:17]

DataFinalizer here does a free, so this can't be pointing into the snapshot.

This stream was allocated by CompressedTokenStreamData::realloc.

The one that points into the snapshot is from clustered_snapshot.cc

   const TokenStream& result = TokenStream::Handle(zone, New());
   result.SetPrivateKey(private_key);
   {
     NoSafepointScope no_safepoint;
     result.SetStream(stream);
     result.SetTokenObjects(token_objects);
   }
 
   token_objects_map = map.Release().raw();
   if (use_shared_tokens) {
@@ skipping 11910 matching lines @@
 
 void String::ToUTF8(uint8_t* utf8_array, intptr_t array_len) const {
   ASSERT(array_len >= Utf8::Length(*this));
   Utf8::Encode(*this, reinterpret_cast<char*>(utf8_array), array_len);
 }
 
 
 static FinalizablePersistentHandle* AddFinalizer(
     const Object& referent,
     void* peer,
-    Dart_WeakPersistentHandleFinalizer callback) {
+    Dart_WeakPersistentHandleFinalizer callback,
+    intptr_t external_size) {
   ASSERT((callback != NULL && peer != NULL) ||
          (callback == NULL && peer == NULL));
-  // TODO(19482): Make API consistent for external size of strings/typed data.
-  const intptr_t external_size = 0;
   return FinalizablePersistentHandle::New(Isolate::Current(),
                                           referent,
                                           peer,
                                           callback,
                                           external_size);
 }
 
 
 RawString* String::MakeExternal(void* array,
                                 intptr_t length,
                                 void* peer,
                                 Dart_PeerFinalizer cback) const {
   ASSERT(FLAG_support_externalizable_strings);
   String& result = String::Handle();
   void* external_data;
+  intptr_t external_size;
   Dart_WeakPersistentHandleFinalizer finalizer;
   {
     NoSafepointScope no_safepoint;
     ASSERT(array != NULL);
     intptr_t str_length = this->Length();
     ASSERT(length >= (str_length * this->CharSize()));
     intptr_t class_id = raw()->GetClassId();
 
     ASSERT(!InVMHeap());
     if (class_id == kOneByteStringCid) {
@@ skipping 23 matching lines @@
       } while (tags != old_tags);
       result = this->raw();
       const uint8_t* ext_array = reinterpret_cast<const uint8_t*>(array);
       ExternalStringData<uint8_t>* ext_data = new ExternalStringData<uint8_t>(
           ext_array, peer, cback);
       ASSERT(result.Length() == str_length);
       ASSERT(!result.HasHash() ||
              (result.Hash() == String::Hash(ext_array, str_length)));
       ExternalOneByteString::SetExternalData(result, ext_data);
       external_data = ext_data;
+      external_size = str_length;

[siva, 2016/08/29 23:03:45]

Shouldn't external_size just be 'length' as that is the size passed in from the
external call and it can potentially be larger than str_length.

[rmacnak, 2016/08/30 20:49:17]

Yes. Renamed external_size here and external_allocation_size in the
Dart_MakeStringExternal for consistency with Dart_NewWeakPersistentHandle

       finalizer = ExternalOneByteString::Finalize;
     } else {
       ASSERT(class_id == kTwoByteStringCid);
       intptr_t used_size = ExternalTwoByteString::InstanceSize();
       intptr_t original_size = TwoByteString::InstanceSize(str_length);
       ASSERT(original_size >= used_size);
 
       // Copy the data into the external array.
       if (str_length > 0) {
         memmove(array,
@@ skipping 18 matching lines @@
       } while (tags != old_tags);
       result = this->raw();
       const uint16_t* ext_array = reinterpret_cast<const uint16_t*>(array);
       ExternalStringData<uint16_t>* ext_data = new ExternalStringData<uint16_t>(
           ext_array, peer, cback);
       ASSERT(result.Length() == str_length);
       ASSERT(!result.HasHash() ||
              (result.Hash() == String::Hash(ext_array, str_length)));
       ExternalTwoByteString::SetExternalData(result, ext_data);
       external_data = ext_data;
+      external_size = str_length * 2;
       finalizer = ExternalTwoByteString::Finalize;
     }
   }  // NoSafepointScope
-  AddFinalizer(result, external_data, finalizer);
+  AddFinalizer(result, external_data, finalizer, external_size);
   return this->raw();
 }
 
 
 RawString* String::Transform(int32_t (*mapping)(int32_t ch),
                              const String& str,
                              Heap::Space space) {
   ASSERT(!str.IsNull());
   bool has_mapping = false;
   int32_t dst_max = 0;
@@ skipping 434 matching lines @@
 }
 
 
 void OneByteString::SetPeer(const String& str,
                             void* peer,
                             Dart_PeerFinalizer cback) {
   ASSERT(!str.IsNull() && str.IsOneByteString());
   ASSERT(peer != NULL);
   ExternalStringData<uint8_t>* ext_data =
       new ExternalStringData<uint8_t>(NULL, peer, cback);
-  AddFinalizer(str, ext_data, OneByteString::Finalize);
+  intptr_t external_size = str.Length();

[siva, 2016/08/29 23:03:45]

For the same reason I am wondering if OneByteString::SetPeer and
TwoByteString::SetPeer should have a length argument that we pass in from the
API call implementation.

[rmacnak, 2016/08/30 20:49:17]

Pushed through from Dart_MakeExternalString.

+  AddFinalizer(str, ext_data, OneByteString::Finalize, external_size);
   Isolate::Current()->heap()->SetPeer(str.raw(), peer);
 }
 
 
 void OneByteString::Finalize(void* isolate_callback_data,
                              Dart_WeakPersistentHandle handle,
                              void* peer) {
   delete reinterpret_cast<ExternalStringData<uint8_t>*>(peer);
 }
 
@@ skipping 190 matching lines @@
 }
 
 
 void TwoByteString::SetPeer(const String& str,
                             void* peer,
                             Dart_PeerFinalizer cback) {
   ASSERT(!str.IsNull() && str.IsTwoByteString());
   ASSERT(peer != NULL);
   ExternalStringData<uint16_t>* ext_data =
       new ExternalStringData<uint16_t>(NULL, peer, cback);
-  AddFinalizer(str, ext_data, TwoByteString::Finalize);
+  intptr_t external_size = str.Length() * 2;
+  AddFinalizer(str, ext_data, TwoByteString::Finalize, external_size);
   Isolate::Current()->heap()->SetPeer(str.raw(), peer);
 }
 
 
 void TwoByteString::Finalize(void* isolate_callback_data,
                              Dart_WeakPersistentHandle handle,
                              void* peer) {
   delete reinterpret_cast<ExternalStringData<uint16_t>*>(peer);
 }
 
@@ skipping 17 matching lines @@
   {
     RawObject* raw = Object::Allocate(ExternalOneByteString::kClassId,
                                       ExternalOneByteString::InstanceSize(),
                                       space);
     NoSafepointScope no_safepoint;
     result ^= raw;
     result.SetLength(len);
     result.SetHash(0);
     SetExternalData(result, external_data);
   }
-  AddFinalizer(result, external_data, ExternalOneByteString::Finalize);
+  intptr_t external_size = len;
+  AddFinalizer(result, external_data, ExternalOneByteString::Finalize,
+               external_size);
   return ExternalOneByteString::raw(result);
 }
 
 
 void ExternalOneByteString::Finalize(void* isolate_callback_data,
                                      Dart_WeakPersistentHandle handle,
                                      void* peer) {
   delete reinterpret_cast<ExternalStringData<uint8_t>*>(peer);
 }
 
@@ skipping 17 matching lines @@
   {
     RawObject* raw = Object::Allocate(ExternalTwoByteString::kClassId,
                                       ExternalTwoByteString::InstanceSize(),
                                       space);
     NoSafepointScope no_safepoint;
     result ^= raw;
     result.SetLength(len);
     result.SetHash(0);
     SetExternalData(result, external_data);
   }
-  AddFinalizer(result, external_data, ExternalTwoByteString::Finalize);
+  intptr_t external_size = len * 2;
+  AddFinalizer(result, external_data, ExternalTwoByteString::Finalize,
+               external_size);
   return ExternalTwoByteString::raw(result);
 }
 
 
 void ExternalTwoByteString::Finalize(void* isolate_callback_data,
                                      Dart_WeakPersistentHandle handle,
                                      void* peer) {
   delete reinterpret_cast<ExternalStringData<uint16_t>*>(peer);
 }
 
@@ skipping 773 matching lines @@
 #define CASE_TYPED_DATA_CLASS(clazz)                                           \
   case kTypedData##clazz##Cid: return #clazz;
   CLASS_LIST_TYPED_DATA(CASE_TYPED_DATA_CLASS);
 #undef CASE_TYPED_DATA_CLASS
   }
   return "TypedData";
 }
 
 
 FinalizablePersistentHandle* ExternalTypedData::AddFinalizer(
-    void* peer, Dart_WeakPersistentHandleFinalizer callback) const {
-  return dart::AddFinalizer(*this, peer, callback);
+    void* peer,
+    Dart_WeakPersistentHandleFinalizer callback,
+    intptr_t external_size) const {
+  return dart::AddFinalizer(*this, peer, callback, external_size);
 }
 
 
 RawExternalTypedData* ExternalTypedData::New(intptr_t class_id,
                                              uint8_t* data,
                                              intptr_t len,
                                              Heap::Space space) {
   ExternalTypedData& result = ExternalTypedData::Handle();
   {
     RawObject* raw = Object::Allocate(class_id,
@@ skipping 629 matching lines @@
   return UserTag::null();
 }
 
 
 const char* UserTag::ToCString() const {
   const String& tag_label = String::Handle(label());
   return tag_label.ToCString();
 }
 
 }  // namespace dart