Amend PersistentValueMap.

include/v8-util.h

Index: include/v8-util.h
diff --git a/include/v8-util.h b/include/v8-util.h
index 3f8cc6d269d0a6598944720980771210de49b73c..ad897eea4a9a93bc8da1968e1bf85465b5cb6c73 100644
--- a/include/v8-util.h
+++ b/include/v8-util.h
@@ -42,6 +42,10 @@ namespace v8 {
 
 typedef uintptr_t PersistentContainerValue;
 static const uintptr_t kPersistentContainerNotFound = 0;
+enum PersistentValueMapCallbackType {

[dcarney, 2014/03/27 16:36:11]

should be persistentcontainercallbacktype, since we'll want to use it for
vectors as well

[vogelheim, 2014/03/27 17:05:12]

Done.

+  kNotWeak,
+  kWeak
+};
 
 
 /**
@@ -92,38 +96,34 @@ class StdMapTraits {
 /**
  * A default trait implementation for PersistentValueMap, which inherits
  * a std:map backing map from StdMapTraits and holds non-weak persistent
- * objects.
+ * objects and has no special Dispose handling.
  *
- * Users have to implement their own dispose trait.
+ * You should not derive from this class, since MapType depends on the
+ * surrounding class, and hence a subclass cannot simply inherit the methods.
  */
 template<typename K, typename V>
-class StrongMapTraits : public StdMapTraits<K, V> {
+class DefaultPersistentValueMapTraits : public StdMapTraits<K, V> {
  public:
   // Weak callback & friends:
-  static const bool kIsWeak = false;
-  typedef typename StdMapTraits<K, V>::Impl Impl;
+  static const PersistentValueMapCallbackType kIsWeak = kNotWeak;

[dcarney, 2014/03/27 16:36:11]

i think you want to rename this to kCallbackType

[vogelheim, 2014/03/27 17:05:12]

Done.

+  typedef PersistentValueMap<K, V, DefaultPersistentValueMapTraits<K, V> >
+      MapType;
   typedef void WeakCallbackDataType;
+
   static WeakCallbackDataType* WeakCallbackParameter(
-      Impl* impl, const K& key, Local<V> value);
-  static Impl* ImplFromWeakCallbackData(
-      const WeakCallbackData<V, WeakCallbackDataType>& data);
+      MapType* map, const K& key, Local<V> value) {
+    return NULL;
+  }
+  static MapType* MapFromWeakCallbackData(
+          const WeakCallbackData<V, WeakCallbackDataType>& data) {
+    return NULL;
+  }
   static K KeyFromWeakCallbackData(
-      const WeakCallbackData<V, WeakCallbackDataType>& data);
-  static void DisposeCallbackData(WeakCallbackDataType* data);
-};
-
-
-/**
- * A default trait implementation for PersistentValueMap, with a std::map
- * backing map, non-weak persistents as values, and no special dispose
- * handling. Can be used as-is.
- */
-template<typename K, typename V>
-class DefaultPersistentValueMapTraits : public StrongMapTraits<K, V> {
- public:
-  typedef typename StrongMapTraits<K, V>::Impl Impl;
-  static void Dispose(Isolate* isolate, UniquePersistent<V> value,
-      Impl* impl, K key) { }
+      const WeakCallbackData<V, WeakCallbackDataType>& data) {
+    return K();
+  }
+  static void DisposeCallbackData(WeakCallbackDataType* data) { }
+  static void Dispose(Isolate* isolate, UniquePersistent<V> value, K key) { }
 };
 
 
