Setup a peer for read only strings that are not externalized in

runtime/vm/dart_api_impl.cc

 // Copyright (c) 2013, 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 "include/dart_api.h"
 #include "include/dart_mirrors_api.h"
 #include "include/dart_native_api.h"
 
 #include "platform/assert.h"
 #include "vm/bigint_operations.h"
@@ skipping 222 matching lines @@
   true_handle_ = Api::NewHandle(isolate, Bool::True().raw());
 
   ASSERT(false_handle_ == NULL);
   false_handle_ = Api::NewHandle(isolate, Bool::False().raw());
 
   ASSERT(null_handle_ == NULL);
   null_handle_ = Api::NewHandle(isolate, Object::null());
 }
 
 
-#define GET_PEER(type, ctype, raw_obj, peer)                                   \
+#define EXTERNAL_PEER_HELPER(type, ctype, raw_obj, peer)                       \

[Ivan Posva, 2013/08/22 15:44:51]

Is this #define really worth the added confusion? To me it feels like the code
would be more readable if we just wrote the code twice with the appropriate
types inline.

[siva, 2013/08/22 16:57:08]

Removed the macro and inline the code below.

On 2013/08/22 15:44:51, Ivan Posva wrote:

+  type* raw_string = reinterpret_cast<type*>(raw_obj)->ptr();                  \
+  ExternalStringData<ctype>* data = raw_string->external_data_;                \
+  *peer = data->peer();                                                        \
 
-
-#define EXTERNAL_PEER_HELPER(cid, raw_obj, peer)                               \
-  switch (cid) {                                                               \
-    case kExternalOneByteStringCid: {                                          \
-      RawExternalOneByteString* raw_string =                                   \
-          reinterpret_cast<RawExternalOneByteString*>(raw_obj)->ptr();         \
-      ExternalStringData<uint8_t>* data = raw_string->external_data_;          \
-      *peer = data->peer();                                                    \
-      return true;                                                             \
-    }                                                                          \
-    case kExternalTwoByteStringCid: {                                          \
-      RawExternalTwoByteString* raw_string =                                   \
-          reinterpret_cast<RawExternalTwoByteString*>(raw_obj)->ptr();         \
-      ExternalStringData<uint16_t>* data = raw_string->external_data_;         \
-      *peer = data->peer();                                                    \
-      return true;                                                             \
-    }                                                                          \
-  }                                                                            \
-  return false;                                                                \
-
-
-bool Api::ExternalStringGetPeerHelper(Dart_NativeArguments args,
-                                      int arg_index,
-                                      void** peer) {
+bool Api::StringGetPeerHelper(Dart_NativeArguments args,
+                              int arg_index,
+                              void** peer) {
   NoGCScope no_gc_scope;
   NativeArguments* arguments = reinterpret_cast<NativeArguments*>(args);
   RawObject* raw_obj = arguments->NativeArgAt(arg_index);
   intptr_t cid = raw_obj->GetClassId();
-  EXTERNAL_PEER_HELPER(cid, raw_obj, peer);
-}
-
-
-bool Api::ExternalStringGetPeerHelper(Dart_Handle object, void** peer) {
-  NoGCScope no_gc_scope;
-  RawObject* raw_obj = Api::UnwrapHandle(object);
-  EXTERNAL_PEER_HELPER(Api::ClassId(object), raw_obj, peer);
+  if (cid == kExternalOneByteStringCid) {
+    EXTERNAL_PEER_HELPER(RawExternalOneByteString, uint8_t, raw_obj, peer);
+    return true;
+  }
+  if (cid == kOneByteStringCid || cid == kTwoByteStringCid) {
+    *peer = Isolate::Current()->heap()->GetPeer(raw_obj);

[Ivan Posva, 2013/08/22 15:44:51]

args carries the isolate or pass it in from the caller?

[siva, 2013/08/22 16:57:08]

Good point.

On 2013/08/22 15:44:51, Ivan Posva wrote:

+    return (*peer != 0);
+  }
+  if (cid == kExternalTwoByteStringCid) {
+    EXTERNAL_PEER_HELPER(RawExternalTwoByteString, uint16_t, raw_obj, peer);
+    return true;
+  }
+  return false;
 }
 
 
 void Api::SetWeakHandleReturnValue(NativeArguments* args,
                                    Dart_WeakPersistentHandle retval) {
   args->SetReturnUnsafe(Api::UnwrapAsWeakPersistentHandle(retval)->raw());
 }
 
 
 // --- Handles ---
@@ skipping 1349 matching lines @@
   DARTSCOPE(isolate);
   if (utf32_array == NULL && length != 0) {
     RETURN_NULL_ERROR(utf32_array);
   }
   CHECK_LENGTH(length, String::kMaxElements);
   CHECK_CALLBACK_STATE(isolate);
   return Api::NewHandle(isolate, String::FromUTF32(utf32_array, length));
 }
 
 
