Revision: 20084
Author:   [email protected]
Date:     Wed Mar 19 15:35:02 2014 UTC
Log:      Provide default traits for PersistentValueMap

Re-try of issue 201643003. This caused linker errors on Win64, since the
linker insists on seeing the StrongMapTrait method implementations even
though they are never used. This will provide default implementations
for them.

[email protected]

BUG=

Review URL: https://codereview.chromium.org/204343006

Patch from Daniel Vogelheim <[email protected]>.
http://code.google.com/p/v8/source/detail?r=20084

Modified:
 /branches/bleeding_edge/include/v8-util.h
 /branches/bleeding_edge/test/cctest/test-api.cc

=======================================
--- /branches/bleeding_edge/include/v8-util.h   Tue Mar 18 16:32:39 2014 UTC
+++ /branches/bleeding_edge/include/v8-util.h   Wed Mar 19 15:35:02 2014 UTC
@@ -29,6 +29,7 @@
 #define V8_UTIL_H_

 #include "v8.h"
+#include <map>

 /**
  * Support for Persistent containers.
@@ -42,6 +43,90 @@
 typedef uintptr_t PersistentContainerValue;
 static const uintptr_t kPersistentContainerNotFound = 0;

+
+/**
+ * A default trait implemenation for PersistentValueMap which uses std::map
+ * as a backing map.
+ *
+ * Users will have to implement their own weak callbacks & dispose traits.
+ */
+template<typename K, typename V>
+class StdMapTraits {
+ public:
+  // STL map & related:
+  typedef std::map<K, PersistentContainerValue> Impl;
+  typedef typename Impl::iterator Iterator;
+
+  static bool Empty(Impl* impl) { return impl->empty(); }
+  static size_t Size(Impl* impl) { return impl->size(); }
+  static void Swap(Impl& a, Impl& b) { std::swap(a, b); }  // NOLINT
+  static Iterator Begin(Impl* impl) { return impl->begin(); }
+  static Iterator End(Impl* impl) { return impl->end(); }
+  static K Key(Iterator it) { return it->first; }
+  static PersistentContainerValue Value(Iterator it) { return it->second; }
+  static PersistentContainerValue Set(Impl* impl, K key,
+      PersistentContainerValue value) {
+ std::pair<Iterator, bool> res = impl->insert(std::make_pair(key, value));
+    PersistentContainerValue old_value = kPersistentContainerNotFound;
+    if (!res.second) {
+      old_value = res.first->second;
+      res.first->second = value;
+    }
+    return old_value;
+  }
+  static PersistentContainerValue Get(Impl* impl, K key) {
+    Iterator it = impl->find(key);
+    if (it == impl->end()) return kPersistentContainerNotFound;
+    return it->second;
+  }
+  static PersistentContainerValue Remove(Impl* impl, K key) {
+    Iterator it = impl->find(key);
+    if (it == impl->end()) return kPersistentContainerNotFound;
+    PersistentContainerValue value = it->second;
+    impl->erase(it);
+    return value;
+  }
+};
+
+
+/**
+ * A default trait implementation for PersistentValueMap, which inherits
+ * a std:map backing map from StdMapTraits and holds non-weak persistent
+ * objects.
+ *
+ * Users have to implement their own dispose trait.
+ */
+template<typename K, typename V>
+class StrongMapTraits : public StdMapTraits<K, V> {
+ public:
+  // Weak callback & friends:
+  static const bool kIsWeak = false;
+  typedef typename StdMapTraits<K, V>::Impl Impl;
+  typedef void WeakCallbackDataType;
+  static WeakCallbackDataType* WeakCallbackParameter(
+      Impl* impl, const K& key, Local<V> value);
+  static Impl* ImplFromWeakCallbackData(
+      const WeakCallbackData<V, WeakCallbackDataType>& data);
+  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) { }
+};
+
+
 /**
  * A map wrapper that allows using UniquePersistent as a mapped value.
  * C++11 embedders don't need this class, as they can use UniquePersistent
@@ -52,7 +137,7 @@
  * PersistentContainerValue, with all conversion into and out of V8
  * handles being transparently handled by this class.
  */