@@ -154,7 +154,7 @@ class PersistentValueMap {
   /**
    * Return whether the map holds weak persistents.
    */
-  V8_INLINE bool IsWeak() { return Traits::kIsWeak; }
+  V8_INLINE bool IsWeak() { return Traits::kIsWeak != kNotWeak; }
 
   /**
    * Get value stored in map.
@@ -167,7 +167,7 @@ class PersistentValueMap {
    * Check whether a value is contained in the map.
    */
   V8_INLINE bool Contains(const K& key) {
-    return Traits::Get(&impl_, key) != 0;
+    return Traits::Get(&impl_, key) != kPersistentContainerNotFound;
   }
 
   /**
@@ -175,14 +175,8 @@ class PersistentValueMap {
    * Return true if a value was found.
    */
   V8_INLINE bool SetReturnValue(const K& key,
-      ReturnValue<Value>& returnValue) {
-    PersistentContainerValue value = Traits::Get(&impl_, key);
-    bool hasValue = value != 0;
-    if (hasValue) {
-      returnValue.SetInternal(
-          *reinterpret_cast<internal::Object**>(FromVal(value)));
-    }
-    return hasValue;
+      ReturnValue<Value> returnValue) {
+    return SetReturnValueFromVal(returnValue, Traits::Get(&impl_, key));
   }
 
   /**
@@ -231,12 +225,76 @@ class PersistentValueMap {
       typename Traits::Impl impl;
       Traits::Swap(impl_, impl);
       for (It i = Traits::Begin(&impl); i != Traits::End(&impl); ++i) {
-        Traits::Dispose(isolate_, Release(Traits::Value(i)).Pass(), &impl,
-          Traits::Key(i));
+        Traits::Dispose(isolate_, Release(Traits::Value(i)).Pass(),
+                        Traits::Key(i));
       }
     }
   }
 
+  /**
+   * Helper class for GetReference/SetWithReference. Do not use outside
+   * that context.
+   */
+  class PersistentValueReference {
+   public:
+    PersistentValueReference() : value_(kPersistentContainerNotFound) { }
+    PersistentValueReference(const PersistentValueReference& other)
+        : value_(other.value_) { }
+
+    Local<V> NewLocal(Isolate* isolate) const {
+      return Local<V>::New(isolate, FromVal(value_));
+    }
+    bool IsEmpty() const {
+      return value_ == kPersistentContainerNotFound;
+    }
+    template<typename T>
+    bool SetReturnValue(ReturnValue<T> returnValue) {
+      return SetReturnValueFromVal(returnValue, value_);
+    }
+    void Reset() {
+      value_ = kPersistentContainerNotFound;
+    }
+    void operator=(const PersistentValueReference& other) {
+      value_ = other.value_;
+    }
+
+   private:
+    friend class PersistentValueMap;
+
+    explicit PersistentValueReference(PersistentContainerValue value)
+        : value_(value) { }
+
+    void operator=(PersistentContainerValue value) {
+      value_ = value;
+    }
+
+    PersistentContainerValue value_;
+  };
+
+  /**
+   * Get a reference to a map value. This enables fast, repeated access
+   * to a value stored in the map while the map remains unchanged.
+   *
+   * Careful: This is potentially unsafe, so please use with care.
+   * The value will become invalid if the value for this key changes
+   * in the underlying map, as a result of Set or Remove for the same
+   * key; as a result of the weak callback for the same key; or as a
+   * result of calling Clear() or destruction of the map.
+   */
+  V8_INLINE PersistentValueReference GetReference(const K& key) {
+    return PersistentValueReference(Traits::Get(&impl_, key));
+  }
+
+  /**
+   * Put a value into the map and update the reference.
+   * Restrictions of GetReference apply here as well.
+   */
+  UniquePersistent<V> Set(const K& key, UniquePersistent<V> value,
+                          PersistentValueReference* reference) {
+    *reference = Leak(&value);
+    return SetUnique(key, &value);
+  }
+
  private:
   PersistentValueMap(PersistentValueMap&);
   void operator=(PersistentValueMap&);
@@ -246,10 +304,10 @@ class PersistentValueMap {
    * by the Traits class.
    */
   UniquePersistent<V> SetUnique(const K& key, UniquePersistent<V>* persistent) {
-    if (Traits::kIsWeak) {
+    if (Traits::kIsWeak != kNotWeak) {
       Local<V> value(Local<V>::New(isolate_, *persistent));
       persistent->template SetWeak<typename Traits::WeakCallbackDataType>(
-        Traits::WeakCallbackParameter(&impl_, key, value), WeakCallback);
+        Traits::WeakCallbackParameter(this, key, value), WeakCallback);
     }
     PersistentContainerValue old_value =
         Traits::Set(&impl_, key, ClearAndLeak(persistent));
@@ -258,11 +316,12 @@ class PersistentValueMap {
 
   static void WeakCallback(
       const WeakCallbackData<V, typename Traits::WeakCallbackDataType>& data) {
-    if (Traits::kIsWeak) {
-      typename Traits::Impl* impl = Traits::ImplFromWeakCallbackData(data);
+    if (Traits::kIsWeak != kNotWeak) {
+      PersistentValueMap<K, V, Traits>* persistentValueMap =
+          Traits::MapFromWeakCallbackData(data);
       K key = Traits::KeyFromWeakCallbackData(data);
-      PersistentContainerValue value = Traits::Remove(impl, key);
-      Traits::Dispose(data.GetIsolate(), Release(value).Pass(), impl, key);
+      Traits::Dispose(data.GetIsolate(),
+                      persistentValueMap->Remove(key).Pass(), key);
     }
   }
 
@@ -270,6 +329,16 @@ class PersistentValueMap {
     return reinterpret_cast<V*>(v);
   }
 
+  V8_INLINE static bool SetReturnValueFromVal(
+      ReturnValue<Value>& returnValue, PersistentContainerValue value) {
+    bool hasValue = value != kPersistentContainerNotFound;
+    if (hasValue) {
+      returnValue.SetInternal(
+          *reinterpret_cast<internal::Object**>(FromVal(value)));
+    }
+    return hasValue;
+  }
+
   V8_INLINE static PersistentContainerValue ClearAndLeak(
       UniquePersistent<V>* persistent) {
     V* v = persistent->val_;
@@ -277,6 +346,11 @@ class PersistentValueMap {
     return reinterpret_cast<PersistentContainerValue>(v);
   }
 
+  V8_INLINE static PersistentContainerValue Leak(
+      UniquePersistent<V>* persistent) {
+    return reinterpret_cast<PersistentContainerValue>(persistent->val_);
+  }
+
   /**
    * Return a container value as UniquePersistent and make sure the weak
    * callback is properly disposed of. All remove functionality should go
@@ -285,7 +359,7 @@ class PersistentValueMap {
   V8_INLINE static UniquePersistent<V> Release(PersistentContainerValue v) {
     UniquePersistent<V> p;
     p.val_ = FromVal(v);
-    if (Traits::kIsWeak && !p.IsEmpty()) {
+    if (Traits::kIsWeak != kNotWeak && !p.IsEmpty()) {
       Traits::DisposeCallbackData(
           p.template ClearWeak<typename Traits::WeakCallbackDataType>());
     }
@@ -312,44 +386,6 @@ class StdPersistentValueMap : public PersistentValueMap<K, V, Traits> {
       : PersistentValueMap<K, V, Traits>(isolate) {}
 };
 
-
-/**
- * Empty default implementations for StrongTraits methods.
- *
- * These should not be necessary, since they're only used in code that
- * is surrounded by if(Traits::kIsWeak), which for StrongMapTraits is
- * compile-time false. Most compilers can live without them; however
- * the compiler we use from 64-bit Win differs.
- *
- * TODO(vogelheim): Remove these once they're no longer necessary.
- */
-template<typename K, typename V>
-typename StrongMapTraits<K, V>::WeakCallbackDataType*
-    StrongMapTraits<K, V>::WeakCallbackParameter(
-        Impl* impl, const K& key, Local<V> value) {
-  return NULL;
-}
-
-
-template<typename K, typename V>
-typename StrongMapTraits<K, V>::Impl*
-    StrongMapTraits<K, V>::ImplFromWeakCallbackData(
-        const WeakCallbackData<V, WeakCallbackDataType>& data) {
-  return NULL;
-}
-
-
-template<typename K, typename V>
-K StrongMapTraits<K, V>::KeyFromWeakCallbackData(
-    const WeakCallbackData<V, WeakCallbackDataType>& data) {
-  return K();
-}
-
-
-template<typename K, typename V>
-void StrongMapTraits<K, V>::DisposeCallbackData(WeakCallbackDataType* data) {
-}
-
 }  // namespace v8
 
 #endif  // V8_UTIL_H_