JavaScriptCore:
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 3 Nov 2007 00:30:44 +0000 (00:30 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 3 Nov 2007 00:30:44 +0000 (00:30 +0000)
        Reviewed by Maciej.

        - http://bugs.webkit.org/show_bug.cgi?id=15807
          HashMap needs a take() function that combines get and remove

        * wtf/HashMap.h: Added take function. Simplistic implementation for now,
        but still does only one hash table lookup.

        * kjs/array_instance.cpp: (KJS::ArrayInstance::put): Use take rather than
        a find followed by a remove.

WebCore:

        Reviewed by Maciej.

        - use the new HashMap::take function where appropriate

        * bindings/js/kjs_binding.cpp:
        (KJS::addWrapper): Made an inline rather than a macro; inlines good, macros bad.
        (KJS::removeWrapper): Ditto.
        (KJS::removeWrappers): Ditto.
        (KJS::ScriptInterpreter::putDOMObject): Use the inline instead of the macro.
        (KJS::ScriptInterpreter::forgetDOMObject): Ditto. This involves using take instead
        of remove -- in theory ever so slightly less efficient, but I think it's fine.
        (KJS::ScriptInterpreter::forgetDOMNodeForDocument): Ditto.
        (KJS::ScriptInterpreter::putDOMNodeForDocument): Use the inline instead of the macro.
        (KJS::ScriptInterpreter::forgetAllDOMNodesForDocument): Use take instead of find/remove.
        (KJS::ScriptInterpreter::updateDOMNodeDocument): Use the inlines instead of the macros.

        * bindings/js/kjs_window.cpp: (KJS::Window::clearTimeout): Use take instead of find/remove.
        * bridge/mac/AXObjectCacheMac.mm: (WebCore::AXObjectCache::remove): Ditto.
        * page/AnimationController.cpp: (WebCore::AnimationControllerPrivate::clear): Ditto.
        * rendering/RenderBlock.cpp:
        (WebCore::RenderBlock::~RenderBlock): Ditto.
        (WebCore::RenderBlock::setDesiredColumnCountAndWidth): Ditto.
        * rendering/RootInlineBox.cpp: Ditto.(WebCore::RootInlineBox::detachEllipsisBox): Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@27385 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/array_instance.cpp
JavaScriptCore/wtf/HashMap.h
WebCore/ChangeLog
WebCore/bindings/js/kjs_binding.cpp
WebCore/bindings/js/kjs_window.cpp
WebCore/bridge/mac/AXObjectCacheMac.mm
WebCore/page/AnimationController.cpp
WebCore/rendering/RenderBlock.cpp
WebCore/rendering/RootInlineBox.cpp

index 67d43307ef6954a48af9ffa9c4021b4fed5f016e..f7369cd6819f1e3885ff213886e0c78c6e55e260 100644 (file)
@@ -1,3 +1,16 @@
+2007-11-02  Darin Adler  <darin@apple.com>
+
+        Reviewed by Maciej.
+
+        - http://bugs.webkit.org/show_bug.cgi?id=15807
+          HashMap needs a take() function that combines get and remove
+
+        * wtf/HashMap.h: Added take function. Simplistic implementation for now,
+        but still does only one hash table lookup.
+
+        * kjs/array_instance.cpp: (KJS::ArrayInstance::put): Use take rather than
+        a find followed by a remove.
+
 2007-11-02  David Carson  <dacarson@gmail.com>
 
         Reviewed by Darin.
index c4b0dfe292be383500fcfce33bd617595e6a2c47..ed2aa4d4484e67b8ecb141b7ad64a2c931f0905f 100644 (file)
@@ -281,15 +281,8 @@ void ArrayInstance::put(ExecState* exec, unsigned i, JSValue* value, int attribu
             storage->m_vector[j] = 0;
         map->remove(i);
     } else {
-        for (unsigned j = vectorLength; j < newVectorLength; ++j) {
-            SparseArrayValueMap::iterator it = map->find(j);
-            if (it == map->end())
-                storage->m_vector[j] = 0;
-            else {
-                storage->m_vector[j] = it->second;
-                map->remove(it);
-            }
-        }
+        for (unsigned j = vectorLength; j < newVectorLength; ++j)
+            storage->m_vector[j] = map->take(j);
     }
 
     storage->m_vector[i] = value;
index b31d94a46aae3cf4889f815b7ccb58c0dd3f28cd..3397d60f5f752e4976fefd82b7e96fd27947b497 100644 (file)
@@ -1,8 +1,6 @@
 // -*- mode: c++; c-basic-offset: 4 -*-
 /*
- * This file is part of the KDE libraries
- *
- * Copyright (C) 2005, 2006 Apple Computer, Inc.
+ * Copyright (C) 2005, 2006, 2007 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -97,9 +95,11 @@ namespace WTF {
         pair<iterator, bool> add(const KeyType&, const MappedType&); 
 
         void remove(const KeyType&);
-        void remove(iterator it);
+        void remove(iterator);
         void clear();
 
+        MappedType take(const KeyType&); // efficient combination of get with remove
+
     private:
         pair<iterator, bool> inlineAdd(const KeyType&, const MappedType&);
         void refAll();
@@ -324,6 +324,19 @@ namespace WTF {
         m_impl.clear();
     }
 
+    template<typename T, typename U, typename V, typename W, typename MappedTraits>
+    typename HashMap<T, U, V, W, MappedTraits>::MappedType
+    HashMap<T, U, V, W, MappedTraits>::take(const KeyType& key)
+    {
+        // This can probably be made more efficient to avoid ref/deref churn.
+        iterator it = find(key);
+        if (it == end())
+            return MappedTraits::emptyValue();
+        typename HashMap<T, U, V, W, MappedTraits>::MappedType result = it->second;
+        remove(it);
+        return result;
+    }
+
     template<typename T, typename U, typename V, typename W, typename X>
     bool operator==(const HashMap<T, U, V, W, X>& a, const HashMap<T, U, V, W, X>& b)
     {
index 3cb6026ff7713dd84a1d080c3a7a1b08f17e75d8..0d58f4e9eef3c7395345b9cb8f899ea2e18119c9 100644 (file)
@@ -1,3 +1,29 @@
+2007-11-02  Darin Adler  <darin@apple.com>
+
+        Reviewed by Maciej.
+
+        - use the new HashMap::take function where appropriate
+
+        * bindings/js/kjs_binding.cpp:
+        (KJS::addWrapper): Made an inline rather than a macro; inlines good, macros bad.
+        (KJS::removeWrapper): Ditto.
+        (KJS::removeWrappers): Ditto.
+        (KJS::ScriptInterpreter::putDOMObject): Use the inline instead of the macro.
+        (KJS::ScriptInterpreter::forgetDOMObject): Ditto. This involves using take instead
+        of remove -- in theory ever so slightly less efficient, but I think it's fine.
+        (KJS::ScriptInterpreter::forgetDOMNodeForDocument): Ditto.
+        (KJS::ScriptInterpreter::putDOMNodeForDocument): Use the inline instead of the macro.
+        (KJS::ScriptInterpreter::forgetAllDOMNodesForDocument): Use take instead of find/remove.
+        (KJS::ScriptInterpreter::updateDOMNodeDocument): Use the inlines instead of the macros.
+
+        * bindings/js/kjs_window.cpp: (KJS::Window::clearTimeout): Use take instead of find/remove.
+        * bridge/mac/AXObjectCacheMac.mm: (WebCore::AXObjectCache::remove): Ditto.
+        * page/AnimationController.cpp: (WebCore::AnimationControllerPrivate::clear): Ditto.
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::~RenderBlock): Ditto.
+        (WebCore::RenderBlock::setDesiredColumnCountAndWidth): Ditto.
+        * rendering/RootInlineBox.cpp: Ditto.(WebCore::RootInlineBox::detachEllipsisBox): Ditto.
+
 2007-11-02  Antti Koivisto  <antti@apple.com>
 
         Reviewed by Darin.
index 3902880f905e0c6303c41bd60be0a5aa126bef61..eb27fa989439423710394f2fbf272e9e7f9fd216 100644 (file)
@@ -1,5 +1,4 @@
 /*
- *  This file is part of the KDE libraries
  *  Copyright (C) 1999-2001 Harri Porten (porten@kde.org)
  *  Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
  *  Copyright (C) 2007 Samuel Weinig <sam@webkit.org>
@@ -66,17 +65,25 @@ typedef HashMap<Document*, NodeMap*> NodePerDocMap;
 // all are unregistered before they are destroyed. This has helped us fix at
 // least one bug.
 
+static void addWrapper(DOMObject* wrapper);
+static void removeWrapper(DOMObject* wrapper);
+static void removeWrappers(const NodeMap& wrappers);
+
 #ifdef NDEBUG
 
-#define ADD_WRAPPER(wrapper)
-#define REMOVE_WRAPPER(wrapper)
-#define REMOVE_WRAPPERS(wrappers)
+static inline void addWrapper(DOMObject*)
+{
+}
 
-#else
+static inline void removeWrapper(DOMObject*)
+{
+}
 
-#define ADD_WRAPPER(wrapper) addWrapper(wrapper)
-#define REMOVE_WRAPPER(wrapper) removeWrapper(wrapper)
-#define REMOVE_WRAPPERS(wrappers) removeWrappers(wrappers)
+static inline void removeWrappers(const NodeMap&)
+{
+}
+
+#else
 
 static HashSet<DOMObject*>& wrapperSet()
 {
@@ -148,14 +155,13 @@ DOMObject* ScriptInterpreter::getDOMObject(void* objectHandle)
 
 void ScriptInterpreter::putDOMObject(void* objectHandle, DOMObject* wrapper) 
 {
-    ADD_WRAPPER(wrapper);
+    addWrapper(wrapper);
     domObjects().set(objectHandle, wrapper);
 }
 
 void ScriptInterpreter::forgetDOMObject(void* objectHandle)
 {
-    REMOVE_WRAPPER(domObjects().get(objectHandle));
-    domObjects().remove(objectHandle);
+    removeWrapper(domObjects().take(objectHandle));
 }
 
 JSNode* ScriptInterpreter::getDOMNodeForDocument(Document* document, Node* node)
@@ -170,19 +176,18 @@ JSNode* ScriptInterpreter::getDOMNodeForDocument(Document* document, Node* node)
 
 void ScriptInterpreter::forgetDOMNodeForDocument(Document* document, Node* node)
 {
-    REMOVE_WRAPPER(getDOMNodeForDocument(document, node));
     if (!document) {
-        domObjects().remove(node);
+        removeWrapper(domObjects().take(node));
         return;
     }
     NodeMap* documentDict = domNodesPerDocument().get(document);
     if (documentDict)
-        documentDict->remove(node);
+        removeWrapper(documentDict->take(node));
 }
 
 void ScriptInterpreter::putDOMNodeForDocument(Document* document, Node* node, JSNode* wrapper)
 {
-    ADD_WRAPPER(wrapper);
+    addWrapper(wrapper);
     if (!document) {
         domObjects().set(node, wrapper);
         return;
@@ -198,12 +203,11 @@ void ScriptInterpreter::putDOMNodeForDocument(Document* document, Node* node, JS
 void ScriptInterpreter::forgetAllDOMNodesForDocument(Document* document)
 {
     ASSERT(document);
-    NodePerDocMap::iterator it = domNodesPerDocument().find(document);
-    if (it != domNodesPerDocument().end()) {
-        REMOVE_WRAPPERS(*it->second);
-        delete it->second;
-        domNodesPerDocument().remove(it);
-    }
+    NodeMap* map = domNodesPerDocument().take(document);
+    if (!map)
+        return;
+    removeWrappers(*map);
+    delete map;
 }
 
 void ScriptInterpreter::markDOMNodesForDocument(Document* doc)
@@ -241,10 +245,10 @@ void ScriptInterpreter::updateDOMNodeDocument(Node* node, Document* oldDoc, Docu
     ASSERT(oldDoc != newDoc);
     JSNode* wrapper = getDOMNodeForDocument(oldDoc, node);
     if (wrapper) {
-        REMOVE_WRAPPER(wrapper);
+        removeWrapper(wrapper);
         putDOMNodeForDocument(newDoc, node, wrapper);
         forgetDOMNodeForDocument(oldDoc, node);
-        ADD_WRAPPER(wrapper);
+        addWrapper(wrapper);
     }
 }
 
index eb735640570eb1e01643b2d4320462462d6e67d3..ce1f0919a470d9446c608bcf23f4c597ec0061ca 100644 (file)
@@ -1625,12 +1625,7 @@ void Window::clearTimeout(int timeoutId, bool delAction)
     if (timeoutId <= 0)
         return;
 
-    WindowPrivate::TimeoutsMap::iterator it = d->m_timeouts.find(timeoutId);
-    if (it == d->m_timeouts.end())
-        return;
-    DOMWindowTimer* timer = it->second;
-    d->m_timeouts.remove(it);
-    delete timer;
+    delete d->m_timeouts.take(timeoutId);
 }
 
 void Window::timerFired(DOMWindowTimer* timer)
index 9e790d3cc141e6c6337ce1f29295f4b175e7a2c9..61d0319da426b6dfb2dffce45fb3ba662a708dbc 100644 (file)
@@ -69,13 +69,11 @@ WebCoreAXObject* AXObjectCache::get(RenderObject* renderer)
 
 void AXObjectCache::remove(RenderObject* renderer)
 {
-    HashMap<RenderObject*, WebCoreAXObject*>::iterator it = m_objects.find(renderer);
-    if (it == m_objects.end())
+    WebCoreAXObject* obj = m_objects.take(renderer);
+    if (!obj)
         return;
-    WebCoreAXObject* obj = (*it).second;
     [obj detach];
     HardRelease(obj);
-    m_objects.remove(it);
 
     ASSERT(m_objects.size() >= m_idsInUse.size());
 }
index e36fbb4354aad93014e18d6d9a8184dbcffa2745..255feac7db7e9c3aadb64a2b8d50b061a63feac0 100644 (file)
@@ -493,16 +493,12 @@ CompositeImplicitAnimation* AnimationControllerPrivate::get(RenderObject* render
 
 bool AnimationControllerPrivate::clear(RenderObject* renderer)
 {
-    CompositeImplicitAnimation* animation = 0;
-    HashMap<RenderObject*, CompositeImplicitAnimation*>::iterator it = m_animations.find(renderer);
-    if (it != m_animations.end()) {
-        animation = it->second;
-        m_animations.remove(it);
-        animation->reset(renderer);
-        delete animation;
-        return true;
-    }
-    return false;
+    CompositeImplicitAnimation* animation = m_animations.take(renderer);
+    if (!animation)
+        return false;
+    animation->reset(renderer);
+    delete animation;
+    return true;
 }
 
 void AnimationControllerPrivate::updateTimer()
index 646ea7aa5dbfd6a21216d760bafa204fd3036332..771f675993c92d443fe5be81abb34b58bc7e100f 100644 (file)
@@ -116,11 +116,8 @@ RenderBlock::~RenderBlock()
     delete m_positionedObjects;
     delete m_maxMargin;
     
-    if (m_hasColumns) {
-        ColumnInfoMap::iterator it = gColumnInfoMap->find(this);
-        delete it->second;
-        gColumnInfoMap->remove(it);
-    }
+    if (m_hasColumns)
+        delete gColumnInfoMap->take(this);
 }
 
 void RenderBlock::setStyle(RenderStyle* _style)
@@ -3160,9 +3157,7 @@ void RenderBlock::setDesiredColumnCountAndWidth(int count, int width)
 {
     if (count == 1) {
         if (m_hasColumns) {
-            ColumnInfoMap::iterator it = gColumnInfoMap->find(this);
-            delete it->second;
-            gColumnInfoMap->remove(it);
+            delete gColumnInfoMap->take(this);
             m_hasColumns = false;
         }
     } else {
index 97b955ae6df8abb79ae15b3ccc3b670c58ee260a..ece36a7d0c684f8c1f8f66a2a56cf1f304843da8 100644 (file)
@@ -67,10 +67,9 @@ void RootInlineBox::destroy(RenderArena* arena)
 void RootInlineBox::detachEllipsisBox(RenderArena* arena)
 {
     if (m_hasEllipsisBox) {
-        EllipsisBoxMap::iterator it = gEllipsisBoxMap->find(this);
-        it->second->setParent(0);
-        it->second->destroy(arena);
-        gEllipsisBoxMap->remove(it);
+        EllipsisBox* box = gEllipsisBoxMap->take(this);
+        box->setParent(0);
+        box->destroy(arena);
         m_hasEllipsisBox = false;
     }
 }