Changeset 140194 in webkit


Ignore:
Timestamp:
Jan 18, 2013 12:38:10 PM (11 years ago)
Author:
ggaren@apple.com
Message:

Weak GC maps should be easier to use
​https://bugs.webkit.org/show_bug.cgi?id=107312

Reviewed by Sam Weinig.

../JavaScriptCore:

This patch changes WeakGCMap to not use a WeakImpl finalizer to remove
items from the map, and to instead have the map automatically remove
stale items itself upon insertion. This has a few advantages:

(1) WeakGCMap is now compatible with all the specializations you would
use for HashMap.

(2) There's no need for clients to write special finalization munging
functions.

(3) Clients can specify custom value finalizers if they like.

  • API/JSWeakObjectMapRefPrivate.cpp: Setter no longer requires a global

data, since we've reduced interdependency.

  • heap/Handle.h: No more need to forward declare, since we've reduced

interdependency.

  • heap/Weak.h:

(Weak): Use explicit so we can assign directly to a weak map iterator
without ambiguity between Weak<T> and PassWeak<T>.

  • runtime/Structure.cpp:

(JSC::StructureTransitionTable::add): See above.

  • runtime/Structure.h:

(JSC):

  • runtime/StructureTransitionTable.h:

(StructureTransitionTable): Bad code goes away, programmer happy.

  • runtime/WeakGCMap.h:

(JSC):
(WeakGCMap):
(JSC::WeakGCMap::WeakGCMap):
(JSC::WeakGCMap::set):
(JSC::WeakGCMap::add):
(JSC::WeakGCMap::find):
(JSC::WeakGCMap::contains):
(JSC::WeakGCMap::gcMap):
(JSC::WeakGCMap::gcMapIfNeeded): Inherit from HashMap and override any
function that might observe a Weak<T> that has died, just enough to
make such items appear as if they are not in the table.

../WebCore:

Since weak GC maps are so easy to use now, let's use them for the DOM
string cache.

  • WebCore.exp.in:
  • bindings/js/DOMWrapperWorld.cpp:

(WebCore):
(WebCore::DOMWrapperWorld::DOMWrapperWorld):

  • bindings/js/DOMWrapperWorld.h:

(WebCore):
(WebCore::DOMWrapperWorld::globalData):
(DOMWrapperWorld):

  • bindings/js/JSDOMBinding.cpp:

(WebCore):

  • bindings/js/JSDOMBinding.h:

(WebCore):
(WebCore::jsStringWithCache):

