Skip to content
  • darin@apple.com's avatar
    Cut down on double hashing and code needlessly using hash table iterators · 78bf2d4c
    darin@apple.com authored
    https://bugs.webkit.org/show_bug.cgi?id=120611
    
    Reviewed by Andreas Kling.
    
    Source/WebCore:
    
    Some of these changes are primarily code cleanup, but others could provide
    a small code size and speed improvement by avoiding extra hashing.
    
    * Modules/geolocation/Geolocation.cpp:
    (WebCore::Geolocation::Watchers::find): Use get instead of find.
    (WebCore::Geolocation::Watchers::remove): Use take instead of find.
    (WebCore::Geolocation::makeCachedPositionCallbacks): Use the return
    value from remove to avoid hashing twice.
    
    * Modules/webaudio/AudioContext.cpp:
    (WebCore::AudioContext::addAutomaticPullNode): Use the return value from
    add to avoid hashing twice.
    (WebCore::AudioContext::removeAutomaticPullNode): Use the return value
    from remove to avoid hashing twice.
    
    * Modules/webaudio/AudioNodeInput.cpp:
    (WebCore::AudioNodeInput::connect): Use the return value from add to avoid
    hashing twice.
    (WebCore::AudioNodeInput::disconnect): Use the return value from remove
    to avoid hashing twice.
    
    * Modules/webaudio/AudioParam.cpp:
    (WebCore::AudioParam::connect): Use the return value from add to avoid
    hashing twice.
    (WebCore::AudioParam::disconnect): Use the return value from remove to
    avoid hashing twice.
    
    * bridge/NP_jsobject.cpp:
    (ObjectMap::remove): Use remove instead of find/remove.
    
    * dom/Node.cpp:
    (WebCore::Node::~Node): Use the return value from remove instead of
    find/remove.
    
    * inspector/InspectorProfilerAgent.cpp:
    (WebCore::InspectorProfilerAgent::removeProfile): Remove needless
    calls to contains.
    
    * loader/DocumentLoader.cpp:
    (WebCore::DocumentLoader::removeSubresourceLoader): Use the return
    value from remove instead of find/remove.
    
    * loader/ResourceLoadScheduler.cpp:
    (WebCore::ResourceLoadScheduler::HostInformation::remove): Use the
    return value from remove to avoid hashing twice.
    
    * loader/appcache/ApplicationCacheGroup.cpp:
    (WebCore::ApplicationCacheGroup::disassociateDocumentLoader): Use
    remove instead of find/remove.
    (WebCore::ApplicationCacheGroup::cacheDestroyed): Removed a needless
    call to contains to avoid hashing twice. It's fine to do the check
    for an empty hash table unconditionally.
    
    * page/DOMWindow.cpp:
    (WebCore::addUnloadEventListener): Eliminated a local variable for clarity.
    (WebCore::removeUnloadEventListener): Ditto. Also use remove instead
    of find/remove.
    (WebCore::removeAllUnloadEventListeners): Ditto. Also use removeAll instead
    of find/removeAll.
    (WebCore::addBeforeUnloadEventListener): Ditto.
    (WebCore::removeBeforeUnloadEventListener): Ditto.
    (WebCore::removeAllBeforeUnloadEventListeners): Ditto.
    
    * page/FrameView.cpp:
    (WebCore::FrameView::removeViewportConstrainedObject): Use the return
    value from remove to avoid hashing twice.
    (WebCore::FrameView::removeScrollableArea): Use the return value from
    remove instead of find/remove.
    (WebCore::FrameView::containsScrollableArea): Use && instead of an if
    statement in a way that is idiomatic for this kind of function.
    
    * page/Page.cpp:
    (WebCore::Page::addRelevantRepaintedObject): Use the return value from
    remove instead of find/remove.
    
    * page/PageGroup.cpp:
    (WebCore::PageGroup::removeUserScriptsFromWorld): Use remove instead
    of find/remove.
    (WebCore::PageGroup::removeUserStyleSheetsFromWorld): Use the return
    value from remove instead of find/remove.
    
    * page/PerformanceUserTiming.cpp:
    (WebCore::clearPeformanceEntries): Removed a needless call to contains.
    
    * platform/graphics/DisplayRefreshMonitor.cpp:
    (WebCore::DisplayRefreshMonitor::removeClient): Use the return value
    from remove instead of find/remove.
    (WebCore::DisplayRefreshMonitorManager::displayDidRefresh): Use remove
    instead of find/remove.
    
    * platform/graphics/blackberry/LayerRenderer.cpp:
    (WebCore::LayerRenderer::removeLayer): Use the return value from remove
    instead of find/remove.
    
    * platform/win/WindowMessageBroadcaster.cpp:
    (WebCore::WindowMessageBroadcaster::removeListener): Use remove instead
    of find/remove. It's fine to do the check for an empty hash table unconditionally.
    
    * plugins/PluginDatabase.cpp:
    (WebCore::PluginDatabase::removeDisabledPluginFile): Use the return value
    from remove instead of find/remove.
    
    * rendering/style/StyleCustomFilterProgramCache.cpp:
    (WebCore::StyleCustomFilterProgramCache::lookup): Use get instead of find.
    (WebCore::StyleCustomFilterProgramCache::add): Use contains instead of find
    in an assertion.
    (WebCore::StyleCustomFilterProgramCache::remove): Use remove instead of
    find/remove.
    
    * svg/SVGCursorElement.cpp:
    (WebCore::SVGCursorElement::removeClient): Use the return value from remove
    instead of find/remove.
    
    * svg/SVGDocumentExtensions.cpp:
    (WebCore::SVGDocumentExtensions::removeResource): Removed an unneeded call
    to contains.
    (WebCore::SVGDocumentExtensions::removeAllTargetReferencesForElement): Use
    remove instead of find/remove. It's fine to do the check for an empty hash
    table unconditionally.
    (WebCore::SVGDocumentExtensions::removeAllElementReferencesForTarget): Use
    remove instead of find/remove. Also removed unhelpful assertions. One is
    already done by HashMap, and the other is just checking a basic invariant
    of every HashMap that doesn't need to be checked.
    
    * svg/graphics/SVGImageCache.cpp:
    (WebCore::SVGImageCache::removeClientFromCache): Removed an unneeded call
    to contains.
    
    * svg/properties/SVGAnimatedProperty.cpp:
    (WebCore::SVGAnimatedProperty::~SVGAnimatedProperty): Use the version of
    remove that takes an iterator rather than the one that takes a key, so
    we don't need to redo the hashing.
    
    Source/WebKit2:
    
    * Platform/CoreIPC/Connection.cpp:
    (CoreIPC::Connection::waitForMessage): Use take instead of find/remove.
    
    * UIProcess/WebPreferences.cpp:
    (WebKit::WebPreferences::removePageGroup): Use the return value from remove
    instead of find/remove.
    
    * WebProcess/Geolocation/GeolocationPermissionRequestManager.cpp:
    (WebKit::GeolocationPermissionRequestManager::cancelRequestForGeolocation):
    (WebKit::GeolocationPermissionRequestManager::didReceiveGeolocationPermissionDecision):
    Use take instead of find/remove.
    
    * WebProcess/Plugins/Netscape/NetscapePlugin.cpp:
    (WebKit::NetscapePlugin::frameDidFinishLoading): Use take instead of find/remove.
    (WebKit::NetscapePlugin::frameDidFail): Use take instead of find/remove.
    
    * WebProcess/WebPage/WebBackForwardListProxy.cpp:
    (WebKit::WebBackForwardListProxy::removeItem): Use take instead of find/remove.
    
    * WebProcess/WebPage/WebPage.cpp:
    (WebKit::WebPage::didFinishCheckingText): Use take instead of get/remove so we
    hash only once.
    (WebKit::WebPage::didCancelCheckingText): Ditto.
    (WebKit::WebPage::stopExtendingIncrementalRenderingSuppression): Use the return
    value from remove instead of contains/remove so we hash only once.
    
    Source/WTF:
    
    Double hashing is common in code that needs to combine a remove with some
    action to only be done if the code is removed. The only way to avoid it is
    to write code using find and a hash table iterator. To help with this, add
    a boolean return value to remove functions to indicate if anything was removed.
    
    Double hashing also happens in code that does a get followed by a remove.
    The take function is helpful in this case. To help with this, add a takeFirst
    funciton to ListHashSet.
    
    * wtf/HashCountedSet.h:
    (WTF::HashCountedSet::removeAll): Added a boolean return value, analogous to the one
    that the HashCountedSet::remove function already has.
    
    * wtf/HashMap.h:
    (WTF::HashMap::remove): Added a boolean return value, true if something was removed.
    * wtf/HashSet.h:
    (WTF::HashSet::remove): Ditto.
    * wtf/RefPtrHashMap.h:
    (WTF::RefPtrHashMap::remove): Ditto.
    
    * wtf/ListHashSet.h:
    (WTF::ListHashSet::takeFirst): Added.
    (WTF::ListHashSet::takeLast): Added.
    (WTF::ListHashSet::remove): Added a boolean return value, true if something was removed.
    
    * wtf/WTFThreadData.h:
    (JSC::IdentifierTable::remove): Use the new remove return value to get rid of most of
    the code in this function.
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@154967 268f45cc-cd09-0410-ab3c-d52691b4dbfc
    78bf2d4c