Oilpan: Reduce sizeof(Persistent) to 16 byte

Source/platform/heap/PersistentNode.h

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef PersistentNode_h
+#define PersistentNode_h
+
+#include "platform/heap/ThreadState.h"
+#include "wtf/Assertions.h"
+#include "wtf/MainThread.h"
+#include "wtf/ThreadingPrimitives.h"
+
+namespace blink {
+
+class PersistentNode final {
+public:
+    PersistentNode()
+        : m_self(nullptr)
+        , m_trace(nullptr)
+    {
+        ASSERT(isFree());
+    }
+
+    ~PersistentNode()
+    {
+        // If you hit this assert, it means that the thread finished
+        // without clearing persistent handles that the thread created.
+        // We don't enable the assert for the main thread because the
+        // main thread finishes without clearing all persistent handles.
+        ASSERT(isMainThread() || isFree());
+    }
+
+    // It is dangrous to copy the PersistentNode because it breaks the
+    // free list.
+    PersistentNode& operator=(const PersistentNode& otherref) = delete;
+
+    // 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 tracePersistentNode(Visitor* visitor)
+    {
+        ASSERT(!isFree());
+        ASSERT(m_trace);
+        m_trace(visitor, m_self);
+    }
+
+    void initialize(void* self, TraceCallback trace)
+    {
+        ASSERT(isFree());
+        m_self = self;
+        m_trace = trace;
+    }
+
+    void setFreeNext(PersistentNode* node)
+    {
+        ASSERT(!node || node->isFree());
+        m_self = node;
+        m_trace = nullptr;
+        ASSERT(isFree());
+    }
+
+    PersistentNode* freeNext()
+    {
+        ASSERT(isFree());
+        PersistentNode* node = reinterpret_cast<PersistentNode*>(m_self);
+        ASSERT(!node || node->isFree());
+        return node;
+    }
+
+    bool isFree() const

[sof, 2015/06/30 09:19:39]

"free" feels a bit overloaded in this context - "unused" better?

[haraken, 2015/06/30 09:51:32]

Done.

+    {
+        return !m_trace;
+    }
+
+private:
+    // If this PersistentNode is in use:
+    //   - m_self points to the corresponding Persistent handle.
+    //   - m_trace points to the trace method.
+    // If this PersistentNode is freed:
+    //   - m_self points to the next freed PersistentNode.
+    //   - m_trace is nullptr.
+    void* m_self;
+    TraceCallback m_trace;
+};
+
+struct PersistentNodeSlots final {
+private:
+    static const int slotCount = 256;
+    PersistentNodeSlots* m_next;
+    PersistentNode m_slot[slotCount];
+    friend class PersistentRegion;
+};
+
+class PersistentRegion final {

[sof, 2015/06/30 09:19:39]

Could you add a comment outlining what this provides?

+public:
+    PersistentRegion()
+        : m_freeHead(nullptr)
+        , m_slots(nullptr)
+#if ENABLE(ASSERT)
+        , m_numberOfPersistents(0)
+#endif
+    {
+    }
+    ~PersistentRegion();
+
+    PersistentNode* allocatePersistentNode(void* self, TraceCallback trace)
+    {
+#if ENABLE(ASSERT)
+        ++m_numberOfPersistents;
+#endif
+        if (UNLIKELY(!m_freeHead))
+            ensurePersistentNodeSlots(self, trace);
+        ASSERT(m_freeHead);
+        PersistentNode* node = m_freeHead;
+        m_freeHead = m_freeHead->freeNext();
+        node->initialize(self, trace);
+        ASSERT(!node->isFree());
+        return node;
+    }
+    void ensurePersistentNodeSlots(void*, TraceCallback);

[sof, 2015/06/30 09:19:39]

make this private?

[haraken, 2015/06/30 09:51:32]

Done.

+    void freePersistentNode(PersistentNode* persistentNode)
+    {
+        ASSERT(m_numberOfPersistents > 0);
+        persistentNode->setFreeNext(m_freeHead);
+        m_freeHead = persistentNode;
+#if ENABLE(ASSERT)
+        --m_numberOfPersistents;
+#endif
+    }
+    void tracePersistentNodes(Visitor*);
+    int numberOfPersistents();
+
+private:
+    PersistentNode* m_freeHead;
+    PersistentNodeSlots* m_slots;
+#if ENABLE(ASSERT)
+    int m_numberOfPersistents;

[sof, 2015/06/30 09:19:39]

Did you consider caching this count outside of ENABLE(ASSERT)?

[haraken, 2015/06/30 09:51:32]

At first I did this, but it added extra overhead to allocate/free
(++m_numberOfPersistents & --m_numberOfPersistents). Removing it improved the
mirco-benchmark by 1.5%.

+#endif
+};
+
+class CrossThreadPersistentRegion final {
+public:
+    CrossThreadPersistentRegion() : m_persistentRegion(adoptPtr(new PersistentRegion)) { }
+
+    PersistentNode* allocatePersistentNode(void* self, TraceCallback trace)
+    {
+        MutexLocker lock(m_mutex);
+        return m_persistentRegion->allocatePersistentNode(self, trace);
+    }
+
+    void freePersistentNode(PersistentNode* persistentNode)
+    {
+        MutexLocker lock(m_mutex);
+        m_persistentRegion->freePersistentNode(persistentNode);
+    }
+
+    void tracePersistentNodes(Visitor* visitor)
+    {
+        MutexLocker lock(m_mutex);
+        m_persistentRegion->tracePersistentNodes(visitor);
+    }
+
+private:
+    // We don't make CrossThreadPersistentRegion inherit from PersistentRegion
+    // because we don't want to virtualize performance-sensitive methods
+    // such as PersistentRegion::allocate/freePersistentNode.
+    OwnPtr<PersistentRegion> m_persistentRegion;
+    Mutex m_mutex;
+};
+
+} // namespace blink
+
+#endif