Location:
trunk/Source
Files:
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/API/JSWeakObjectMapRefPrivate.cpp

    r139541 r140194  
    6060        return;
    6161    ASSERT(obj->inherits(&JSCallbackObject<JSGlobalObject>::s_info) || obj->inherits(&JSCallbackObject<JSDestructibleObject>::s_info));
    62     map->map().set(exec->globalData(), key, obj);
     62    map->map().set(key, obj);
    6363}
    6464

  • trunk/Source/JavaScriptCore/ChangeLog

    r140186 r140194  
     12013-01-18  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Weak GC maps should be easier to use
     4        https://bugs.webkit.org/show_bug.cgi?id=107312
     5
     6        Reviewed by Sam Weinig.
     7
     8        This patch changes WeakGCMap to not use a WeakImpl finalizer to remove
     9        items from the map, and to instead have the map automatically remove
     10        stale items itself upon insertion. This has a few advantages:
     11
     12        (1) WeakGCMap is now compatible with all the specializations you would
     13        use for HashMap.
     14
     15        (2) There's no need for clients to write special finalization munging
     16        functions.
     17
     18        (3) Clients can specify custom value finalizers if they like.
     19
     20        * JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCoreExports.def: Def!
     21
     22        * API/JSWeakObjectMapRefPrivate.cpp: Setter no longer requires a global
     23        data, since we've reduced interdependency.
     24
     25        * heap/Handle.h: No more need to forward declare, since we've reduced
     26        interdependency.
     27
     28        * heap/Weak.h:
     29        (Weak): Use explicit so we can assign directly to a weak map iterator
     30        without ambiguity between Weak<T> and PassWeak<T>.
     31
     32        * runtime/Structure.cpp:
     33        (JSC::StructureTransitionTable::add): See above.
     34
     35        * runtime/Structure.h:
     36        (JSC):
     37        * runtime/StructureTransitionTable.h:
     38        (StructureTransitionTable): Bad code goes away, programmer happy.
     39
     40        * runtime/WeakGCMap.h:
     41        (JSC):
     42        (WeakGCMap):
     43        (JSC::WeakGCMap::WeakGCMap):
     44        (JSC::WeakGCMap::set):
     45        (JSC::WeakGCMap::add):
     46        (JSC::WeakGCMap::find):
     47        (JSC::WeakGCMap::contains):
     48        (JSC::WeakGCMap::gcMap):
     49        (JSC::WeakGCMap::gcMapIfNeeded): Inherit from HashMap and override any
     50        function that might observe a Weak<T> that has died, just enough to
     51        make such items appear as if they are not in the table.
     52
    1532013-01-18  Michael Saboff  <msaboff@apple.com>
    254

  • trunk/Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCoreExports.def

    r139259 r140194  
    115115    ?className@JSProxy@JSC@@KA?AVString@WTF@@PBVJSObject@2@@Z
    116116    ?clear@SourceProviderCache@JSC@@QAEXXZ
    117     ?clearRareData@JSGlobalObject@JSC@@CAXPAVJSCell@2@@Z
    118117    ?collate@Collator@WTF@@QBE?AW4Result@12@PB_WI0I@Z
    119118    ?collectAllGarbage@Heap@JSC@@QAEXXZ

  • trunk/Source/JavaScriptCore/heap/Handle.h

    r127191 r140194  
    4444template <> class Handle<JSValue>;
    4545
    46 // Forward declare WeakGCMap
    47 template<typename KeyType, typename MappedType, typename FinalizerCallback, typename HashArg, typename KeyTraitsArg> class WeakGCMap;
    48 
    4946class HandleBase {
    5047    template <typename T> friend class Weak;
    5148    friend class HandleSet;
    5249    friend struct JSCallbackObjectData;
    53     template <typename KeyType, typename MappedType, typename FinalizerCallback, typename HashArg, typename KeyTraitsArg> friend class WeakGCMap;
    5450
    5551public:

  • trunk/Source/JavaScriptCore/heap/Weak.h

    r134697 r140194  
    4141
    4242    Weak();
    43     Weak(std::nullptr_t);
    44     Weak(GetType, WeakHandleOwner* = 0, void* context = 0);
     43    explicit Weak(std::nullptr_t);
     44    explicit Weak(GetType, WeakHandleOwner* = 0, void* context = 0);
    4545
    4646    enum HashTableDeletedValueTag { HashTableDeletedValue };

  • trunk/Source/JavaScriptCore/runtime/Structure.cpp

    r139491 r140194  
    102102    // When either of the parameters are bitfields, the C++ compiler will try to bind them as lvalues, which is invalid. To work around this, use unary "+" to make the parameter an rvalue.
    103103    // See https://bugs.webkit.org/show_bug.cgi?id=59261 for more details
    104     map()->set(globalData, make_pair(structure->m_nameInPrevious, +structure->m_attributesInPrevious), structure);
     104    map()->set(make_pair(structure->m_nameInPrevious, +structure->m_attributesInPrevious), structure);
    105105}
    106106

  • trunk/Source/JavaScriptCore/runtime/Structure.h

    r139541 r140194  
    528528    }
    529529
    530     inline StructureTransitionTable::Hash::Key StructureTransitionTable::keyForWeakGCMapFinalizer(void*, Structure* structure)
    531     {
    532         // Newer versions of the STL have an std::make_pair function that takes rvalue references.
    533         // When either of the parameters are bitfields, the C++ compiler will try to bind them as lvalues, which is invalid. To work around this, use unary "+" to make the parameter an rvalue.
    534         // See https://bugs.webkit.org/show_bug.cgi?id=59261 for more details.
    535         return Hash::Key(structure->m_nameInPrevious.get(), +structure->m_attributesInPrevious);
    536     }
    537 
    538530    inline bool Structure::transitivelyTransitionedFrom(Structure* structureToFind)
    539531    {

  • trunk/Source/JavaScriptCore/runtime/StructureTransitionTable.h

    r133953 r140194  
    112112    };
    113113
    114     struct WeakGCMapFinalizerCallback {
    115         static void* finalizerContextFor(Hash::Key)
    116         {
    117             return 0;
    118         }
    119 
    120         static inline Hash::Key keyForFinalizer(void* context, Structure* structure)
    121         {
    122             return keyForWeakGCMapFinalizer(context, structure);
    123         }
    124     };
    125 
    126     typedef WeakGCMap<Hash::Key, Structure, WeakGCMapFinalizerCallback, Hash> TransitionMap;
    127 
    128     static Hash::Key keyForWeakGCMapFinalizer(void* context, Structure*);
     114    typedef WeakGCMap<Hash::Key, Structure, Hash> TransitionMap;
    129115
    130116public:

  • trunk/Source/JavaScriptCore/runtime/WeakGCMap.h

    r130612 r140194  
    2727#define WeakGCMap_h
    2828
    29 #include "Handle.h"
    30 #include "JSGlobalData.h"
     29#include "Weak.h"
    3130#include <wtf/HashMap.h>
    3231
    3332namespace JSC {
    3433
    35 // A HashMap for GC'd values that removes entries when the associated value
    36 // dies.
    37 template <typename KeyType, typename MappedType> struct DefaultWeakGCMapFinalizerCallback {
    38     static void* finalizerContextFor(KeyType key)
    39     {
    40         return reinterpret_cast<void*>(key);
    41     }
     34// A HashMap with Weak<JSCell> values, which automatically removes values once they're garbage collected.
    4235
    43     static KeyType keyForFinalizer(void* context, typename HandleTypes<MappedType>::ExternalType)
    44     {
    45         return reinterpret_cast<KeyType>(context);
    46     }
    47 };
    48 
    49 template<typename KeyType, typename MappedType, typename FinalizerCallback = DefaultWeakGCMapFinalizerCallback<KeyType, MappedType>, typename HashArg = typename DefaultHash<KeyType>::Hash, typename KeyTraitsArg = HashTraits<KeyType> >
    50 class WeakGCMap : private WeakHandleOwner {
    51     WTF_MAKE_FAST_ALLOCATED;
    52     WTF_MAKE_NONCOPYABLE(WeakGCMap);
    53 
    54     typedef HashMap<KeyType, WeakImpl*, HashArg, KeyTraitsArg> MapType;
    55     typedef typename HandleTypes<MappedType>::ExternalType ExternalType;
     36template<typename KeyArg, typename RawMappedArg, typename HashArg = typename DefaultHash<KeyArg>::Hash,
     37    typename KeyTraitsArg = HashTraits<KeyArg> >
     38class WeakGCMap : public HashMap<KeyArg, Weak<RawMappedArg>, HashArg, KeyTraitsArg> {
     39    typedef Weak<RawMappedArg> MappedType;
     40    typedef HashMap<KeyArg, MappedType, HashArg, KeyTraitsArg> Base;
     41    typedef WeakGCMap<KeyArg, RawMappedArg, HashArg, KeyTraitsArg> Self;
     42    typedef HashTraits<MappedType> MappedTraits;
     43    typedef typename MappedTraits::PassInType MappedPassInType;
    5644
    5745public:
     46    using typename Base::KeyType;
     47    using typename Base::AddResult;
     48    using typename Base::iterator;
     49    using typename Base::const_iterator;
     50    using Base::begin;
     51    using Base::end;
     52    using Base::size;
     53    using Base::remove;
     54
    5855    WeakGCMap()
     56        : m_gcThreshold(minGCThreshold)
    5957    {
    6058    }
    6159
    62     void clear()
     60    AddResult set(const KeyType& key, MappedPassInType value)
    6361    {
    64         typename MapType::iterator end = m_map.end();
    65         for (typename MapType::iterator ptr = m_map.begin(); ptr != end; ++ptr)
    66             WeakSet::deallocate(ptr->value);
    67         m_map.clear();
     62        gcMapIfNeeded();
     63        return Base::set(key, value);
    6864    }
    6965
    70     ExternalType get(const KeyType& key) const
     66    AddResult add(const KeyType& key, MappedPassInType value)
    7167    {
    72         WeakImpl* impl = m_map.get(key);
    73         if (!impl || impl->state() != WeakImpl::Live)
    74             return ExternalType();
    75         return HandleTypes<MappedType>::getFromSlot(const_cast<JSValue*>(&impl->jsValue()));
     68        gcMapIfNeeded();
     69        AddResult addResult = Base::add(key, value);
     70        if (!addResult.isNewEntry && !addResult.iterator->value) { // Found a zombie value.
     71            addResult.isNewEntry = true;
     72            addResult.iterator->value = value;
     73        }
     74        return addResult;
    7675    }
    7776
    78     void set(JSGlobalData& globalData, const KeyType& key, ExternalType value)
     77    iterator find(const KeyType& key)
    7978    {
    80         ASSERT_UNUSED(globalData, globalData.apiLock().currentThreadIsHoldingLock());
    81         typename MapType::AddResult result = m_map.add(key, 0);
    82         if (!result.isNewEntry)
    83             WeakSet::deallocate(result.iterator->value);
    84         result.iterator->value = WeakSet::allocate(value, this, FinalizerCallback::finalizerContextFor(key));
     79        iterator it = Base::find(key);
     80        iterator end = Base::end();
     81        if (it != end && !it->value) // Found a zombie value.
     82            return end;
     83        return it;
    8584    }
    8685
    87     void remove(const KeyType& key)
     86    const_iterator find(const KeyType& key) const
    8887    {
    89         WeakImpl* impl = m_map.take(key);
    90         if (!impl)
    91             return;
    92         WeakSet::deallocate(impl);
     88        return const_cast<Self*>(this)->find(key);
    9389    }
    9490
    95     ~WeakGCMap()
     91    template<typename T, typename HashTranslator> bool contains(const T& key) const
    9692    {
    97         clear();
    98     }
    99    
    100 private:
    101     virtual void finalize(Handle<Unknown> handle, void* context)
    102     {
    103         WeakImpl* impl = m_map.take(FinalizerCallback::keyForFinalizer(context, HandleTypes<MappedType>::getFromSlot(handle.slot())));
    104         ASSERT(impl);
    105         WeakSet::deallocate(impl);
     93        return find(key) != end();
    10694    }
    10795
    108     MapType m_map;
     96private:
     97    static const int minGCThreshold;
     98
     99    void gcMap()
     100    {
     101        Vector<KeyType, 4> zombies;
     102        iterator end = this->end();
     103        for (iterator it = begin(); it != end; ++it) {
     104            if (!it->value)
     105                zombies.append(it->key);
     106        }
     107        for (size_t i = 0; i < zombies.size(); ++i)
     108            remove(zombies[i]);
     109    }
     110
     111    void gcMapIfNeeded()
     112    {
     113        if (size() < m_gcThreshold)
     114            return;
     115
     116        gcMap();
     117        m_gcThreshold = std::max(minGCThreshold, size() * 2 - 1);
     118    }
     119
     120    int m_gcThreshold;
    109121};
     122
     123template<typename KeyArg, typename RawMappedArg, typename HashArg, typename KeyTraitsArg>
     124const int WeakGCMap<KeyArg, RawMappedArg, HashArg, KeyTraitsArg>::minGCThreshold = 3;
    110125
    111126} // namespace JSC

  • trunk/Source/WebCore/ChangeLog

    r140193 r140194  
     12013-01-18  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Weak GC maps should be easier to use
     4        https://bugs.webkit.org/show_bug.cgi?id=107312
     5
     6        Reviewed by Sam Weinig.
     7
     8        Since weak GC maps are so easy to use now, let's use them for the DOM
     9        string cache.
     10
     11        * WebCore.exp.in:
     12        * bindings/js/DOMWrapperWorld.cpp:
     13        (WebCore):
     14        (WebCore::DOMWrapperWorld::DOMWrapperWorld):
     15        * bindings/js/DOMWrapperWorld.h:
     16        (WebCore):
     17        (WebCore::DOMWrapperWorld::globalData):
     18        (DOMWrapperWorld):
     19        * bindings/js/JSDOMBinding.cpp:
     20        (WebCore):
     21        * bindings/js/JSDOMBinding.h:
     22        (WebCore):
     23        (WebCore::jsStringWithCache):
     24
    1252013-01-18  Tim Volodine  <timvolodine@chromium.org>
    226

  • trunk/Source/WebCore/WebCore.exp.in

    r140105 r140194  
    688688__ZN7WebCore25computeViewportAttributesENS_17ViewportArgumentsEiiifNS_7IntSizeE
    689689__ZN7WebCore25createCanonicalUUIDStringEv
    690 __ZN7WebCore25jsStringWithCacheSlowCaseEPN3JSC9ExecStateERN3WTF7HashMapIPNS3_10StringImplENS0_4WeakINS0_8JSStringEEENS3_7PtrHashIS6_EENS3_10HashTraitsIS6_EENSC_IS9_EEEES6_
    691690__ZN7WebCore26NetscapePlugInStreamLoader6createEPNS_5FrameEPNS_32NetscapePlugInStreamLoaderClientERKNS_15ResourceRequestE
    692691__ZN7WebCore26UserTypingGestureIndicator27processingUserTypingGestureEv

  • trunk/Source/WebCore/bindings/js/DOMWrapperWorld.cpp

    r118483 r140194  
    3131namespace WebCore {
    3232
    33 void JSStringOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
    34 {
    35     JSString* jsString = jsCast<JSString*>(handle.get().asCell());
    36     StringImpl* stringImpl = static_cast<StringImpl*>(context);
    37     weakRemove(m_world->m_stringCache, stringImpl, jsString);
    38 }
    39 
    4033DOMWrapperWorld::DOMWrapperWorld(JSC::JSGlobalData* globalData, bool isNormal)
    4134    : m_globalData(globalData)
    4235    , m_isNormal(isNormal)
    43     , m_stringWrapperOwner(this)
    4436{
    4537    JSGlobalData::ClientData* clientData = m_globalData->clientData;

  • trunk/Source/WebCore/bindings/js/DOMWrapperWorld.h

    r119341 r140194  
    3535
    3636typedef HashMap<void*, JSC::Weak<JSDOMWrapper> > DOMObjectWrapperMap;
    37 typedef HashMap<StringImpl*, JSC::Weak<JSC::JSString>, PtrHash<StringImpl*> > JSStringCache;
    38 
    39 class JSStringOwner : public JSC::WeakHandleOwner {
    40 public:
    41     JSStringOwner(DOMWrapperWorld*);
    42     virtual void finalize(JSC::Handle<JSC::Unknown>, void* context);
    43 
    44 private:
    45     DOMWrapperWorld* m_world;
    46 };
    47 
    48 inline JSStringOwner::JSStringOwner(DOMWrapperWorld* world)
    49     : m_world(world)
    50 {
    51 }
     37typedef JSC::WeakGCMap<StringImpl*, JSC::JSString, PtrHash<StringImpl*> > JSStringCache;
    5238
    5339class DOMWrapperWorld : public RefCounted<DOMWrapperWorld> {
    … …  
    7359
    7460    JSC::JSGlobalData* globalData() const { return m_globalData; }
    75     JSStringOwner* stringWrapperOwner() { return &m_stringWrapperOwner; }
    7661
    7762protected:
    … …  
    8267    HashSet<ScriptController*> m_scriptControllersWithWindowShells;
    8368    bool m_isNormal;
    84     JSStringOwner m_stringWrapperOwner;
    8569};
    8670

  • trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp

    r135009 r140194  
    5151}
    5252
    53 JSValue jsStringWithCacheSlowCase(ExecState* exec, JSStringCache& stringCache, StringImpl* stringImpl)
    54 {
    55     JSString* wrapper = JSC::jsString(exec, String(stringImpl));
    56     weakAdd(stringCache, stringImpl, PassWeak<JSString>(wrapper, currentWorld(exec)->stringWrapperOwner(), stringImpl));
    57     return wrapper;
    58 }
    59 
    6053JSValue jsStringOrNull(ExecState* exec, const String& s)
    6154{

  • trunk/Source/WebCore/bindings/js/JSDOMBinding.h

    r139541 r140194  
    4747#include <wtf/Forward.h>
    4848#include <wtf/Noncopyable.h>
     49#include <wtf/NullPtr.h>
    4950#include <wtf/Vector.h>
    5051
    … …  
    270271
    271272    JSC::JSValue jsStringWithCache(JSC::ExecState*, const String&);
    272     JSC::JSValue jsStringWithCacheSlowCase(JSC::ExecState*, JSStringCache&, StringImpl*);
    273273    JSC::JSValue jsString(JSC::ExecState*, const KURL&); // empty if the URL is null
    274274    inline JSC::JSValue jsStringWithCache(JSC::ExecState* exec, const AtomicString& s)
    … …  
    504504
    505505        JSStringCache& stringCache = currentWorld(exec)->m_stringCache;
    506         if (JSC::JSString* string = stringCache.get(stringImpl))
    507             return string;
    508 
    509         return jsStringWithCacheSlowCase(exec, stringCache, stringImpl);
     506        JSStringCache::AddResult addResult = stringCache.add(stringImpl, nullptr);
     507        if (addResult.isNewEntry)
     508            addResult.iterator->value = JSC::jsString(exec, String(stringImpl));
     509        return JSC::JSValue(addResult.iterator->value.get());
    510510    }
    511511
Note: See TracChangeset for help on using the changeset viewer.