-template<class K, class V, class Traits>
+template<typename K, typename V, typename Traits>
 class PersistentValueMap {
  public:
V8_INLINE explicit PersistentValueMap(Isolate* isolate) : isolate_(isolate) {}
@@ -65,6 +150,11 @@
    * Return size of the map.
    */
   V8_INLINE size_t Size() { return Traits::Size(&impl_); }
+
+  /**
+   * Return whether the map holds weak persistents.
+   */
+  V8_INLINE bool IsWeak() { return Traits::kIsWeak; }

   /**
    * Get value stored in map.
@@ -85,13 +175,21 @@
    * Return true if a value was found.
    */
   V8_INLINE bool SetReturnValue(const K& key,
-    ReturnValue<Value>& returnValue);
+      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;
+  }

   /**
    * Call Isolate::SetReference with the given parent and the map value.
    */
   V8_INLINE void SetReference(const K& key,
-      const v8::Persistent<v8::Object>& parent) {
+      const Persistent<Object>& parent) {
     GetIsolate()->SetReference(
       reinterpret_cast<internal::Object**>(parent.val_),
reinterpret_cast<internal::Object**>(FromVal(Traits::Get(&impl_, key))));
@@ -125,7 +223,19 @@
   * Traverses the map repeatedly,
   * in case side effects of disposal cause insertions.
   **/
-  void Clear();
+  void Clear() {
+    typedef typename Traits::Iterator It;
+    HandleScope handle_scope(isolate_);
+    // TODO(dcarney): figure out if this swap and loop is necessary.
+    while (!Traits::Empty(&impl_)) {
+      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));
+      }
+    }
+  }

  private:
   PersistentValueMap(PersistentValueMap&);
@@ -147,10 +257,19 @@
   }

   static void WeakCallback(
- const WeakCallbackData<V, typename Traits::WeakCallbackDataType>& data); + const WeakCallbackData<V, typename Traits::WeakCallbackDataType>& data) {
+    if (Traits::kIsWeak) {
+      typename Traits::Impl* impl = Traits::ImplFromWeakCallbackData(data);
+      K key = Traits::KeyFromWeakCallbackData(data);
+      PersistentContainerValue value = Traits::Remove(impl, key);
+      Traits::Dispose(data.GetIsolate(), Release(value).Pass(), impl, key);
+    }
+  }
+
   V8_INLINE static V* FromVal(PersistentContainerValue v) {
     return reinterpret_cast<V*>(v);
   }
+
   V8_INLINE static PersistentContainerValue ClearAndLeak(
       UniquePersistent<V>* persistent) {
     V* v = persistent->val_;
@@ -177,41 +296,58 @@
   typename Traits::Impl impl_;
 };

-template <class K, class V, class Traits>
-bool PersistentValueMap<K, V, Traits>::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;
+
+/**
+ * A map that uses UniquePersistent as value and std::map as the backing
+ * implementation. Persistents are held non-weak.
+ *
+ * C++11 embedders don't need this class, as they can use
+ * UniquePersistent directly in std containers.
+ */
+template<typename K, typename V,
+    typename Traits = DefaultPersistentValueMapTraits<K, V> >
+class StdPersistentValueMap : public PersistentValueMap<K, V, Traits> {
+ public:
+  explicit StdPersistentValueMap(Isolate* isolate)
+      : 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 <class K, class V, class Traits>
-void PersistentValueMap<K, V, Traits>::Clear() {
-  typedef typename Traits::Iterator It;
-  HandleScope handle_scope(isolate_);
-  // TODO(dcarney): figure out if this swap and loop is necessary.
-  while (!Traits::Empty(&impl_)) {
-    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));
-    }
-  }
+
+template<typename K, typename V>
+typename StrongMapTraits<K, V>::Impl*
+    StrongMapTraits<K, V>::ImplFromWeakCallbackData(
+        const WeakCallbackData<V, WeakCallbackDataType>& data) {
+  return NULL;
 }


-template <class K, class V, class Traits>
-void PersistentValueMap<K, V, Traits>::WeakCallback(
- const WeakCallbackData<V, typename Traits::WeakCallbackDataType>& data) {
-  typename Traits::Impl* impl = Traits::ImplFromWeakCallbackData(data);
-  K key = Traits::KeyFromWeakCallbackData(data);
-  PersistentContainerValue value = Traits::Remove(impl, key);
-  Traits::Dispose(data.GetIsolate(), Release(value).Pass(), impl, key);
+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
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Wed Mar 19 13:24:13 2014 UTC +++ /branches/bleeding_edge/test/cctest/test-api.cc Wed Mar 19 15:35:02 2014 UTC
@@ -3445,47 +3445,15 @@
 }


-template<typename K, typename V, bool is_weak>
-class StdPersistentValueMapTraits {
+template<typename K, typename V>
+class WeakStdMapTraits : public v8::StdMapTraits<K, V> {
  public:
-  static const bool kIsWeak = is_weak;
-  typedef v8::PersistentContainerValue VInt;
-  typedef std::map<K, VInt> Impl;
+  typedef typename v8::DefaultPersistentValueMapTraits<K, V>::Impl Impl;
+  static const bool kIsWeak = true;
   struct WeakCallbackDataType {
     Impl* impl;
     K key;
   };
-  typedef typename Impl::iterator Iterator;
-  static bool Empty(Impl* impl) { return impl->empty(); }
-  static size_t Size(Impl* impl) { return impl->size(); }
-  static void Swap(Impl& a, Impl& b) { std::swap(a, b); }  // NOLINT
-  static Iterator Begin(Impl* impl) { return impl->begin(); }
-  static Iterator End(Impl* impl) { return impl->end(); }
-  static K Key(Iterator it) { return it->first; }
-  static VInt Value(Iterator it) { return it->second; }
-  static VInt Set(Impl* impl, K key, VInt value) {
- std::pair<Iterator, bool> res = impl->insert(std::make_pair(key, value));
-    VInt old_value = v8::kPersistentContainerNotFound;
-    if (!res.second) {
-      old_value = res.first->second;
-      res.first->second = value;
-    }
-    return old_value;
-  }
-  static VInt Get(Impl* impl, K key) {
-    Iterator it = impl->find(key);
-    if (it == impl->end()) return v8::kPersistentContainerNotFound;
-    return it->second;
-  }
-  static VInt Remove(Impl* impl, K key) {
-    Iterator it = impl->find(key);
-    if (it == impl->end()) return v8::kPersistentContainerNotFound;
-    VInt value = it->second;
-    impl->erase(it);
-    return value;
-  }
-  static void Dispose(v8::Isolate* isolate, v8::UniquePersistent<V> value,
-      Impl* impl, K key) {}
   static WeakCallbackDataType* WeakCallbackParameter(
       Impl* impl, const K& key, Local<V> value) {
     WeakCallbackDataType* data = new WeakCallbackDataType;
@@ -3504,15 +3472,15 @@
   static void DisposeCallbackData(WeakCallbackDataType* data) {
     delete data;
   }
+  static void Dispose(v8::Isolate* isolate, v8::UniquePersistent<V> value,
+      Impl* impl, K key) { }
 };


-template<bool is_weak>
+template<typename Map>
 static void TestPersistentValueMap() {
   LocalContext env;
   v8::Isolate* isolate = env->GetIsolate();
-  typedef v8::PersistentValueMap<int, v8::Object,
-      StdPersistentValueMapTraits<int, v8::Object, is_weak> > Map;
   Map map(isolate);
   v8::internal::GlobalHandles* global_handles =
       reinterpret_cast<v8::internal::Isolate*>(isolate)->global_handles();
@@ -3538,7 +3506,7 @@
     CHECK_EQ(1, static_cast<int>(map.Size()));
   }
CHECK_EQ(initial_handle_count + 1, global_handles->global_handles_count());
-  if (is_weak) {
+  if (map.IsWeak()) {
     reinterpret_cast<v8::internal::Isolate*>(isolate)->heap()->
         CollectAllGarbage(i::Heap::kAbortIncrementalMarkingMask);
   } else {
@@ -3550,8 +3518,13 @@


 TEST(PersistentValueMap) {
-  TestPersistentValueMap<false>();
-  TestPersistentValueMap<true>();
+  // Default case, w/o weak callbacks:
+  TestPersistentValueMap<v8::StdPersistentValueMap<int, v8::Object> >();
+
+  // Custom traits with weak callbacks:
+  typedef v8::StdPersistentValueMap<int, v8::Object,
+      WeakStdMapTraits<int, v8::Object> > WeakPersistentValueMap;
+  TestPersistentValueMap<WeakPersistentValueMap>();
 }


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to