Improve hashCode for closures

runtime/vm/object.cc

Index: runtime/vm/object.cc
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index aa2f340ee9b21715ad748a72dab7a7db0ed740cd..00679133ac47b859dfd4d007eda8b4a0bc7ac6fb 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -6902,25 +6902,13 @@ RawInstance* Function::ImplicitInstanceClosure(const Instance& receiver) const {
                       *this, context);
 }
 
-RawSmi* Function::GetClosureHashCode() const {
+intptr_t Function::ComputeClosureHash() const {
   ASSERT(IsClosureFunction());
-  const Object& obj = Object::Handle(raw_ptr()->data_);
-  ASSERT(!obj.IsNull());
-  if (ClosureData::Cast(obj).hash() != Object::null()) {
-    return Smi::RawCast(ClosureData::Cast(obj).hash());
-  }
-  // Hash not yet computed. Compute and cache it.
   const Class& cls = Class::Handle(Owner());
   intptr_t result = String::Handle(name()).Hash();
   result += String::Handle(Signature()).Hash();
   result += String::Handle(cls.Name()).Hash();
-  // Finalize hash value like for strings so that it fits into a smi.
-  result += result << 3;
-  result ^= result >> 11;
-  result += result << 15;
-  result &= ((static_cast<intptr_t>(1) << String::kHashBits) - 1);
-  ClosureData::Cast(obj).set_hash(result);
-  return Smi::New(result);
+  return result;
 }
 
 RawString* Function::BuildSignature(NameVisibility name_visibility) const {
@@ -7330,10 +7318,6 @@ void ClosureData::set_implicit_static_closure(const Instance& closure) const {
   StorePointer(&raw_ptr()->closure_, closure.raw());
 }
 
-void ClosureData::set_hash(intptr_t value) const {
-  StorePointer(&raw_ptr()->hash_, static_cast<RawObject*>(Smi::New(value)));
-}
-
 void ClosureData::set_parent_function(const Function& value) const {
   StorePointer(&raw_ptr()->parent_function_, value.raw());
 }
@@ -14882,6 +14866,10 @@ RawObject* Instance::HashCode() const {
   return DartLibraryCalls::HashCode(*this);
 }
 
+RawObject* Instance::IdentityHashCode() const {
+  return DartLibraryCalls::IdentityHashCode(*this);
+}
+
 bool Instance::CanonicalizeEquals(const Instance& other) const {
   if (this->raw() == other.raw()) {
     return true;  // "===".
@@ -21654,6 +21642,24 @@ const char* Closure::ToCString() const {
                      from, fun_desc);
 }
 
+int64_t Closure::ComputeHash() const {
+  const Function& func = Function::Handle(function());
+  uint32_t result = func.ComputeClosureHash();
+  if (func.IsImplicitInstanceClosureFunction()) {
+    // Calculate identityHashCode of cached closure receiver and
+    // combine it with function's hash code.
+    const Context& context = Context::Handle(this->context());
+    const Object& receiver = Object::Handle(context.At(0));
+    const Object& receiverHash =
+        Object::Handle(Instance::Cast(receiver).IdentityHashCode());
+    if (receiverHash.IsInteger()) {

[zra, 2017/07/19 20:10:57]

I think if this isn't an integer then it's probably a bug. Can we assert this
instead?

[alexmarkov, 2017/07/19 20:57:50]

Added error handling.

+      result = CombineHashes(
+          result, Integer::Cast(receiverHash).AsTruncatedUint32Value());
+    }
+  }
+  return FinalizeHash(result, String::kHashBits);
+}
+
 RawClosure* Closure::New(const TypeArguments& instantiator_type_arguments,
                          const TypeArguments& function_type_arguments,
                          const Function& function,