-DART_EXPORT Dart_Handle Dart_ExternalStringGetPeer(Dart_Handle object,
-                                                   void** peer) {
-  if (peer == NULL) {
-    RETURN_NULL_ERROR(peer);
-  }
-
-  if (Api::ExternalStringGetPeerHelper(object, peer)) {
-    return Api::Success();
-  }
-
-  // It's not an external string, return appropriate error.
-  if (!RawObject::IsStringClassId(Api::ClassId(object))) {
-    RETURN_TYPE_ERROR(Isolate::Current(), object, String);
-  } else {
-    return
-        Api::NewError(
-            "%s expects argument 'object' to be an external String.",
-            CURRENT_FUNC);
-  }
-}
-
-
 DART_EXPORT Dart_Handle Dart_NewExternalLatin1String(
     const uint8_t* latin1_array,
     intptr_t length,
     void* peer,
     Dart_PeerFinalizer cback) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   if (latin1_array == NULL && length != 0) {
     RETURN_NULL_ERROR(latin1_array);
   }
@@ skipping 153 matching lines @@
   intptr_t str_size = (str_obj.Length() * str_obj.CharSize());
   if ((length < str_size) || (length > String::kMaxElements)) {
     return Api::NewError("Dart_MakeExternalString "
                          "expects argument length to be in the range"
                          "[%"Pd"..%"Pd"].",
                          str_size, String::kMaxElements);
   }
   if (str_obj.InVMHeap()) {
     // Since the string object is read only we do not externalize
     // the string but instead copy the contents of the string into the
-    // specified buffer and return a Null object.
-    // This ensures that the embedder does not have to call again
-    // to get at the contents.
+    // specified buffer add the specified peer/cback as a Peer object
+    // to this string. The Api::StringGetPeerHelper function picks up
+    // the peer from the Peer table.
     intptr_t copy_len = str_obj.Length();
     if (str_obj.IsOneByteString()) {
       ASSERT(length >= copy_len);
       uint8_t* latin1_array = reinterpret_cast<uint8_t*>(array);
       for (intptr_t i = 0; i < copy_len; i++) {
         latin1_array[i] = static_cast<uint8_t>(str_obj.CharAt(i));
       }
+      OneByteString::SetPeer(str_obj, peer, cback);
     } else {
       ASSERT(str_obj.IsTwoByteString());
       ASSERT(length >= (copy_len * str_obj.CharSize()));
       uint16_t* utf16_array = reinterpret_cast<uint16_t*>(array);
       for (intptr_t i = 0; i < copy_len; i++) {
         utf16_array[i] = static_cast<uint16_t>(str_obj.CharAt(i));
       }
+      TwoByteString::SetPeer(str_obj, peer, cback);
     }
-    return Api::Null();
+    return str;
   }
   return Api::NewHandle(isolate,
                         str_obj.MakeExternal(array, length, peer, cback));
 }
 
 
 DART_EXPORT Dart_Handle Dart_StringGetProperties(Dart_Handle object,
                                                  intptr_t* char_size,
                                                  intptr_t* str_len,
                                                  void** peer) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   const String& str = Api::UnwrapStringHandle(isolate, object);
   if (str.IsNull()) {
     RETURN_TYPE_ERROR(isolate, object, String);
   }
   if (str.IsExternal()) {
     *peer = str.GetPeer();
     ASSERT(*peer != NULL);
   } else {
-    *peer = NULL;
+    NoGCScope no_gc_scope;
+    *peer = isolate->heap()->GetPeer(str.raw());
   }
   *char_size = str.CharSize();
   *str_len = str.Length();
   return Api::Success();
 }
 
 
 // --- Lists ---
 
 DART_EXPORT Dart_Handle Dart_NewList(intptr_t length) {
@@ skipping 1802 matching lines @@
   return Api::Success();
 }
 
 
 DART_EXPORT Dart_Handle Dart_GetNativeStringArgument(Dart_NativeArguments args,
                                                      int arg_index,
                                                      void** peer) {
   NativeArguments* arguments = reinterpret_cast<NativeArguments*>(args);
   Isolate* isolate = arguments->isolate();
   CHECK_ISOLATE(isolate);
-  if (Api::ExternalStringGetPeerHelper(args, arg_index, peer)) {
+  if (Api::StringGetPeerHelper(args, arg_index, peer)) {
     return Api::Success();

[Ivan Posva, 2013/08/22 15:44:51]

This function does not do what it is documented to do: Return the string object
handle if no error occurs.

[siva, 2013/08/22 16:57:08]

Updated comment to reflect this. If a peer is present then the
caller is only interested in the peer.

On 2013/08/22 15:44:51, Ivan Posva wrote:

   }
   *peer = NULL;
   ReusableObjectHandleScope reused_obj_handle(isolate);
   Object& obj = reused_obj_handle.Handle();
   obj = arguments->NativeArgAt(arg_index);
   if (RawObject::IsStringClassId(obj.GetClassId())) {
     return Api::NewHandle(isolate, obj.raw());
   }
   return Api::NewError("%s expects argument to be of"
                        " type String.", CURRENT_FUNC);
@@ skipping 530 matching lines @@
   }
   {
     NoGCScope no_gc;
     RawObject* raw_obj = obj.raw();
     isolate->heap()->SetPeer(raw_obj, peer);
   }
   return Api::Success();
 }
 
 }  // namespace dart