Commit b49e0c6a authored by adamk@chromium.org's avatar adamk@chromium.org

Simplify and optimize ChildListMutationScope

https://bugs.webkit.org/show_bug.cgi?id=97352

Reviewed by Ryosuke Niwa.

ChildListMutationScope is one of the most complicated bits of
MutationObserver implementation. This patch aims to simplify it for
clarity and improve its performance (mostly by just doing less).

The big change is to remove the MutationAccumulatorRouter class,
replacing it with lifetime-management logic in ChildListMutationAccumulator
ChildListMutationScope is expected to call getOrCreate() in
its constructor, and each scope holds a RefPtr to the accumulator.
When the last scope holding such a RefPtr is destroyed,
ChildListMutationAccumulator's destructor enqueues the accumulated record.

This greatly reduces the number of lines of code, and condenses
two HashMaps into one. It also reduces hash lookups, which now
occur only on scope creation and when the refcount for a given
accumulator reaches 0 (previously, each childAdded and willRemoveChild
call could result in two hash lookups each).

There are some minor changes as well: the ChildListMutationAccumulator::clear()
method is gone, as it was doing more work than necessary;
DEFINE_STATIC_LOCAL is now used instead of hand-rolled static-management
code; ChildListMutationAccumulator::m_lastAdded is no longer a RefPtr, since it
always points at a Node that's already being ref'd by the accumulator.
Also various minor syntactic cleanups.

No new tests, no change in behavior.

