Skip to content
Permalink
Browse files
Reduce usage of std::unique_ptr<HashSet> / std::unique_ptr<HashMap>
https://bugs.webkit.org/show_bug.cgi?id=239903

Reviewed by Yusuke Suzuki.

Reduce usage of std::unique_ptr<HashSet> / std::unique_ptr<HashMap> as HashSet / HashMap
are already essentially pointers. This avoids some unnecessary dereferencing.

* Source/JavaScriptCore/heap/Heap.cpp:
(JSC::Heap::addCoreConstraints):
* Source/JavaScriptCore/heap/Heap.h:
* Source/JavaScriptCore/heap/HeapInlines.h:
* Source/WebKit/WebProcess/WebCoreSupport/WebPasteboardOverrides.cpp:
(WebKit::WebPasteboardOverrides::addOverride):
(WebKit::WebPasteboardOverrides::removeOverride):
(WebKit::WebPasteboardOverrides::overriddenTypes):
(WebKit::WebPasteboardOverrides::getDataForOverride const):
* Source/WebKit/WebProcess/WebCoreSupport/WebPasteboardOverrides.h:
* Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp:
(WebCore::DatabaseTracker::recordCreatingDatabase):
(WebCore::DatabaseTracker::doneCreatingDatabase):
(WebCore::DatabaseTracker::creatingDatabase):
(WebCore::DatabaseTracker::recordDeletingDatabase):
(WebCore::DatabaseTracker::doneDeletingDatabase):
(WebCore::DatabaseTracker::isDeletingDatabase):
(WebCore::DatabaseTracker::canDeleteOrigin):
* Source/WebCore/Modules/webdatabase/DatabaseTracker.h:
* Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp:
(WebCore::PropertySetCSSStyleDeclaration::wrapForDeprecatedCSSOM):
(WebCore::StyleRuleCSSStyleDeclaration::didMutate):
(WebCore::InlineCSSStyleDeclaration::didMutate):
* Source/WebCore/css/PropertySetCSSStyleDeclaration.h:
* Source/WebCore/dom/MutationObserver.cpp:
(WebCore::MutationObserver::deliver):
* Source/WebCore/dom/MutationObserverRegistration.cpp:
(WebCore::MutationObserverRegistration::observedSubtreeNodeWillDetach):
(WebCore::MutationObserverRegistration::takeTransientRegistrations):
(WebCore::MutationObserverRegistration::isReachableFromOpaqueRoots const):
* Source/WebCore/dom/MutationObserverRegistration.h:
(WebCore::MutationObserverRegistration::hasTransientRegistrations const):
* Source/WebCore/platform/graphics/GlyphMetricsMap.h:
(WebCore::GlyphMetricsMap<T>::locatePageSlowCase):
* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::insertIntoTrackedRendererMaps):
(WebCore::removeFromTrackedRendererMaps):
(WebCore::removeBlockFromPercentageDescendantAndContainerMaps):
* Source/WebCore/style/StyleScope.cpp:
(WebCore::Style::Scope::updateActiveStyleSheets):
(WebCore::Style::Scope::activeStyleSheetsContains const):
* Source/WebCore/style/StyleScope.h:

Canonical link: https://commits.webkit.org/250141@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293637 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
cdumez committed Apr 29, 2022
1 parent 9af21f7 commit 1d2fb8c6bf04920850c3ebf5c72f53c8c599527d
Show file tree
Hide file tree
Showing 16 changed files with 73 additions and 104 deletions.
@@ -2845,9 +2845,9 @@ void Heap::addCoreConstraints()
visitor.appendUnbarriered(pair.key);
}

if (m_markListSet && m_markListSet->size()) {
if (!m_markListSet.isEmpty()) {
SetRootMarkReasonScope rootScope(visitor, RootMarkReason::ConservativeScan);
MarkedArgumentBufferBase::markLists(visitor, *m_markListSet);
MarkedArgumentBufferBase::markLists(visitor, m_markListSet);
}

{
@@ -778,7 +778,7 @@ class Heap {
size_t m_deprecatedExtraMemorySize { 0 };

ProtectCountSet m_protectedValues;
std::unique_ptr<HashSet<MarkedArgumentBufferBase*>> m_markListSet;
HashSet<MarkedArgumentBufferBase*> m_markListSet;
SentinelLinkedList<MarkedJSValueRefArray, BasicRawSentinelNode<MarkedJSValueRefArray>> m_markedJSValueRefArrays;

std::unique_ptr<MachineThreads> m_machineThreads;
@@ -208,9 +208,7 @@ inline void Heap::decrementDeferralDepthAndGCIfNeeded()

inline HashSet<MarkedArgumentBufferBase*>& Heap::markListSet()
{
if (!m_markListSet)
m_markListSet = makeUnique<HashSet<MarkedArgumentBufferBase*>>();
return *m_markListSet;
return m_markListSet;
}

inline void Heap::reportExtraMemoryAllocated(size_t size)
@@ -936,13 +936,10 @@ void DatabaseTracker::recordCreatingDatabase(const SecurityOriginData& origin, c
ASSERT(m_databaseGuard.isHeld());

// We don't use HashMap::ensure here to avoid making an isolated copy of the origin every time.
auto* nameSet = m_beingCreated.get(origin);
if (!nameSet) {
auto ownedSet = makeUnique<HashCountedSet<String>>();
nameSet = ownedSet.get();
m_beingCreated.add(origin.isolatedCopy(), WTFMove(ownedSet));
}
nameSet->add(name.isolatedCopy());
auto it = m_beingCreated.find(origin);
if (it == m_beingCreated.end())
it = m_beingCreated.add(origin.isolatedCopy(), HashCountedSet<String>()).iterator;
it->value.add(name.isolatedCopy());
}

void DatabaseTracker::doneCreatingDatabase(const SecurityOriginData& origin, const String& name)
@@ -955,7 +952,7 @@ void DatabaseTracker::doneCreatingDatabase(const SecurityOriginData& origin, con
if (iterator == m_beingCreated.end())
return;

auto& countedSet = *iterator->value;
auto& countedSet = iterator->value;
ASSERT(countedSet.contains(name));

if (countedSet.remove(name) && countedSet.isEmpty())
@@ -967,7 +964,7 @@ bool DatabaseTracker::creatingDatabase(const SecurityOriginData& origin, const S
ASSERT(m_databaseGuard.isHeld());

auto iterator = m_beingCreated.find(origin);
return iterator != m_beingCreated.end() && iterator->value->contains(name);
return iterator != m_beingCreated.end() && iterator->value.contains(name);
}

bool DatabaseTracker::canDeleteDatabase(const SecurityOriginData& origin, const String& name)
@@ -982,14 +979,11 @@ void DatabaseTracker::recordDeletingDatabase(const SecurityOriginData& origin, c
ASSERT(canDeleteDatabase(origin, name));

// We don't use HashMap::ensure here to avoid making an isolated copy of the origin every time.
auto* nameSet = m_beingDeleted.get(origin);
if (!nameSet) {
auto ownedSet = makeUnique<MemoryCompactRobinHoodHashSet<String>>();
nameSet = ownedSet.get();
m_beingDeleted.add(origin.isolatedCopy(), WTFMove(ownedSet));
}
ASSERT(!nameSet->contains(name));
nameSet->add(name.isolatedCopy());
auto it = m_beingDeleted.find(origin);
if (it == m_beingDeleted.end())
it = m_beingDeleted.add(origin.isolatedCopy(), MemoryCompactRobinHoodHashSet<String>()).iterator;
ASSERT(!it->value.contains(name));
it->value.add(name.isolatedCopy());
}

void DatabaseTracker::doneDeletingDatabase(const SecurityOriginData& origin, const String& name)
@@ -1001,23 +995,23 @@ void DatabaseTracker::doneDeletingDatabase(const SecurityOriginData& origin, con
if (iterator == m_beingDeleted.end())
return;

ASSERT(iterator->value->contains(name));
iterator->value->remove(name);
if (iterator->value->isEmpty())
ASSERT(iterator->value.contains(name));
iterator->value.remove(name);
if (iterator->value.isEmpty())
m_beingDeleted.remove(iterator);
}

bool DatabaseTracker::isDeletingDatabase(const SecurityOriginData& origin, const String& name)
{
ASSERT(m_databaseGuard.isHeld());
auto* nameSet = m_beingDeleted.get(origin);
return nameSet && nameSet->contains(name);
auto it = m_beingDeleted.find(origin);
return it != m_beingDeleted.end() && it->value.contains(name);
}

bool DatabaseTracker::canDeleteOrigin(const SecurityOriginData& origin)
{
ASSERT(m_databaseGuard.isHeld());
return !(isDeletingOrigin(origin) || m_beingCreated.get(origin));
return !(isDeletingOrigin(origin) || m_beingCreated.contains(origin));
}

bool DatabaseTracker::isDeletingOrigin(const SecurityOriginData& origin)
@@ -176,8 +176,8 @@ class DatabaseTracker {

DatabaseManagerClient* m_client { nullptr };

HashMap<SecurityOriginData, std::unique_ptr<HashCountedSet<String>>> m_beingCreated WTF_GUARDED_BY_LOCK(m_databaseGuard);
HashMap<SecurityOriginData, std::unique_ptr<MemoryCompactRobinHoodHashSet<String>>> m_beingDeleted WTF_GUARDED_BY_LOCK(m_databaseGuard);
HashMap<SecurityOriginData, HashCountedSet<String>> m_beingCreated WTF_GUARDED_BY_LOCK(m_databaseGuard);
HashMap<SecurityOriginData, MemoryCompactRobinHoodHashSet<String>> m_beingDeleted WTF_GUARDED_BY_LOCK(m_databaseGuard);
HashSet<SecurityOriginData> m_originsBeingDeleted WTF_GUARDED_BY_LOCK(m_databaseGuard);
bool isDeletingDatabaseOrOriginFor(const SecurityOriginData&, const String& name) WTF_REQUIRES_LOCK(m_databaseGuard);
void recordCreatingDatabase(const SecurityOriginData&, const String& name) WTF_REQUIRES_LOCK(m_databaseGuard);
@@ -329,14 +329,11 @@ RefPtr<DeprecatedCSSOMValue> PropertySetCSSStyleDeclaration::wrapForDeprecatedCS

// The map is here to maintain the object identity of the CSSValues over multiple invocations.
// FIXME: It is likely that the identity is not important for web compatibility and this code should be removed.
if (!m_cssomValueWrappers)
m_cssomValueWrappers = makeUnique<HashMap<CSSValue*, WeakPtr<DeprecatedCSSOMValue>>>();

auto& clonedValue = m_cssomValueWrappers->add(internalValue, WeakPtr<DeprecatedCSSOMValue>()).iterator->value;
auto& clonedValue = m_cssomValueWrappers.add(internalValue, WeakPtr<DeprecatedCSSOMValue>()).iterator->value;
if (clonedValue)
return clonedValue.get();

RefPtr<DeprecatedCSSOMValue> wrapper = internalValue->createDeprecatedCSSOMWrapper(*this);
auto wrapper = internalValue->createDeprecatedCSSOMWrapper(*this);
clonedValue = wrapper;
return wrapper;
}
@@ -397,7 +394,7 @@ void StyleRuleCSSStyleDeclaration::didMutate(MutationType type)
ASSERT(m_parentRule->parentStyleSheet());

if (type == PropertyChanged)
m_cssomValueWrappers = nullptr;
m_cssomValueWrappers.clear();

// Style sheet mutation needs to be signaled even if the change failed. willMutate*/didMutate* must pair.
m_parentRule->parentStyleSheet()->didMutateRuleFromCSSStyleDeclaration();
@@ -439,7 +436,7 @@ void InlineCSSStyleDeclaration::didMutate(MutationType type)
if (type == NoChanges)
return;

m_cssomValueWrappers = nullptr;
m_cssomValueWrappers.clear();

if (!m_parentElement)
return;
@@ -59,7 +59,7 @@ class PropertySetCSSStyleDeclaration : public CSSStyleDeclaration {
virtual CSSParserContext cssParserContext() const;

MutableStyleProperties* m_propertySet;
std::unique_ptr<HashMap<CSSValue*, WeakPtr<DeprecatedCSSOMValue>>> m_cssomValueWrappers;
HashMap<CSSValue*, WeakPtr<DeprecatedCSSOMValue>> m_cssomValueWrappers;

private:
void ref() override;
@@ -193,7 +193,7 @@ void MutationObserver::deliver()
// Calling takeTransientRegistrations() can modify m_registrations, so it's necessary
// to make a copy of the transient registrations before operating on them.
Vector<MutationObserverRegistration*, 1> transientRegistrations;
Vector<std::unique_ptr<HashSet<GCReachableRef<Node>>>, 1> nodesToKeepAlive;
Vector<HashSet<GCReachableRef<Node>>, 1> nodesToKeepAlive;
HashSet<GCReachableRef<Node>> pendingTargets;
pendingTargets.swap(m_pendingTargets);
for (auto& registration : m_registrations) {
@@ -68,26 +68,24 @@ void MutationObserverRegistration::observedSubtreeNodeWillDetach(Node& node)
node.registerTransientMutationObserver(*this);
m_observer->setHasTransientRegistration(node.document());

if (!m_transientRegistrationNodes) {
m_transientRegistrationNodes = makeUnique<HashSet<GCReachableRef<Node>>>();

if (m_transientRegistrationNodes.isEmpty()) {
ASSERT(!m_nodeKeptAlive);
m_nodeKeptAlive = &m_node; // Balanced in takeTransientRegistrations.
}
m_transientRegistrationNodes->add(node);
m_transientRegistrationNodes.add(node);
}

std::unique_ptr<HashSet<GCReachableRef<Node>>> MutationObserverRegistration::takeTransientRegistrations()
HashSet<GCReachableRef<Node>> MutationObserverRegistration::takeTransientRegistrations()
{
if (!m_transientRegistrationNodes) {
if (m_transientRegistrationNodes.isEmpty()) {
ASSERT(!m_nodeKeptAlive);
return nullptr;
return { };
}

for (auto& node : *m_transientRegistrationNodes)
for (auto& node : m_transientRegistrationNodes)
node->unregisterTransientMutationObserver(*this);

auto returnValue = WTFMove(m_transientRegistrationNodes);
auto returnValue = std::exchange(m_transientRegistrationNodes, { });

ASSERT(m_nodeKeptAlive);
m_nodeKeptAlive = nullptr; // Balanced in observeSubtreeNodeWillDetach.
@@ -118,10 +116,7 @@ bool MutationObserverRegistration::isReachableFromOpaqueRoots(JSC::AbstractSlotV
if (visitor.containsOpaqueRoot(root(m_node)))
return true;

if (!m_transientRegistrationNodes)
return false;

for (auto& node : *m_transientRegistrationNodes) {
for (auto& node : m_transientRegistrationNodes) {
if (visitor.containsOpaqueRoot(root(node)))
return true;
}
@@ -53,8 +53,8 @@ class MutationObserverRegistration : public CanMakeWeakPtr<MutationObserverRegis

void resetObservation(MutationObserverOptions, const MemoryCompactLookupOnlyRobinHoodHashSet<AtomString>& attributeFilter);
void observedSubtreeNodeWillDetach(Node&);
std::unique_ptr<HashSet<GCReachableRef<Node>>> takeTransientRegistrations();
bool hasTransientRegistrations() const { return m_transientRegistrationNodes && !m_transientRegistrationNodes->isEmpty(); }
HashSet<GCReachableRef<Node>> takeTransientRegistrations();
bool hasTransientRegistrations() const { return !m_transientRegistrationNodes.isEmpty(); }

bool shouldReceiveMutationFrom(Node&, MutationObserverOptionType, const QualifiedName* attributeName) const;
bool isSubtree() const { return m_options.contains(MutationObserverOptionType::Subtree); }
@@ -70,7 +70,7 @@ class MutationObserverRegistration : public CanMakeWeakPtr<MutationObserverRegis
Ref<MutationObserver> m_observer;
Node& m_node;
RefPtr<Node> m_nodeKeptAlive;
std::unique_ptr<HashSet<GCReachableRef<Node>>> m_transientRegistrationNodes;
HashSet<GCReachableRef<Node>> m_transientRegistrationNodes;
MutationObserverOptions m_options;
MemoryCompactLookupOnlyRobinHoodHashSet<AtomString> m_attributeFilter;
};
@@ -102,7 +102,7 @@ template<class T> class GlyphMetricsMap {

bool m_filledPrimaryPage { false };
GlyphMetricsPage m_primaryPage; // We optimize for the page that contains glyph indices 0-255.
std::unique_ptr<HashMap<int, std::unique_ptr<GlyphMetricsPage>>> m_pages;
HashMap<int, std::unique_ptr<GlyphMetricsPage>> m_pages;
};

template<> inline float GlyphMetricsMap<float>::unknownMetrics()
@@ -129,13 +129,9 @@ template<class T> typename GlyphMetricsMap<T>::GlyphMetricsPage& GlyphMetricsMap
return m_primaryPage;
}

if (!m_pages)
m_pages = makeUnique<HashMap<int, std::unique_ptr<GlyphMetricsPage>>>();

auto& page = m_pages->ensure(pageNumber, [] {
return *m_pages.ensure(pageNumber, [] {
return makeUnique<GlyphMetricsPage>(unknownMetrics());
}).iterator->value;
return *page;
}

} // namespace WebCore
@@ -95,7 +95,7 @@ struct SameSizeAsRenderBlock : public RenderBox {
static_assert(sizeof(RenderBlock) == sizeof(SameSizeAsRenderBlock), "RenderBlock should stay small");

typedef HashMap<const RenderBlock*, std::unique_ptr<TrackedRendererListHashSet>> TrackedDescendantsMap;
typedef HashMap<const RenderBox*, std::unique_ptr<HashSet<const RenderBlock*>>> TrackedContainerMap;
typedef HashMap<const RenderBox*, HashSet<const RenderBlock*>> TrackedContainerMap;

static TrackedDescendantsMap* percentHeightDescendantsMap;
static TrackedContainerMap* percentHeightContainerMap;
@@ -113,29 +113,26 @@ static void insertIntoTrackedRendererMaps(const RenderBlock& container, RenderBo

bool added = descendantSet->add(&descendant).isNewEntry;
if (!added) {
ASSERT(percentHeightContainerMap->get(&descendant));
ASSERT(percentHeightContainerMap->get(&descendant)->contains(&container));
#if ASSERT_ENABLED
auto it = percentHeightContainerMap->find(&descendant);
ASSERT(it != percentHeightContainerMap->end());
ASSERT(it->value.contains(&container));
#endif
return;
}

auto& containerSet = percentHeightContainerMap->ensure(&descendant, [] {
return makeUnique<HashSet<const RenderBlock*>>();
}).iterator->value;

ASSERT(!containerSet->contains(&container));
containerSet->add(&container);
auto& containerSet = percentHeightContainerMap->add(&descendant, HashSet<const RenderBlock*>()).iterator->value;
ASSERT(!containerSet.contains(&container));
containerSet.add(&container);
}

static void removeFromTrackedRendererMaps(RenderBox& descendant)
{
if (!percentHeightDescendantsMap)
return;

std::unique_ptr<HashSet<const RenderBlock*>> containerSet = percentHeightContainerMap->take(&descendant);
if (!containerSet)
return;

for (auto* container : *containerSet) {
HashSet<const RenderBlock*> containerSet = percentHeightContainerMap->take(&descendant);
for (auto* container : containerSet) {
// FIXME: Disabling this assert temporarily until we fix the layout
// bugs associated with positioned objects not properly cleared from
// their ancestor chain before being moved. See webkit bug 93766.
@@ -349,10 +346,10 @@ static void removeBlockFromPercentageDescendantAndContainerMaps(RenderBlock* blo
ASSERT(it != percentHeightContainerMap->end());
if (it == percentHeightContainerMap->end())
continue;
auto* containerSet = it->value.get();
ASSERT(containerSet->contains(block));
containerSet->remove(block);
if (containerSet->isEmpty())
auto& containerSet = it->value;
ASSERT(containerSet.contains(block));
containerSet.remove(block);
if (containerSet.isEmpty())
percentHeightContainerMap->remove(it);
}
}
@@ -500,7 +500,7 @@ void Scope::updateActiveStyleSheets(UpdateType updateType)

updateResolver(activeCSSStyleSheets, styleSheetChange.resolverUpdateType);

m_weakCopyOfActiveStyleSheetListForFastLookup = nullptr;
m_weakCopyOfActiveStyleSheetListForFastLookup.clear();
m_activeStyleSheets.swap(activeCSSStyleSheets);
m_styleSheetsForStyleSheetList.swap(activeStyleSheets);

@@ -588,12 +588,14 @@ const Vector<RefPtr<CSSStyleSheet>> Scope::activeStyleSheetsForInspector()

bool Scope::activeStyleSheetsContains(const CSSStyleSheet* sheet) const
{
if (!m_weakCopyOfActiveStyleSheetListForFastLookup) {
m_weakCopyOfActiveStyleSheetListForFastLookup = makeUnique<HashSet<const CSSStyleSheet*>>();
if (m_activeStyleSheets.isEmpty())
return false;

if (m_weakCopyOfActiveStyleSheetListForFastLookup.isEmpty()) {
for (auto& activeStyleSheet : m_activeStyleSheets)
m_weakCopyOfActiveStyleSheetListForFastLookup->add(activeStyleSheet.get());
m_weakCopyOfActiveStyleSheetListForFastLookup.add(activeStyleSheet.get());
}
return m_weakCopyOfActiveStyleSheetListForFastLookup->contains(sheet);
return m_weakCopyOfActiveStyleSheetListForFastLookup.contains(sheet);
}

void Scope::flushPendingSelfUpdate()
@@ -186,7 +186,7 @@ class Scope : public CanMakeWeakPtr<Scope> {

Timer m_pendingUpdateTimer;

mutable std::unique_ptr<HashSet<const CSSStyleSheet*>> m_weakCopyOfActiveStyleSheetListForFastLookup;
mutable HashSet<const CSSStyleSheet*> m_weakCopyOfActiveStyleSheetListForFastLookup;

// Track the currently loading top-level stylesheets needed for rendering.
// Sheets loaded using the @import directive are not included in this count.

0 comments on commit 1d2fb8c

Please sign in to comment.