Modified: trunk/Source/WebCore/ChangeLog (130102 => 130103)
--- trunk/Source/WebCore/ChangeLog 2012-10-02 00:14:33 UTC (rev 130102)
+++ trunk/Source/WebCore/ChangeLog 2012-10-02 00:16:05 UTC (rev 130103)
@@ -1,3 +1,37 @@
+2012-10-01 Adam Barth <aba...@webkit.org>
+
+ [V8] ScriptWrappable should hold the wrapper handle directly (Dromaeo/dom-traverse gets 4% faster)
+ https://bugs.webkit.org/show_bug.cgi?id=97974
+
+ Reviewed by Kentaro Hara.
+
+ Previously, we stored a pointer to a handle to a wrapper in Node. That
+ is an extra layer of indirection that slows down finding the wrapper
+ for the node. A handle is just a pointer, so we might as we just store
+ the handle in the Node directly. That speeds up dom-traverse by about 4%.
+
+ We were using the extra layer of indirection in IntrusiveDOMWrapperMap
+ to make removal more efficient. Rather than using a chunked table, we
+ now use a HashSet, which also lets us remove elements quickly.
+
+ * bindings/v8/IntrusiveDOMWrapperMap.h:
+ (WebCore::IntrusiveDOMWrapperMap::IntrusiveDOMWrapperMap):
+ (WebCore::IntrusiveDOMWrapperMap::get):
+ (WebCore::IntrusiveDOMWrapperMap::set):
+ (WebCore::IntrusiveDOMWrapperMap::contains):
+ (WebCore::IntrusiveDOMWrapperMap::visit):
+ (WebCore::IntrusiveDOMWrapperMap::removeIfPresent):
+ (WebCore::IntrusiveDOMWrapperMap::clear):
+ * bindings/v8/ScriptWrappable.h:
+ (WebCore::ScriptWrappable::ScriptWrappable):
+ (WebCore::ScriptWrappable::wrapper):
+ (WebCore::ScriptWrappable::setWrapper):
+ (WebCore::ScriptWrappable::disposeWrapper):
+ (WebCore::ScriptWrappable::reportMemoryUsage):
+ (ScriptWrappable):
+ * bindings/v8/V8DOMWrapper.h:
+ (WebCore::V8DOMWrapper::getCachedWrapper):
+
2012-10-01 Ojan Vafai <o...@chromium.org>
Unreviewed, rolling out r130062.
Modified: trunk/Source/WebCore/bindings/v8/IntrusiveDOMWrapperMap.h (130102 => 130103)
--- trunk/Source/WebCore/bindings/v8/IntrusiveDOMWrapperMap.h 2012-10-02 00:14:33 UTC (rev 130102)
+++ trunk/Source/WebCore/bindings/v8/IntrusiveDOMWrapperMap.h 2012-10-02 00:16:05 UTC (rev 130103)
@@ -34,196 +34,68 @@
#include "DOMDataStore.h"
#include "V8Node.h"
#include "WebCoreMemoryInstrumentation.h"
+#include <wtf/HashSet.h>
namespace WebCore {
-template <class T, int CHUNK_SIZE, class Traits>
-class ChunkedTable {
- public:
- ChunkedTable() : m_chunks(0), m_current(0), m_last(0) { }
-
- T* add(T element)
- {
- if (m_current == m_last) {
- m_chunks = new Chunk(m_chunks);
- m_current = m_chunks->m_entries;
- m_last = m_current + CHUNK_SIZE;
- }
- ASSERT((m_chunks->m_entries <= m_current) && (m_current < m_last));
- T* p = m_current++;
- *p = element;
- return p;
- }
-
- void remove(T* element)
- {
- ASSERT(element);
- ASSERT(m_current > m_chunks->m_entries);
- m_current--;
- if (element != m_current)
- Traits::move(element, m_current);
- if (m_current == m_chunks->m_entries) {
- Chunk* toDelete = m_chunks;
- m_chunks = toDelete->m_previous;
- m_current = m_last = m_chunks ? m_chunks->m_entries + CHUNK_SIZE : 0;
- delete toDelete;
- }
- ASSERT(!m_chunks || ((m_chunks->m_entries < m_current) && (m_current <= m_last)));
- }
-
- void clear()
- {
- if (!m_chunks)
- return;
-
- clearEntries(m_chunks->m_entries, m_current);
- Chunk* last = m_chunks;
- while (true) {
- Chunk* previous = last->m_previous;
- if (!previous)
- break;
- delete last;
- clearEntries(previous->m_entries, previous->m_entries + CHUNK_SIZE);
- last = previous;
- }
-
- m_chunks = last;
- m_current = m_chunks->m_entries;
- m_last = m_current + CHUNK_SIZE;
- }
-
- void visit(DOMDataStore* store, typename Traits::Visitor* visitor)
- {
- if (!m_chunks)
- return;
-
- visitEntries(store, m_chunks->m_entries, m_current, visitor);
- for (Chunk* chunk = m_chunks->m_previous; chunk; chunk = chunk->m_previous)
- visitEntries(store, chunk->m_entries, chunk->m_entries + CHUNK_SIZE, visitor);
- }
-
- void reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const
- {
- MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::Binding);
- for (Chunk* chunk = m_chunks; chunk; chunk = chunk->m_previous)
- info.addMember(chunk);
- }
-
- private:
- struct Chunk {
- explicit Chunk(Chunk* previous) : m_previous(previous) { }
- Chunk* const m_previous;
- T m_entries[CHUNK_SIZE];
- };
-
- static void clearEntries(T* first, T* last)
- {
- for (T* entry = first; entry < last; entry++)
- Traits::clear(entry);
- }
-
- static void visitEntries(DOMDataStore* store, T* first, T* last, typename Traits::Visitor* visitor)
- {
- for (T* entry = first; entry < last; entry++)
- Traits::visit(store, entry, visitor);
- }
-
- Chunk* m_chunks;
- T* m_current;
- T* m_last;
-};
-
-
class IntrusiveDOMWrapperMap : public AbstractWeakReferenceMap<Node, v8::Object> {
public:
IntrusiveDOMWrapperMap(v8::WeakReferenceCallback callback)
- : AbstractWeakReferenceMap<Node, v8::Object>(callback) { }
+ : AbstractWeakReferenceMap<Node, v8::Object>(callback)
+ {
+ }
- virtual v8::Persistent<v8::Object> get(Node* obj)
+ virtual v8::Persistent<v8::Object> get(Node* node)
{
- v8::Persistent<v8::Object>* wrapper = obj->wrapper();
- return wrapper ? *wrapper : v8::Persistent<v8::Object>();
+ return node->wrapper();
}
- virtual void set(Node* obj, v8::Persistent<v8::Object> wrapper)
+ virtual void set(Node* node, v8::Persistent<v8::Object> wrapper)
{
- ASSERT(obj);
- ASSERT(!obj->wrapper());
- v8::Persistent<v8::Object>* entry = m_table.add(wrapper);
- obj->setWrapper(entry);
- wrapper.MakeWeak(obj, weakReferenceCallback());
+ ASSERT(node && node->wrapper().IsEmpty());
+ m_nodes.add(node);
+ node->setWrapper(wrapper);
+ wrapper.MakeWeak(node, weakReferenceCallback());
}
- virtual bool contains(Node* obj)
+ virtual bool contains(Node* node)
{
- return obj->wrapper();
+ bool nodeHasWrapper = !node->wrapper().IsEmpty();
+ ASSERT(nodeHasWrapper == m_nodes.contains(node));
+ return nodeHasWrapper;
}
virtual void visit(DOMDataStore* store, Visitor* visitor)
{
- m_table.visit(store, visitor);
+ for (HashSet<Node*>::iterator it = m_nodes.begin(); it != m_nodes.end(); ++it)
+ visitor->visitDOMWrapper(store, *it, (*it)->wrapper());
}
- virtual bool removeIfPresent(Node* obj, v8::Persistent<v8::Object> value)
+ virtual bool removeIfPresent(Node* node, v8::Persistent<v8::Object> value)
{
- ASSERT(obj);
- v8::Persistent<v8::Object>* entry = obj->wrapper();
- if (!entry)
+ ASSERT(node);
+ v8::Persistent<v8::Object> wrapper = node->wrapper();
+ if (wrapper.IsEmpty())
return false;
- if (*entry != value)
- return false;
- obj->clearWrapper();
- m_table.remove(entry);
- value.Dispose();
+ ASSERT(wrapper == value);
+ node->disposeWrapper();
+ m_nodes.remove(node);
return true;
}
-
virtual void clear()
{
- m_table.clear();
+ m_nodes.clear();
}
virtual void reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const OVERRIDE
{
MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::Binding);
- info.addMember(m_table);
+ info.addMember(m_nodes);
}
private:
- static int const numberOfEntries = (1 << 10) - 1;
-
- struct ChunkedTableTraits {
- typedef IntrusiveDOMWrapperMap::Visitor Visitor;
-
- static void move(v8::Persistent<v8::Object>* target, v8::Persistent<v8::Object>* source)
- {
- *target = *source;
- Node* node = V8Node::toNative(*target);
- ASSERT(node);
- node->setWrapper(target);
- }
-
- static void clear(v8::Persistent<v8::Object>* entry)
- {
- Node* node = V8Node::toNative(*entry);
- ASSERT(node->wrapper() == entry);
-
- node->clearWrapper();
- entry->Dispose();
- }
-
- static void visit(DOMDataStore* store, v8::Persistent<v8::Object>* entry, Visitor* visitor)
- {
- Node* node = V8Node::toNative(*entry);
- ASSERT(node->wrapper() == entry);
-
- visitor->visitDOMWrapper(store, node, *entry);
- }
- };
-
- typedef ChunkedTable<v8::Persistent<v8::Object>, numberOfEntries, ChunkedTableTraits> Table;
- Table m_table;
+ HashSet<Node*> m_nodes;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/bindings/v8/ScriptWrappable.h (130102 => 130103)
--- trunk/Source/WebCore/bindings/v8/ScriptWrappable.h 2012-10-02 00:14:33 UTC (rev 130102)
+++ trunk/Source/WebCore/bindings/v8/ScriptWrappable.h 2012-10-02 00:16:05 UTC (rev 130103)
@@ -38,29 +38,36 @@
class ScriptWrappable {
public:
- ScriptWrappable() : m_wrapper(0) { }
+ ScriptWrappable()
+ {
+ }
- v8::Persistent<v8::Object>* wrapper() const
+ v8::Persistent<v8::Object> wrapper() const
{
return m_wrapper;
}
- void setWrapper(v8::Persistent<v8::Object>* wrapper)
+ void setWrapper(v8::Persistent<v8::Object> wrapper)
{
- ASSERT(wrapper);
+ ASSERT(!wrapper.IsEmpty());
m_wrapper = wrapper;
}
- void clearWrapper() { m_wrapper = 0; }
+ void disposeWrapper()
+ {
+ ASSERT(!m_wrapper.IsEmpty());
+ m_wrapper.Dispose();
+ m_wrapper.Clear();
+ }
void reportMemoryUsage(MemoryObjectInfo* memoryObjectInfo) const
{
MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::DOM);
- info.addWeakPointer(m_wrapper);
+ info.addWeakPointer(const_cast<v8::Persistent<v8::Object>*>(&m_wrapper));
}
private:
- v8::Persistent<v8::Object>* m_wrapper;
+ v8::Persistent<v8::Object> m_wrapper;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h (130102 => 130103)
--- trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h 2012-10-02 00:14:33 UTC (rev 130102)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h 2012-10-02 00:16:05 UTC (rev 130103)
@@ -113,18 +113,15 @@
{
ASSERT(isMainThread());
if (LIKELY(!DOMWrapperWorld::isolatedWorldsExist())) {
- v8::Persistent<v8::Object>* wrapper = node->wrapper();
- if (LIKELY(!!wrapper))
- return *wrapper;
+ v8::Persistent<v8::Object> wrapper = node->wrapper();
+ if (LIKELY(!wrapper.IsEmpty()))
+ return wrapper;
}
V8DOMWindowShell* context = V8DOMWindowShell::getEntered();
- if (LIKELY(!context)) {
- v8::Persistent<v8::Object>* wrapper = node->wrapper();
- if (!wrapper)
- return v8::Handle<v8::Object>();
- return *wrapper;
- }
+ if (LIKELY(!context))
+ return node->wrapper();
+
DOMDataStore* store = context->world()->domDataStore();
DOMNodeMapping& domNodeMap = node->isActiveNode() ? store->activeDomNodeMap() : store->domNodeMap();
return domNodeMap.get(node);