* dom/ChildListMutationScope.cpp:
(WebCore::accumulatorMap): Reduced two maps to one, and manage its lifetime with DEFINE_STATIC_LOCAL.
(WebCore::ChildListMutationAccumulator::ChildListMutationAccumulator): Remove unnecessary call to clear() (which itself has been removed).
(WebCore::ChildListMutationAccumulator::~ChildListMutationAccumulator): Enqueue record if not empty at destruction, and have the accumulator
remove itself from the map.
(WebCore::ChildListMutationAccumulator::getOrCreate): Replaces half of MutationAccumulatorRouter's job.
(WebCore::ChildListMutationAccumulator::childAdded): Minor RefPtr usage improvements.
(WebCore::ChildListMutationAccumulator::isRemovedNodeInOrder): Simplify RefPtr syntax.
(WebCore::ChildListMutationAccumulator::willRemoveChild): Minor RefPtr usage improvements.
(WebCore::ChildListMutationAccumulator::enqueueMutationRecord): Replace call to clear() with clearing m_lastAdded,
since it's the only bit not cleared by the MutationRecord creation call. Also remove
isEmpty check and replace with asserts now that it's a private method.
(WebCore::ChildListMutationAccumulator::isEmpty): Added more assertions about emptiness.
* dom/ChildListMutationScope.h:
(WebCore):
(ChildListMutationAccumulator): Extract the inner class to make everything easier to read.
(WebCore::ChildListMutationScope::ChildListMutationScope): Store m_accumulator rather than m_target.
(WebCore::ChildListMutationScope::~ChildListMutationScope): ditto
(WebCore::ChildListMutationScope::childAdded): ditto
(WebCore::ChildListMutationScope::willRemoveChild): ditto
(ChildListMutationScope):
* html/HTMLElement.cpp: Remove unused ChildListMutationScope.h #include.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@129280 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent ca2554ec
2012-09-21 Adam Klein <adamk@chromium.org>
Simplify and optimize ChildListMutationScope
https://bugs.webkit.org/show_bug.cgi?id=97352
Reviewed by Ryosuke Niwa.
ChildListMutationScope is one of the most complicated bits of
MutationObserver implementation. This patch aims to simplify it for
clarity and improve its performance (mostly by just doing less).
The big change is to remove the MutationAccumulatorRouter class,
replacing it with lifetime-management logic in ChildListMutationAccumulator
ChildListMutationScope is expected to call getOrCreate() in
its constructor, and each scope holds a RefPtr to the accumulator.
When the last scope holding such a RefPtr is destroyed,
ChildListMutationAccumulator's destructor enqueues the accumulated record.
This greatly reduces the number of lines of code, and condenses
two HashMaps into one. It also reduces hash lookups, which now
occur only on scope creation and when the refcount for a given
accumulator reaches 0 (previously, each childAdded and willRemoveChild
call could result in two hash lookups each).
There are some minor changes as well: the ChildListMutationAccumulator::clear()
method is gone, as it was doing more work than necessary;
DEFINE_STATIC_LOCAL is now used instead of hand-rolled static-management
code; ChildListMutationAccumulator::m_lastAdded is no longer a RefPtr, since it
always points at a Node that's already being ref'd by the accumulator.
Also various minor syntactic cleanups.
No new tests, no change in behavior.
* dom/ChildListMutationScope.cpp:
(WebCore::accumulatorMap): Reduced two maps to one, and manage its lifetime with DEFINE_STATIC_LOCAL.
(WebCore::ChildListMutationAccumulator::ChildListMutationAccumulator): Remove unnecessary call to clear() (which itself has been removed).
(WebCore::ChildListMutationAccumulator::~ChildListMutationAccumulator): Enqueue record if not empty at destruction, and have the accumulator
remove itself from the map.
(WebCore::ChildListMutationAccumulator::getOrCreate): Replaces half of MutationAccumulatorRouter's job.
(WebCore::ChildListMutationAccumulator::childAdded): Minor RefPtr usage improvements.
(WebCore::ChildListMutationAccumulator::isRemovedNodeInOrder): Simplify RefPtr syntax.
(WebCore::ChildListMutationAccumulator::willRemoveChild): Minor RefPtr usage improvements.
(WebCore::ChildListMutationAccumulator::enqueueMutationRecord): Replace call to clear() with clearing m_lastAdded,
since it's the only bit not cleared by the MutationRecord creation call. Also remove
isEmpty check and replace with asserts now that it's a private method.
(WebCore::ChildListMutationAccumulator::isEmpty): Added more assertions about emptiness.
* dom/ChildListMutationScope.h:
(WebCore):
(ChildListMutationAccumulator): Extract the inner class to make everything easier to read.
(WebCore::ChildListMutationScope::ChildListMutationScope): Store m_accumulator rather than m_target.
(WebCore::ChildListMutationScope::~ChildListMutationScope): ditto
(WebCore::ChildListMutationScope::childAdded): ditto
(WebCore::ChildListMutationScope::willRemoveChild): ditto
(ChildListMutationScope):
* html/HTMLElement.cpp: Remove unused ChildListMutationScope.h #include.
2012-09-21 Chris Rogers <crogers@google.com>
BiquadFilterNode must take audio-rate parameter changes into account
......@@ -42,60 +42,54 @@
#include "StaticNodeList.h"
#include <wtf/HashMap.h>
#include <wtf/OwnPtr.h>
#include <wtf/StdLibExtras.h>
namespace WebCore {
// MutationAccumulator expects that all removals from a parent take place in order
// and precede any additions. If this is violated (i.e. because of code changes elsewhere
// in WebCore) it will likely result in both (a) ASSERTions failing, and (b) mutation records
// being enqueued for delivery before the outer-most scope closes.
class ChildListMutationScope::MutationAccumulator {
WTF_MAKE_NONCOPYABLE(MutationAccumulator);
public:
MutationAccumulator(PassRefPtr<Node>, PassOwnPtr<MutationObserverInterestGroup> observers);
~MutationAccumulator();
void childAdded(PassRefPtr<Node>);
void willRemoveChild(PassRefPtr<Node>);
void enqueueMutationRecord();
private:
void clear();
bool isEmpty();
bool isAddedNodeInOrder(Node*);
bool isRemovedNodeInOrder(Node*);
RefPtr<Node> m_target;
Vector<RefPtr<Node> > m_removedNodes;
Vector<RefPtr<Node> > m_addedNodes;
RefPtr<Node> m_previousSibling;
RefPtr<Node> m_nextSibling;
RefPtr<Node> m_lastAdded;
OwnPtr<MutationObserverInterestGroup> m_observers;
};
ChildListMutationScope::MutationAccumulator::MutationAccumulator(PassRefPtr<Node> target, PassOwnPtr<MutationObserverInterestGroup> observers)
typedef HashMap<Node*, ChildListMutationAccumulator*> AccumulatorMap;
static AccumulatorMap& accumulatorMap()
{
DEFINE_STATIC_LOCAL(AccumulatorMap, map, ());
return map;
}
ChildListMutationAccumulator::ChildListMutationAccumulator(PassRefPtr<Node> target, PassOwnPtr<MutationObserverInterestGroup> observers)
: m_target(target)
, m_lastAdded(0)
, m_observers(observers)
{
clear();
}
ChildListMutationScope::MutationAccumulator::~MutationAccumulator()
ChildListMutationAccumulator::~ChildListMutationAccumulator()
{
ASSERT(isEmpty());
if (!isEmpty())
enqueueMutationRecord();
accumulatorMap().remove(m_target.get());
}
inline bool ChildListMutationScope::MutationAccumulator::isAddedNodeInOrder(Node* child)
PassRefPtr<ChildListMutationAccumulator> ChildListMutationAccumulator::getOrCreate(Node* target)
{
AccumulatorMap::AddResult result = accumulatorMap().add(target, 0);
RefPtr<ChildListMutationAccumulator> accumulator;
if (!result.isNewEntry)
accumulator = result.iterator->second;
else {
accumulator = adoptRef(new ChildListMutationAccumulator(target, MutationObserverInterestGroup::createForChildListMutation(target)));
result.iterator->second = accumulator.get();
}
return accumulator.release();
}
inline bool ChildListMutationAccumulator::isAddedNodeInOrder(Node* child)
{
return isEmpty() || (m_lastAdded == child->previousSibling() && m_nextSibling == child->nextSibling());
}
void ChildListMutationScope::MutationAccumulator::childAdded(PassRefPtr<Node> prpChild)
void ChildListMutationAccumulator::childAdded(PassRefPtr<Node> prpChild)
{
RefPtr<Node> child = prpChild;
ASSERT(hasObservers());
ASSERT(isAddedNodeInOrder(child.get()));
if (!isAddedNodeInOrder(child.get()))
enqueueMutationRecord();
......@@ -105,21 +99,21 @@ void ChildListMutationScope::MutationAccumulator::childAdded(PassRefPtr<Node> pr
m_nextSibling = child->nextSibling();
}
m_addedNodes.append(child);
m_lastAdded = child;
m_lastAdded = child.get();
m_addedNodes.append(child.release());
}
inline bool ChildListMutationScope::MutationAccumulator::isRemovedNodeInOrder(Node* child)
inline bool ChildListMutationAccumulator::isRemovedNodeInOrder(Node* child)
{
return isEmpty() || m_nextSibling.get() == child;
return isEmpty() || m_nextSibling == child;
}
void ChildListMutationScope::MutationAccumulator::willRemoveChild(PassRefPtr<Node> prpChild)
void ChildListMutationAccumulator::willRemoveChild(PassRefPtr<Node> prpChild)
{
RefPtr<Node> child = prpChild;
ASSERT(hasObservers());
ASSERT(m_addedNodes.isEmpty() && isRemovedNodeInOrder(child.get()));
if (!m_addedNodes.isEmpty() || !isRemovedNodeInOrder(child.get()))
enqueueMutationRecord();
......@@ -130,114 +124,33 @@ void ChildListMutationScope::MutationAccumulator::willRemoveChild(PassRefPtr<Nod
} else
m_nextSibling = child->nextSibling();
m_removedNodes.append(child);
m_removedNodes.append(child.release());
}
void ChildListMutationScope::MutationAccumulator::enqueueMutationRecord()
void ChildListMutationAccumulator::enqueueMutationRecord()
{
if (isEmpty()) {
clear();
return;
}
m_observers->enqueueMutationRecord(MutationRecord::createChildList(
m_target, StaticNodeList::adopt(m_addedNodes), StaticNodeList::adopt(m_removedNodes), m_previousSibling.release(), m_nextSibling.release()));
clear();
}
ASSERT(hasObservers());
ASSERT(!isEmpty());
void ChildListMutationScope::MutationAccumulator::clear()
{
if (!m_removedNodes.isEmpty())
m_removedNodes.clear();
if (!m_addedNodes.isEmpty())
m_addedNodes.clear();
m_previousSibling = 0;
m_nextSibling = 0;
RefPtr<NodeList> addedNodes = StaticNodeList::adopt(m_addedNodes);
RefPtr<NodeList> removedNodes = StaticNodeList::adopt(m_removedNodes);
RefPtr<MutationRecord> record = MutationRecord::createChildList(m_target, addedNodes.release(), removedNodes.release(), m_previousSibling.release(), m_nextSibling.release());
m_observers->enqueueMutationRecord(record.release());
m_lastAdded = 0;
ASSERT(isEmpty());
}
bool ChildListMutationScope::MutationAccumulator::isEmpty()
{
return m_removedNodes.isEmpty() && m_addedNodes.isEmpty();
}
ChildListMutationScope::MutationAccumulationRouter* ChildListMutationScope::MutationAccumulationRouter::s_instance = 0;
ChildListMutationScope::MutationAccumulationRouter::MutationAccumulationRouter()
{
}
ChildListMutationScope::MutationAccumulationRouter::~MutationAccumulationRouter()
{
ASSERT(m_scopingLevels.isEmpty());
ASSERT(m_accumulations.isEmpty());
}
void ChildListMutationScope::MutationAccumulationRouter::initialize()
{
ASSERT(!s_instance);
s_instance = adoptPtr(new ChildListMutationScope::MutationAccumulationRouter).leakPtr();
}
ChildListMutationScope::MutationAccumulationRouter* ChildListMutationScope::MutationAccumulationRouter::instance()
{
if (!s_instance)
initialize();
return s_instance;
}
void ChildListMutationScope::MutationAccumulationRouter::childAdded(Node* target, Node* child)
{
HashMap<Node*, OwnPtr<ChildListMutationScope::MutationAccumulator> >::iterator iter = m_accumulations.find(target);
ASSERT(iter != m_accumulations.end());
if (iter == m_accumulations.end() || !iter->second)
return;
iter->second->childAdded(child);
}
void ChildListMutationScope::MutationAccumulationRouter::willRemoveChild(Node* target, Node* child)
{
HashMap<Node*, OwnPtr<ChildListMutationScope::MutationAccumulator> >::iterator iter = m_accumulations.find(target);
ASSERT(iter != m_accumulations.end());
if (iter == m_accumulations.end() || !iter->second)
return;
iter->second->willRemoveChild(child);
}
void ChildListMutationScope::MutationAccumulationRouter::incrementScopingLevel(Node* target)
bool ChildListMutationAccumulator::isEmpty()
{
ScopingLevelMap::AddResult result = m_scopingLevels.add(target, 1);
if (!result.isNewEntry) {
++(result.iterator->second);
return;
}
if (OwnPtr<MutationObserverInterestGroup> observers = MutationObserverInterestGroup::createForChildListMutation(target))
m_accumulations.set(target, adoptPtr(new ChildListMutationScope::MutationAccumulator(target, observers.release())));
bool result = m_removedNodes.isEmpty() && m_addedNodes.isEmpty();
#ifndef NDEBUG
else
m_accumulations.set(target, nullptr);
if (result) {
ASSERT(!m_previousSibling);
ASSERT(!m_nextSibling);
ASSERT(!m_lastAdded);
}
#endif
}
void ChildListMutationScope::MutationAccumulationRouter::decrementScopingLevel(Node* target)
{
ScopingLevelMap::iterator iter = m_scopingLevels.find(target);
ASSERT(iter != m_scopingLevels.end());
--(iter->second);
if (iter->second > 0)
return;
m_scopingLevels.remove(iter);
OwnPtr<MutationAccumulator> record(m_accumulations.take(target));
if (record)
record->enqueueMutationRecord();
return result;
}
} // namespace WebCore
......
......@@ -39,65 +39,70 @@
#include <wtf/HashMap.h>
#include <wtf/Noncopyable.h>
#include <wtf/OwnPtr.h>
#include <wtf/RefCounted.h>
namespace WebCore {
class MutationObserverInterestGroup;
// ChildListMutationAccumulator is not meant to be used directly; ChildListMutationScope is the public interface.
//
// ChildListMutationAccumulator expects that all removals from a parent take place in order
// and precede any additions. If this is violated (i.e. because of code changes elsewhere
// in WebCore) it will likely result in both (a) ASSERTions failing, and (b) mutation records
// being enqueued for delivery before the outer-most scope closes.
class ChildListMutationAccumulator : public RefCounted<ChildListMutationAccumulator> {
public:
static PassRefPtr<ChildListMutationAccumulator> getOrCreate(Node*);
~ChildListMutationAccumulator();
void childAdded(PassRefPtr<Node>);
void willRemoveChild(PassRefPtr<Node>);
bool hasObservers() const { return m_observers; }
private:
ChildListMutationAccumulator(PassRefPtr<Node>, PassOwnPtr<MutationObserverInterestGroup>);
void enqueueMutationRecord();
bool isEmpty();
bool isAddedNodeInOrder(Node*);
bool isRemovedNodeInOrder(Node*);
RefPtr<Node> m_target;
Vector<RefPtr<Node> > m_removedNodes;
Vector<RefPtr<Node> > m_addedNodes;
RefPtr<Node> m_previousSibling;
RefPtr<Node> m_nextSibling;
Node* m_lastAdded;
OwnPtr<MutationObserverInterestGroup> m_observers;
};
class ChildListMutationScope {
WTF_MAKE_NONCOPYABLE(ChildListMutationScope);
public:
explicit ChildListMutationScope(Node* target)
: m_target(target->document()->hasMutationObserversOfType(MutationObserver::ChildList) ? target : 0)
{
if (m_target)
MutationAccumulationRouter::instance()->incrementScopingLevel(m_target);
}
~ChildListMutationScope()
{
if (m_target)
MutationAccumulationRouter::instance()->decrementScopingLevel(m_target);
if (target->document()->hasMutationObserversOfType(MutationObserver::ChildList))
m_accumulator = ChildListMutationAccumulator::getOrCreate(target);
}
void childAdded(Node* child)
{
if (m_target)
MutationAccumulationRouter::instance()->childAdded(m_target, child);
if (m_accumulator && m_accumulator->hasObservers())
m_accumulator->childAdded(child);
}
void willRemoveChild(Node* child)
{
if (m_target)
MutationAccumulationRouter::instance()->willRemoveChild(m_target, child);
if (m_accumulator && m_accumulator->hasObservers())
m_accumulator->willRemoveChild(child);
}
private:
class MutationAccumulator;
class MutationAccumulationRouter {
WTF_MAKE_NONCOPYABLE(MutationAccumulationRouter);
public:
~MutationAccumulationRouter();
static MutationAccumulationRouter* instance();
void incrementScopingLevel(Node*);
void decrementScopingLevel(Node*);
void childAdded(Node* target, Node* child);
void willRemoveChild(Node* target, Node* child);
private:
MutationAccumulationRouter();
static void initialize();
typedef HashMap<Node*, unsigned> ScopingLevelMap;
ScopingLevelMap m_scopingLevels;
HashMap<Node*, OwnPtr<MutationAccumulator> > m_accumulations;
static MutationAccumulationRouter* s_instance;
};
Node* m_target;
RefPtr<ChildListMutationAccumulator> m_accumulator;
};
} // namespace WebCore
......
......@@ -30,7 +30,6 @@
#include "CSSPropertyNames.h"
#include "CSSValueKeywords.h"
#include "CSSValuePool.h"
#include "ChildListMutationScope.h"
#include "DOMSettableTokenList.h"
#include "DocumentFragment.h"
#include "Event.h"
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment