- Do not mix scalar values and object fields in RawInstance to

runtime/vm/object.h

Index: runtime/vm/object.h
===================================================================
--- runtime/vm/object.h	(revision 14777)
+++ runtime/vm/object.h	(working copy)
@@ -3074,14 +3074,9 @@
     return ((index >= 0) && (index < clazz()->ptr()->num_native_fields_));
   }
 
-  intptr_t GetNativeField(int index) const {
-    return *NativeFieldAddr(index);
-  }
+  inline intptr_t GetNativeField(Isolate* isolate, int index) const;
+  void SetNativeField(int index, intptr_t value) const;

[cshapiro, 2012/11/12 18:48:45]

Interesting, GetNativeField needs the isolate but SetNativeField does not?

[Tom Ball, 2012/11/12 21:05:15]

SetNativeField calls Object::Handle(RawObject*), which calls
Isolate::Current().  If the isolate is already known to the caller, passing it
in would avoid this extra call.

[Ivan Posva, 2012/11/12 22:01:03]

The reason I pass the isolate in GetNativeField is because that is a much more
frequent operation according to our measurement of Dart API usage. The impact of
this change drops from 16.5% to 6% with passing the isolate and inlining the
call. I am sure there is more optimization opportunity inside GetNativeField.

 
-  void SetNativeField(int index, intptr_t value) const {
-    *NativeFieldAddr(index) = value;
-  }
-
   // Returns true if the instance is a closure object.
   bool IsClosure() const;
 
@@ -3099,11 +3094,8 @@
   RawObject** FieldAddr(const Field& field) const {
     return FieldAddrAtOffset(field.Offset());
   }
-  intptr_t* NativeFieldAddr(int index) const {
-    ASSERT(IsValidNativeIndex(index));
-    return reinterpret_cast<intptr_t*>((raw_value() - kHeapObjectTag)
-                                       + (index * kWordSize)
-                                       + sizeof(RawObject));
+  RawObject** NativeFieldsAddr() const {
+    return FieldAddrAtOffset(sizeof(RawObject));
   }
   void SetFieldAtOffset(intptr_t offset, const Object& value) const {
     StorePointer(FieldAddrAtOffset(offset), value.raw());
@@ -5849,6 +5841,16 @@
 }
 
 
+intptr_t Instance::GetNativeField(Isolate* isolate, int index) const {
+  ASSERT(IsValidNativeIndex(index));
+  const Object& native_fields = Object::Handle(isolate, *NativeFieldsAddr());
+  if (native_fields.IsNull()) {
+    return 0;
+  }
+  return IntPtrArray::Cast(native_fields).At(index);
+}
+
+
 bool String::Equals(const String& str) const {
   if (raw() == str.raw()) {
     return true;  // Both handles point to the same raw instance.