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

runtime/vm/object.h

 // 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.
 
 #ifndef VM_OBJECT_H_
 #define VM_OBJECT_H_
 
 #include "include/dart_api.h"
 #include "platform/assert.h"
 #include "platform/utils.h"
@@ skipping 3056 matching lines @@
 
   // Check if the type of this instance is a subtype of the given type.
   bool IsInstanceOf(const AbstractType& type,
                     const AbstractTypeArguments& type_instantiator,
                     Error* malformed_error) const;
 
   bool IsValidNativeIndex(int index) const {
     return ((index >= 0) && (index < clazz()->ptr()->num_native_fields_));
   }
 
-  intptr_t GetNativeField(int index) const {
-    return *NativeFieldAddr(index);
-  }
-
-  void SetNativeField(int index, intptr_t value) const {
-    *NativeFieldAddr(index) = value;
-  }
+  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.

 
   // Returns true if the instance is a closure object.
   bool IsClosure() const;
 
   static intptr_t InstanceSize() {
     return RoundedAllocationSize(sizeof(RawInstance));
   }
 
   static RawInstance* New(const Class& cls, Heap::Space space = Heap::kNew);
 
  private:
   RawObject** FieldAddrAtOffset(intptr_t offset) const {
     ASSERT(IsValidFieldOffset(offset));
     return reinterpret_cast<RawObject**>(raw_value() - kHeapObjectTag + offset);
   }
   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());
   }
   bool IsValidFieldOffset(int offset) const;
 
   // TODO(iposva): Determine if this gets in the way of Smi.
   HEAP_OBJECT_IMPLEMENTATION(Instance, Object);
   friend class Class;
   friend class Closure;
@@ skipping 2725 matching lines @@
   ASSERT(!is_static());  // SetOffset is valid only for instance fields.
   raw_ptr()->value_ = Smi::New(value);
 }
 
 
 void Context::SetAt(intptr_t index, const Instance& value) const {
   StorePointer(InstanceAddr(index), value.raw());
 }
 
 
+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.
   }
   if (str.IsNull()) {
     return false;
   }
   return Equals(str, 0, str.Length());
 }
 
@@ skipping 12 matching lines @@
     if (this->CharAt(i) != str.CharAt(begin_index + i)) {
       return false;
     }
   }
   return true;
 }
 
 }  // namespace dart
 
 #endif  // VM_OBJECT_H_