Improve constant pool implementation in the assembler.

runtime/vm/assembler_x64.h

Index: runtime/vm/assembler_x64.h
===================================================================
--- runtime/vm/assembler_x64.h	(revision 42727)
+++ runtime/vm/assembler_x64.h	(working copy)
@@ -1026,34 +1026,44 @@
   // Pair type parameter for DirectChainedHashMap.
   class ObjIndexPair {
    public:
-    // TODO(zra): A WeakTable should be used here instead, but then it would
-    // also have to be possible to register and de-register WeakTables with the
-    // heap. Also, the Assembler would need to become a StackResource.
-    // Issue 13305. In the meantime...
-    // CAUTION: the RawObject* below is only safe because:
-    // The HashMap that will use this pair type will not contain any RawObject*
-    // keys that are not in the object_pool_ array. Since the keys will be
-    // visited by the GC when it visits the object_pool_, and since all objects
-    // in the object_pool_ are Old (and so will not be moved) the GC does not
-    // also need to visit the keys here in the HashMap.
-
     // Typedefs needed for the DirectChainedHashMap template.
-    typedef RawObject* Key;
+    typedef const Object* Key;
     typedef intptr_t Value;
     typedef ObjIndexPair Pair;
 
-    ObjIndexPair(Key key, Value value) : key_(key), value_(value) { }
+    explicit ObjIndexPair(Value value) : key_(NULL), value_(NoValue()) { }
 
+    ObjIndexPair(Key key, Value value)
+       : key_(key->IsNotTemporaryScopedHandle()
+           ? key : &Object::ZoneHandle(key->raw())),

[zra, 2015/01/12 17:33:59]

I'm not sure I understand what the implications are here. Are we stuck with
these objects in the zone, even if the code is collected?

[Florian Schneider, 2015/01/13 09:54:19]

The zone goes away as soon as compilation is finished.

+         value_(value) { }
+
     static Key KeyOf(Pair kv) { return kv.key_; }
 
     static Value ValueOf(Pair kv) { return kv.value_; }
 
+    static Value NoValue() { return -1; }
+
     static intptr_t Hashcode(Key key) {
-      return reinterpret_cast<intptr_t>(key) >> kObjectAlignmentLog2;
+      if (key->IsSmi()) {
+        return Smi::Cast(*key).Value();
+      }
+      if (key->IsDouble()) {
+        return static_cast<intptr_t>(
+            bit_cast<int32_t, float>(
+                static_cast<float>(Double::Cast(*key).value())));
+      }
+      if (key->IsMint()) {
+        return static_cast<intptr_t>(Mint::Cast(*key).value());
+      }
+      if (key->IsString()) {
+        return String::Cast(*key).Hash();
+      }
+      return key->GetClassId();

[zra, 2015/01/12 17:33:59]

So, Bool::True and Bool::False will end up in the same chain? Is there anything
else we could do here? I guess this is okay for now since we're not putting too
many objects with the same class into the object pools, but it'd be nice if
there were at least a plan in a TODO for what to do in case that changes.

[Florian Schneider, 2015/01/13 09:54:19]

So, for Object::null, Bool::True/False we don't even need to lookup the index,
since those are at index 0, 1 and 2.
For other classes, the plan is to have a better hash function for those classes
eventually. I'll add a TODO.

     }
 
     static inline bool IsKeyEqual(Pair kv, Key key) {
-      return kv.key_ == key;
+      return kv.key_->raw() == key->raw();
     }
 
    private: