Add more oilpan collections support

Source/heap/Handle.h

 /*
  * Copyright (C) 2014 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 20 matching lines @@
 #ifndef Handle_h
 #define Handle_h
 
 #include "heap/Heap.h"
 #include "heap/ThreadState.h"
 #include "heap/Visitor.h"
 
 namespace WebCore {
 
 template<typename T> class Member;
+template<typename T> struct OffHeapCollectionTraceTrait;
 
 class PersistentNode {
 public:
-    explicit PersistentNode(TraceCallback trace) : m_trace(trace) { }
+    explicit PersistentNode(TraceCallback trace)
+        : m_trace(trace)
+#ifndef NDEBUG

[Mads Ager (chromium), 2014/01/15 08:48:17]

Thanks for adding verification; that should make finding bugs with corrupted
persistent chains easier! :)

We should be able to do that without the #ifndef NDEBUG code by just using the
TraceCallback member. Set it to 0 on destruction and check that it is non-zero?
You can add a protected method called isAlive to do that check to make it clear
what it means.

+        , m_alive(true)
+#endif
+    {
+    }
 
-    virtual ~PersistentNode() { }
+    virtual ~PersistentNode()
+    {
+#ifndef NDEBUG
+        ASSERT(m_alive);
+        m_alive = false;
+#endif
+    }
 
     // Ideally the trace method should be virtual and automatically dispatch
     // to the most specific implementation. However having a virtual method
     // on PersistentNode leads to too eager template instantiation with MSVC
     // which leads to include cycles.
     // Instead we call the constructor with a TraceCallback which knows the
     // type of the most specific child and calls trace directly. See
     // TraceMethodDelegate in Visitor.h for how this is done.
     void trace(Visitor* visitor)
     {
         m_trace(visitor, this);
     }
 
 protected:
     TraceCallback m_trace;
 
+#ifndef NDEBUG
+    bool m_alive;
+#endif
+
 private:
     PersistentNode* m_next;
     PersistentNode* m_prev;
 
     template<ThreadAffinity affinity, typename Owner> friend class PersistentBase;
     friend class PersistentAnchor;
     friend class ThreadState;
 };
 
 template<ThreadAffinity Affinity, typename Owner>
 class PersistentBase : public PersistentNode {
 public:
     ~PersistentBase()
     {
 #ifndef NDEBUG
+        ASSERT(m_alive);
+        ASSERT(m_next->m_alive);
+        ASSERT(m_prev->m_alive);
         m_threadState->checkThread();
 #endif
         m_next->m_prev = m_prev;
         m_prev->m_next = m_next;
     }
 
 protected:
     inline PersistentBase()
         : PersistentNode(TraceMethodDelegate<Owner, &Owner::trace>::trampoline)
 #ifndef NDEBUG
@@ skipping 40 matching lines @@
 #endif
 };
 
 // A dummy Persistent handle that ensures the list of persistents is never null.
 // This removes a test from a hot path.
 class PersistentAnchor : public PersistentNode {
 public:
     void trace(Visitor*) { }
 
 private:
-    virtual ~PersistentAnchor() { }
+    virtual ~PersistentAnchor()
+    {
+        ASSERT(m_next == this);
+        ASSERT(m_prev == this);
+    }
     PersistentAnchor() : PersistentNode(TraceMethodDelegate<PersistentAnchor, &PersistentAnchor::trace>::trampoline)
     {
         m_next = this;
         m_prev = this;
     }
 
     friend class ThreadState;
 };
 
 // Persistent handles are used to store pointers into the
@@ skipping 165 matching lines @@
 
     template<typename U>
     Member& operator=(U* other)
     {
         m_raw = other;
         return *this;
     }
 
     void swap(Member<T>& other) { std::swap(m_raw, other.m_raw); }
 
-protected:
     T* raw() const { return m_raw; }
 
+protected:

[haraken, 2014/01/15 04:48:35]

Don't we want private here?

[zerny-chromium, 2014/01/15 08:05:27]

WeakMember needs access to m_raw.

btw, most of the friends decls will go with Mads's latest CL.

[Mads Ager (chromium), 2014/01/15 08:48:17]

Drive-by answer. :-)

We don't need private here. Since we are using conservative scanning there is no
longer anything dangerous is getting hold of the raw pointer. Leaving it private
makes it simpler to write the comparison operators as well. I have a patch out
to rename this to get(), make it public and use it in the comparisons.

     T* m_raw;
 
     template<typename U> friend class Member;
     template<typename U> friend class Persistent;
     friend class Visitor;
     template<typename U> friend struct WTF::PtrHash;
     // FIXME: Uncomment when HeapObjectHash is moved.
     // friend struct HeapObjectHash<T>;
     friend struct ObjectAliveTrait<Member<T> >;
     template<bool x, bool y, bool z, typename U, typename V> friend struct CollectionBackingTraceTrait;
     template<typename U, typename V> friend bool operator==(const Member<U>&, const Persistent<V>&);

[Mads Ager (chromium), 2014/01/15 08:48:17]

Let's git rid of all of these operator friend declarations here. I'll do that in
my change to rename raw() to get() and fix comparison operators.

     template<typename U, typename V> friend bool operator!=(const Member<U>&, const Persistent<V>&);
     template<typename U, typename V> friend bool operator==(const Persistent<U>&, const Member<V>&);
     template<typename U, typename V> friend bool operator!=(const Persistent<U>&, const Member<V>&);
     template<typename U, typename V> friend bool operator==(const Member<U>&, const Member<V>&);
     template<typename U, typename V> friend bool operator!=(const Member<U>&, const Member<V>&);
 };
 
 template<typename T>
 class TraceTrait<Member<T> > {
 public:
@@ skipping 62 matching lines @@
     friend class Visitor;
     template<typename U> friend struct WTF::PtrHash;
     // FIXME: Uncomment when moving HeapObjectHash to trunk.
     // friend struct HeapObjectHash<T>;
     friend struct ObjectAliveTrait<WeakMember<T> >;
 };
 
 // Comparison operators between (Weak)Members and Persistents
 template<typename T, typename U> inline bool operator==(const Member<T>& a, const Member<U>& b) { return a.m_raw == b.m_raw; }
 template<typename T, typename U> inline bool operator!=(const Member<T>& a, const Member<U>& b) { return a.m_raw != b.m_raw; }
-template<typename T, typename U> inline bool operator==(const Member<T>& a, const Persistent<U>& b) { return a.m_raw == b.m_raw; }
-template<typename T, typename U> inline bool operator!=(const Member<T>& a, const Persistent<U>& b) { return a.m_raw != b.m_raw; }
-template<typename T, typename U> inline bool operator==(const Persistent<T>& a, const Member<U>& b) { return a.m_raw == b.m_raw; }
-template<typename T, typename U> inline bool operator!=(const Persistent<T>& a, const Member<U>& b) { return a.m_raw != b.m_raw; }
-template<typename T, typename U> inline bool operator==(const Persistent<T>& a, const Persistent<U>& b) { return a.m_raw == b.m_raw; }
-template<typename T, typename U> inline bool operator!=(const Persistent<T>& a, const Persistent<U>& b) { return a.m_raw != b.m_raw; }
+template<typename T, typename U> inline bool operator==(const Member<T>& a, const Persistent<U>& b) { return a.m_raw == b.raw(); }

[Mads Ager (chromium), 2014/01/15 08:48:17]

This is a complete mess. Let us just use a.get() == b.get() consistently for all
of these. I'll do that with my patch for the ones that are already in the
system.

+template<typename T, typename U> inline bool operator!=(const Member<T>& a, const Persistent<U>& b) { return a.m_raw != b.raw(); }
+template<typename T, typename U> inline bool operator==(const Persistent<T>& a, const Member<U>& b) { return a.raw() == b.m_raw; }
+template<typename T, typename U> inline bool operator!=(const Persistent<T>& a, const Member<U>& b) { return a.raw() != b.m_raw; }
+template<typename T, typename U> inline bool operator==(const Persistent<T>& a, const Persistent<U>& b) { return a.raw() == b.raw(); }
+template<typename T, typename U> inline bool operator!=(const Persistent<T>& a, const Persistent<U>& b) { return a.raw() != b.raw(); }
+
+template<typename Collection, ThreadAffinity Affinity = MainThreadOnly> class CollectionPersistent;
+
+// Used to inject correctly typed operator[] into CollectionPersistent when we are wrapping Vector.
+template<typename T> class IndexingBehavior { };
+
+template<typename T, size_t inlineCapacity>
+class IndexingBehavior<CollectionPersistent<Vector<T, inlineCapacity, WTF::DefaultAllocator> > > {
+    typedef CollectionPersistent<Vector<T, inlineCapacity, WTF::DefaultAllocator> > CollectionPersistentType;
+public:
+    T& operator[] (size_t i) { return (**static_cast<CollectionPersistentType*>(this))[i]; }
+    const T& operator[] (size_t i) const { return (**static_cast<const CollectionPersistentType*>(this))[i]; }
+};
+
+template<typename Collection, ThreadAffinity Affinity>
+class CollectionPersistent
+    : public PersistentBase<Affinity, CollectionPersistent<Collection, Affinity> >
+    , public IndexingBehavior<CollectionPersistent<Collection, Affinity> > {
+public:
+    typedef Collection CollectionType;
+    typedef typename Collection::iterator iterator;
+    typedef typename Collection::const_iterator const_iterator;
+
+    CollectionPersistent() { }
+    explicit CollectionPersistent(size_t size) : m_collection(Collection(size)) { }
+    explicit CollectionPersistent(const Collection& collection) : m_collection(collection) { }
+    CollectionPersistent& operator=(const Collection& collection) { m_collection = collection; return *this; }
+    Collection* operator->() { return &m_collection; }
+    const Collection* operator->() const { return &m_collection; }
+    Collection& operator*() { return m_collection; }
+    const Collection& operator*() const { return m_collection; }
+
+    void trace(Visitor* visitor)
+    {
+        OffHeapCollectionTraceTrait<Collection>::mark(visitor, m_collection);                                                                     }

[haraken, 2014/01/15 04:48:35]

Write } in the next line.

[Erik Corry, 2014/01/15 09:27:32]

Done.

+
+#if defined(TRACE_GC_MARKING) && TRACE_GC_MARKING
+    virtual const char* name() OVERRIDE
+    {
+        ASSERT(this == static_cast<PersistentNode*>(this));
+        const char* n = FieldAnnotationBase::fromAddress(this);
+        return n ? n : "CollectionPersistent";
+    }
+#endif
+
+private:
+    Collection m_collection;
+};
 
 } // namespace WebCore
 
 namespace WTF {
 
 template <typename T> struct VectorTraits<WebCore::Member<T> > : VectorTraitsBase<false, WebCore::Member<T> > {
     static const bool needsDestruction = false;
     static const bool canInitializeWithMemset = true;
     static const bool canMoveWithMemcpy = true;
 };
 
 template <typename T> struct VectorTraits<WebCore::WeakMember<T> > : VectorTraitsBase<false, WebCore::WeakMember<T> > {
     static const bool needsDestruction = false;
     static const bool canInitializeWithMemset = true;
     static const bool canMoveWithMemcpy = true;
 };
 
 template<typename T> struct HashTraits<WebCore::Member<T> > : SimpleClassHashTraits<WebCore::Member<T> > {
     static const bool needsDestruction = false;
     // FIXME: The distinction between PeekInType and PassInType is there for
     // the sake of the reference counting handles. When they are gone the two
     // types can be merged into PassInType.
     // FIXME: Implement proper const'ness for iterator types. Requires support
     // in the marking Visitor.
     typedef T* PeekInType;
     typedef T* PassInType;
-    typedef T* IteratorGetType;
-    typedef T* IteratorConstGetType;
+    typedef WebCore::Member<T>* IteratorGetType;
+    typedef const WebCore::Member<T>* IteratorConstGetType;
     typedef T* IteratorReferenceType;
     typedef T* IteratorConstReferenceType;
     static IteratorConstGetType getToConstGetConversion(const WebCore::Member<T>* x) { return x->raw(); }
-    static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return x; }
-    static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return x; }
+    static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return x->raw(); }
+    static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return x->raw(); }
     // FIXME: Similarly, there is no need for a distinction between PeekOutType
     // and PassOutType without reference counting.
     typedef T* PeekOutType;
     typedef T* PassOutType;
 
     template<typename U>
     static void store(const U& value, WebCore::Member<T>& storage) { storage = value; }
 
     static PeekOutType peek(const WebCore::Member<T>& value) { return value; }
     static PassOutType passOut(const WebCore::Member<T>& value) { return value; }
 };
 
 template<typename T> struct HashTraits<WebCore::WeakMember<T> > : SimpleClassHashTraits<WebCore::WeakMember<T> > {
     static const bool needsDestruction = false;
     // FIXME: The distinction between PeekInType and PassInType is there for
     // the sake of the reference counting handles. When they are gone the two
     // types can be merged into PassInType.
     // FIXME: Implement proper const'ness for iterator types. Requires support
     // in the marking Visitor.
     typedef T* PeekInType;
     typedef T* PassInType;
-    typedef T* IteratorGetType;
-    typedef T* IteratorConstGetType;
+    typedef WebCore::WeakMember<T>* IteratorGetType;
+    typedef const WebCore::WeakMember<T>* IteratorConstGetType;
     typedef T* IteratorReferenceType;
     typedef T* IteratorConstReferenceType;
     static IteratorConstGetType getToConstGetConversion(const WebCore::WeakMember<T>* x) { return x->raw(); }
-    static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return x; }
-    static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return x; }
+    static IteratorReferenceType getToReferenceConversion(IteratorGetType x) { return x->raw(); }
+    static IteratorConstReferenceType getToReferenceConstConversion(IteratorConstGetType x) { return x->raw(); }
     // FIXME: Similarly, there is no need for a distinction between PeekOutType
     // and PassOutType without reference counting.
     typedef T* PeekOutType;
     typedef T* PassOutType;
 
     template<typename U>
     static void store(const U& value, WebCore::WeakMember<T>& storage) { storage = value; }
 
     static PeekOutType peek(const WebCore::WeakMember<T>& value) { return value; }
     static PassOutType passOut(const WebCore::WeakMember<T>& value) { return value; }
@@ skipping 31 matching lines @@
 };
 
 template<typename Key, typename Value, typename Extractor, typename Traits, typename KeyTraits>
 struct IsWeak<WebCore::HeapHashTableBacking<Key, Value, Extractor, Traits, KeyTraits> > {
     static const bool value = Traits::isWeak;
 };
 
 } // namespace WTF
 
 #endif