Use symbols when looking up fields in a class

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/cpu.h"
@@ skipping 4143 matching lines @@
     return Field::null();
   }
   REUSABLE_ARRAY_HANDLESCOPE(thread);
   REUSABLE_FIELD_HANDLESCOPE(thread);
   REUSABLE_STRING_HANDLESCOPE(thread);
   Array& flds = thread->ArrayHandle();
   flds ^= fields();
   ASSERT(!flds.IsNull());
   intptr_t len = flds.Length();
   Field& field = thread->FieldHandle();
+  if (name.IsSymbol()) {
+    // Use fast raw pointer string compare for symbols.
+    for (intptr_t i = 0; i < len; i++) {
+      field ^= flds.At(i);
+      if (name.raw() == field.name()) {
+        if (kind == kInstance) {
+          return field.is_static() ? Field::null() : field.raw();
+        } else if (kind == kStatic) {
+          return field.is_static() ? field.raw() : Field::null();
+        }
+        ASSERT(kind == kAny);
+        return field.raw();
+      }
+    }
+  } else {
+    String& field_name = thread->StringHandle();
+    for (intptr_t i = 0; i < len; i++) {
+      field ^= flds.At(i);
+      field_name ^= field.name();
+      if (name.Equals(field_name)) {
+        if (kind == kInstance) {
+          return field.is_static() ? Field::null() : field.raw();
+        } else if (kind == kStatic) {
+          return field.is_static() ? field.raw() : Field::null();
+        }
+        ASSERT(kind == kAny);
+        return field.raw();

[siva, 2016/04/07 22:51:30]

Not sure if it is worth extracting the code inside the if block into :

static RawField* getField(MemberKind kind, const Field& field) { ... }

and do return getField(kind, field); in the two places.

[hausner, 2016/04/07 23:37:03]

I hesitate to create a mini function to factor out 5 lines of code that
otherwise don't have any use. In the *AllowPrivate version of this function, I
pulled this check out into the caller. That requires the caller to allocate
another Handle to do the is_static() check. I didn't want that for the fast
path.

If you insist, I'll do it in a follow-up CL.

+      }
+    }
+  }
+  return Field::null();
+}
+
+
+RawField* Class::LookupFieldAllowPrivate(const String& name) const {
+  // Use slow string compare, ignoring privacy name mangling.
+  Thread* thread = Thread::Current();
+  if (EnsureIsFinalized(thread) != Error::null()) {
+    return Field::null();
+  }
+  REUSABLE_ARRAY_HANDLESCOPE(thread);
+  REUSABLE_FIELD_HANDLESCOPE(thread);
+  REUSABLE_STRING_HANDLESCOPE(thread);
+  Array& flds = thread->ArrayHandle();
+  flds ^= fields();
+  ASSERT(!flds.IsNull());
+  intptr_t len = flds.Length();
+  Field& field = thread->FieldHandle();
   String& field_name = thread->StringHandle();
   for (intptr_t i = 0; i < len; i++) {
     field ^= flds.At(i);
     field_name ^= field.name();
     if (String::EqualsIgnoringPrivateKey(field_name, name)) {
-      if (kind == kInstance) {
-        if (!field.is_static()) {
-          return field.raw();
-        }
-      } else if (kind == kStatic) {
-        if (field.is_static()) {
-          return field.raw();
-        }
-      } else if (kind == kAny) {
-        return field.raw();
-      }
-      return Field::null();
+      return field.raw();
     }
   }
-  // No field found.
   return Field::null();
 }
 
+
+RawField* Class::LookupInstanceFieldAllowPrivate(const String& name) const {
+  Field& field = Field::Handle(LookupFieldAllowPrivate(name));
+  if (!field.IsNull() && !field.is_static()) {
+    return field.raw();
+  }
+  return Field::null();
+}
+
+
+RawField* Class::LookupStaticFieldAllowPrivate(const String& name) const {
+  Field& field = Field::Handle(LookupFieldAllowPrivate(name));
+  if (!field.IsNull() && field.is_static()) {
+    return field.raw();
+  }
+  return Field::null();
+}
+
 
 RawLibraryPrefix* Class::LookupLibraryPrefix(const String& name) const {
   Zone* zone = Thread::Current()->zone();
   const Library& lib = Library::Handle(zone, library());
   const Object& obj = Object::Handle(zone, lib.LookupLocalObject(name));
   if (!obj.IsNull() && obj.IsLibraryPrefix()) {
     return LibraryPrefix::Cast(obj).raw();
   }
   return LibraryPrefix::null();
 }
@@ skipping 5210 matching lines @@
       ASSERT(Array::Cast(metadata).raw() != Object::empty_array().raw());
       field.SetStaticValue(Array::Cast(metadata), true);
     }
   }
   return metadata.raw();
 }
 
 
 static bool ShouldBePrivate(const String& name) {
   return
-      (name.Length() >= 1 &&
-       name.CharAt(0) == '_') ||
+      (name.Length() >= 1 && name.CharAt(0) == '_') ||
       (name.Length() >= 5 &&
-       (name.CharAt(4) == '_' &&
-        (name.CharAt(0) == 'g' || name.CharAt(0) == 's') &&
-        name.CharAt(1) == 'e' &&
-        name.CharAt(2) == 't' &&
-        name.CharAt(3) == ':'));
+        (name.CharAt(4) == '_' &&
+          (name.CharAt(0) == 'g' || name.CharAt(0) == 's') &&
+          name.CharAt(1) == 'e' &&
+          name.CharAt(2) == 't' &&
+          name.CharAt(3) == ':'));

[sra1, 2016/04/08 20:30:06]

Testing ':' before 'g' / 's' will fail faster.

[hausner, 2016/04/08 20:41:20]

The test (name.CharAt(4) == '_') eliminates practically all non-getter and
non-setter names. I would not lose sleep over this :)

 }
 
 
 RawObject* Library::ResolveName(const String& name) const {
   Object& obj = Object::Handle();
   if (FLAG_use_lib_cache && LookupResolvedNamesCache(name, &obj)) {
     return obj.raw();
   }
   obj = LookupLocalObject(name);
   if (!obj.IsNull()) {
     // Names that are in this library's dictionary and are unmangled
     // are not cached. This reduces the size of the the cache.
     return obj.raw();
   }
+
   String& accessor_name = String::Handle(Field::LookupGetterSymbol(name));
   if (!accessor_name.IsNull()) {
     obj = LookupLocalObject(accessor_name);
   }
   if (obj.IsNull()) {
     accessor_name = Field::LookupSetterSymbol(name);
     if (!accessor_name.IsNull()) {
       obj = LookupLocalObject(accessor_name);
     }
     if (obj.IsNull() && !ShouldBePrivate(name)) {
       obj = LookupImportedObject(name);
     }
   }
+
   AddToResolvedNamesCache(name, obj);
   return obj.raw();
 }
 
 
 class StringEqualsTraits {
  public:
   static bool IsMatch(const Object& a, const Object& b) {
     return String::Cast(a).Equals(String::Cast(b));
   }
@@ skipping 12419 matching lines @@
   return UserTag::null();
 }
 
 
 const char* UserTag::ToCString() const {
   const String& tag_label = String::Handle(label());
   return tag_label.ToCString();
 }
 
 }  // namespace dart