Changeset 84934 in webkit


Ignore:
Timestamp:
Apr 26, 2011 11:31:03 AM (13 years ago)
Author:
ggaren@apple.com
Message:

2011-04-25 Geoffrey Garen <ggaren@apple.com>

Reviewed by Oliver Hunt.

Nixed special finalizer handling for WebCore strings
https://bugs.webkit.org/show_bug.cgi?id=59425


SunSpider reports no change.


Not needed anymore, since weak handles have finalizers.

  • runtime/JSString.cpp: (JSC::JSString::resolveRope): (JSC::JSString::resolveRopeSlowCase): (JSC::JSString::outOfMemory): (JSC::JSString::substringFromRope): (JSC::JSString::replaceCharacter): Updated for removal of union.
  • runtime/JSString.h: (JSC::RopeBuilder::JSString): (JSC::RopeBuilder::~JSString): (JSC::RopeBuilder::appendStringInConstruct): (JSC::RopeBuilder::appendValueInConstructAndIncrementLength): No need for union or special constructor anymore.

2011-04-25 Geoffrey Garen <ggaren@apple.com>

Reviewed by Oliver Hunt.

Nixed special finalizer handling for WebCore strings
https://bugs.webkit.org/show_bug.cgi?id=59425

Not needed anymore, since weak handles have finalizers.

  • WebCore.exp.in: Exports!
  • bindings/js/DOMWrapperWorld.cpp: (WebCore::JSStringOwner::finalize): (WebCore::DOMWrapperWorld::DOMWrapperWorld): Use a weak handle finalizer, so we don't need special treatment anymore.
  • bindings/js/DOMWrapperWorld.h: (WebCore::JSStringOwner::JSStringOwner): (WebCore::DOMWrapperWorld::stringWrapperOwner): Use a HashMap of Weak<T> instead of a WeakGCMap, so we can specify a custom finalizer.
  • bindings/js/JSDOMBinding.cpp: (WebCore::jsStringSlowCase):
  • bindings/js/JSDOMBinding.h: (WebCore::jsString): Updated for string map change.
Location:
trunk/Source
Files:
9 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r84911 r84934  
     12011-04-25  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Reviewed by Oliver Hunt.
     4
     5        Nixed special finalizer handling for WebCore strings
     6        https://bugs.webkit.org/show_bug.cgi?id=59425
     7       
     8        SunSpider reports no change.
     9       
     10        Not needed anymore, since weak handles have finalizers.
     11
     12        * runtime/JSString.cpp:
     13        (JSC::JSString::resolveRope):
     14        (JSC::JSString::resolveRopeSlowCase):
     15        (JSC::JSString::outOfMemory):
     16        (JSC::JSString::substringFromRope):
     17        (JSC::JSString::replaceCharacter): Updated for removal of union.
     18
     19        * runtime/JSString.h:
     20        (JSC::RopeBuilder::JSString):
     21        (JSC::RopeBuilder::~JSString):
     22        (JSC::RopeBuilder::appendStringInConstruct):
     23        (JSC::RopeBuilder::appendValueInConstructAndIncrementLength): No need for
     24        union or special constructor anymore.
     25
    1262011-04-26  Gabor Loki  <loki@webkit.org>
    227
  • trunk/Source/JavaScriptCore/runtime/JSString.cpp

    r84800 r84934  
    4949    }
    5050
    51     RopeImpl::Fiber currentFiber = m_other.m_fibers[0];
     51    RopeImpl::Fiber currentFiber = m_fibers[0];
    5252
    5353    if ((m_fiberCount > 2) || (RopeImpl::isRope(currentFiber))
    54         || ((m_fiberCount == 2) && (RopeImpl::isRope(m_other.m_fibers[1])))) {
     54        || ((m_fiberCount == 2) && (RopeImpl::isRope(m_fibers[1])))) {
    5555        resolveRopeSlowCase(exec, buffer);
    5656        return;
     
    6464    if (m_fiberCount > 1) {
    6565        position += length;
    66         currentFiber = m_other.m_fibers[1];
     66        currentFiber = m_fibers[1];
    6767        string = static_cast<StringImpl*>(currentFiber);
    6868        length = string->length();
     
    7373    ASSERT((buffer + m_length) == position);
    7474    for (unsigned i = 0; i < m_fiberCount; ++i) {
    75         RopeImpl::deref(m_other.m_fibers[i]);
    76         m_other.m_fibers[i] = 0;
     75        RopeImpl::deref(m_fibers[i]);
     76        m_fibers[i] = 0;
    7777    }
    7878    m_fiberCount = 0;
     
    101101    RopeImpl::Fiber currentFiber;
    102102    for (unsigned i = 0; i < (m_fiberCount - 1); ++i)
    103         workQueue.append(m_other.m_fibers[i]);
    104     currentFiber = m_other.m_fibers[m_fiberCount - 1];
     103        workQueue.append(m_fibers[i]);
     104    currentFiber = m_fibers[m_fiberCount - 1];
    105105    while (true) {
    106106        if (RopeImpl::isRope(currentFiber)) {
     
    123123                ASSERT(buffer == position);
    124124                for (unsigned i = 0; i < m_fiberCount; ++i) {
    125                     RopeImpl::deref(m_other.m_fibers[i]);
    126                     m_other.m_fibers[i] = 0;
     125                    RopeImpl::deref(m_fibers[i]);
     126                    m_fibers[i] = 0;
    127127                }
    128128                m_fiberCount = 0;
     
    142142{
    143143    for (unsigned i = 0; i < m_fiberCount; ++i) {
    144         RopeImpl::deref(m_other.m_fibers[i]);
    145         m_other.m_fibers[i] = 0;
     144        RopeImpl::deref(m_fibers[i]);
     145        m_fibers[i] = 0;
    146146    }
    147147    m_fiberCount = 0;
     
    170170
    171171    RopeIterator end;
    172     for (RopeIterator it(m_other.m_fibers.data(), m_fiberCount); it != end; ++it) {
     172    for (RopeIterator it(m_fibers.data(), m_fiberCount); it != end; ++it) {
    173173        ++fiberCount;
    174174        StringImpl* fiberString = *it;
     
    221221    StringImpl* matchString = 0;
    222222    size_t matchPosition = notFound;
    223     for (RopeIterator it(m_other.m_fibers.data(), m_fiberCount); it != end; ++it) {
     223    for (RopeIterator it(m_fibers.data(), m_fiberCount); it != end; ++it) {
    224224        ++fiberCount;
    225225        if (matchString)
     
    240240        return throwOutOfMemoryError(exec);
    241241
    242     for (RopeIterator it(m_other.m_fibers.data(), m_fiberCount); it != end; ++it) {
     242    for (RopeIterator it(m_fibers.data(), m_fiberCount); it != end; ++it) {
    243243        StringImpl* string = *it;
    244244        if (string != matchString) {
  • trunk/Source/JavaScriptCore/runtime/JSString.h

    r84800 r84934  
    6060    JSString* jsOwnedString(ExecState*, const UString&);
    6161
    62     typedef void (*JSStringFinalizerCallback)(JSString*, void* context);
    63     JSString* jsStringWithFinalizer(ExecState*, const UString&, JSStringFinalizerCallback callback, void* context);
    64 
    6562    class JS_EXPORTCLASS JSString : public JSCell {
    6663    public:
     
    9491                if (jsString->isRope()) {
    9592                    for (unsigned i = 0; i < jsString->m_fiberCount; ++i)
    96                         append(jsString->m_other.m_fibers[i]);
     93                        append(jsString->m_fibers[i]);
    9794                } else
    9895                    append(jsString->string());
     
    217214            , m_fiberCount(1)
    218215        {
    219             m_other.m_fibers[0] = rope.leakRef();
     216            m_fibers[0] = rope.leakRef();
    220217        }
    221218        // This constructor constructs a new string by concatenating s1 & s2.
     
    299296        }
    300297
    301         JSString(JSGlobalData* globalData, const UString& value, JSStringFinalizerCallback finalizer, void* context)
    302             : JSCell(*globalData, globalData->stringStructure.get())
    303             , m_length(value.length())
    304             , m_value(value)
    305             , m_fiberCount(0)
    306         {
    307             ASSERT(!m_value.isNull());
    308             // nasty hack because we can't union non-POD types
    309             m_other.m_finalizerCallback = finalizer;
    310             m_other.m_finalizerContext = context;
    311             Heap::heap(this)->reportExtraMemoryCost(value.impl()->cost());
    312         }
    313 
    314298        ~JSString()
    315299        {
    316300            ASSERT(vptr() == JSGlobalData::jsStringVPtr);
    317             if (!m_fiberCount) {
    318                 if (m_other.m_finalizerCallback)
    319                     m_other.m_finalizerCallback(this, m_other.m_finalizerContext);
    320             } else {
    321                 unsigned i = 0;
    322                 do
    323                     RopeImpl::deref(m_other.m_fibers[i]);
    324                 while (++i < m_fiberCount);
    325             }
     301            for (unsigned i = 0; i < m_fiberCount; ++i)
     302                RopeImpl::deref(m_fibers[i]);
    326303        }
    327304
     
    372349            StringImpl* impl = string.impl();
    373350            impl->ref();
    374             m_other.m_fibers[index++] = impl;
     351            m_fibers[index++] = impl;
    375352        }
    376353
     
    379356            if (jsString->isRope()) {
    380357                for (unsigned i = 0; i < jsString->m_fiberCount; ++i) {
    381                     RopeImpl::Fiber fiber = jsString->m_other.m_fibers[i];
     358                    RopeImpl::Fiber fiber = jsString->m_fibers[i];
    382359                    fiber->ref();
    383                     m_other.m_fibers[index++] = fiber;
     360                    m_fibers[index++] = fiber;
    384361                }
    385362            } else
     
    399376                StringImpl* impl = u.impl();
    400377                impl->ref();
    401                 m_other.m_fibers[index++] = impl;
     378                m_fibers[index++] = impl;
    402379                m_length += u.length();
    403380            }
     
    424401        mutable UString m_value;
    425402        mutable unsigned m_fiberCount;
    426         // This structure exists to support a temporary workaround for a GC issue.
    427         struct JSStringFinalizerStruct {
    428             JSStringFinalizerStruct() : m_finalizerCallback(0) {}
    429             union {
    430                 mutable FixedArray<RopeImpl::Fiber, s_maxInternalRopeLength> m_fibers;
    431                 struct {
    432                     JSStringFinalizerCallback m_finalizerCallback;
    433                     void* m_finalizerContext;
    434                 };
    435             };
    436         } m_other;
     403        mutable FixedArray<RopeImpl::Fiber, s_maxInternalRopeLength> m_fibers;
    437404
    438405        bool isRope() const { return m_fiberCount; }
     
    445412        friend JSValue jsString(ExecState* exec, Register* strings, unsigned count);
    446413        friend JSValue jsString(ExecState* exec, JSValue thisValue);
    447         friend JSString* jsStringWithFinalizer(ExecState*, const UString&, JSStringFinalizerCallback callback, void* context);
    448414        friend JSString* jsSubstring(ExecState* exec, JSString* s, unsigned offset, unsigned length);
    449415    };
     
    525491    }
    526492
    527     inline JSString* jsStringWithFinalizer(ExecState* exec, const UString& s, JSStringFinalizerCallback callback, void* context)
    528     {
    529         ASSERT(s.length() && (s.length() > 1 || s.characters()[0] > maxSingleCharacterString));
    530         JSGlobalData* globalData = &exec->globalData();
    531         return fixupVPtr(globalData, new (globalData) JSString(globalData, s, callback, context));
    532     }
    533    
    534493    inline JSString* jsSubstring(ExecState* exec, JSString* s, unsigned offset, unsigned length)
    535494    {
  • trunk/Source/WebCore/ChangeLog

    r84932 r84934  
     12011-04-25  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Reviewed by Oliver Hunt.
     4
     5        Nixed special finalizer handling for WebCore strings
     6        https://bugs.webkit.org/show_bug.cgi?id=59425
     7
     8        Not needed anymore, since weak handles have finalizers.
     9
     10        * WebCore.exp.in: Exports!
     11
     12        * bindings/js/DOMWrapperWorld.cpp:
     13        (WebCore::JSStringOwner::finalize):
     14        (WebCore::DOMWrapperWorld::DOMWrapperWorld): Use a weak handle finalizer,
     15        so we don't need special treatment anymore.
     16
     17        * bindings/js/DOMWrapperWorld.h:
     18        (WebCore::JSStringOwner::JSStringOwner):
     19        (WebCore::DOMWrapperWorld::stringWrapperOwner): Use a HashMap of Weak<T>
     20        instead of a WeakGCMap, so we can specify a custom finalizer.
     21
     22        * bindings/js/JSDOMBinding.cpp:
     23        (WebCore::jsStringSlowCase):
     24        * bindings/js/JSDOMBinding.h:
     25        (WebCore::jsString): Updated for string map change.
     26
    1272011-04-26  David Kilzer  <ddkilzer@apple.com>
    228
  • trunk/Source/WebCore/WebCore.exp.in

    r84769 r84934  
    15981598__ZN7WebCore16ScriptController20windowScriptNPObjectEv
    15991599__ZN7WebCore16ScriptController29cleanupScriptObjectsForPluginEPv
    1600 __ZN7WebCore16jsStringSlowCaseEPN3JSC9ExecStateERNS0_9WeakGCMapIPN3WTF10StringImplENS0_8JSStringENS0_33DefaultWeakGCMapFinalizerCallbackIS6_S7_EENS4_10StringHashENS4_10HashTraitsIS6_EEEES6_
     1600__ZN7WebCore16jsStringSlowCaseEPN3JSC9ExecStateERN3WTF7HashMapIPNS3_10StringImplENS0_4WeakINS0_8JSStringEEENS3_10StringHashENS3_10HashTraitsIS6_EENSB_IS9_EEEES6_
    16011601__ZN7WebCore17HTMLPlugInElement11getNPObjectEv
    16021602__ZNK7WebCore14SecurityOrigin9canAccessEPKS0_
  • trunk/Source/WebCore/bindings/js/DOMWrapperWorld.cpp

    r84204 r84934  
    3737}
    3838
     39void JSStringOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
     40{
     41    JSString* jsString = static_cast<JSString*>(handle.get().asCell());
     42    StringImpl* stringImpl = static_cast<StringImpl*>(context);
     43    ASSERT_UNUSED(jsString, m_world->m_stringCache.find(stringImpl)->second.get() == jsString);
     44    m_world->m_stringCache.remove(stringImpl);
     45}
     46
    3947DOMWrapperWorld::DOMWrapperWorld(JSC::JSGlobalData* globalData, bool isNormal)
    4048    : m_globalData(globalData)
    4149    , m_isNormal(isNormal)
    4250    , m_defaultWrapperOwner(this)
     51    , m_stringWrapperOwner(this)
    4352{
    4453    JSGlobalData::ClientData* clientData = m_globalData->clientData;
  • trunk/Source/WebCore/bindings/js/DOMWrapperWorld.h

    r84194 r84934  
    3434
    3535typedef HashMap<void*, JSC::Weak<JSDOMWrapper> > DOMObjectWrapperMap;
    36 typedef JSC::WeakGCMap<StringImpl*, JSC::JSString> JSStringCache;
     36typedef HashMap<StringImpl*, JSC::Weak<JSC::JSString> > JSStringCache;
    3737
    3838class JSDOMWrapperOwner : public JSC::WeakHandleOwner {
     
    4646
    4747inline JSDOMWrapperOwner::JSDOMWrapperOwner(DOMWrapperWorld* world)
     48    : m_world(world)
     49{
     50}
     51
     52class JSStringOwner : public JSC::WeakHandleOwner {
     53public:
     54    JSStringOwner(DOMWrapperWorld*);
     55    virtual void finalize(JSC::Handle<JSC::Unknown>, void* context);
     56
     57private:
     58    DOMWrapperWorld* m_world;
     59};
     60
     61inline JSStringOwner::JSStringOwner(DOMWrapperWorld* world)
    4862    : m_world(world)
    4963{
     
    7286    JSC::JSGlobalData* globalData() const { return m_globalData; }
    7387    JSDOMWrapperOwner* defaultWrapperOwner() { return &m_defaultWrapperOwner; }
     88    JSStringOwner* stringWrapperOwner() { return &m_stringWrapperOwner; }
    7489
    7590protected:
     
    8196    bool m_isNormal;
    8297    JSDOMWrapperOwner m_defaultWrapperOwner;
     98    JSStringOwner m_stringWrapperOwner;
    8399};
    84100
  • trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp

    r84791 r84934  
    6060}
    6161
    62 static void stringWrapperDestroyed(JSString*, void* context)
    63 {
    64     StringImpl* cacheKey = static_cast<StringImpl*>(context);
    65     cacheKey->deref();
    66 }
    67 
    6862JSValue jsStringSlowCase(ExecState* exec, JSStringCache& stringCache, StringImpl* stringImpl)
    6963{
    70     JSString* wrapper = jsStringWithFinalizer(exec, UString(stringImpl), stringWrapperDestroyed, stringImpl);
    71     stringCache.set(exec->globalData(), stringImpl, wrapper);
    72     // ref explicitly instead of using a RefPtr-keyed hashtable because the wrapper can
    73     // outlive the cache, so the stringImpl has to match the wrapper's lifetime.
    74     stringImpl->ref();
     64    JSString* wrapper = jsString(exec, UString(stringImpl));
     65    stringCache.add(stringImpl, Weak<JSString>(exec->globalData(), wrapper, currentWorld(exec)->stringWrapperOwner(), stringImpl));
    7566    return wrapper;
    7667}
  • trunk/Source/WebCore/bindings/js/JSDOMBinding.h

    r84812 r84934  
    309309
    310310        JSStringCache& stringCache = currentWorld(exec)->m_stringCache;
    311         if (JSC::JSString* wrapper = stringCache.get(stringImpl))
    312             return wrapper;
     311        JSStringCache::iterator it = stringCache.find(stringImpl);
     312        if (it != stringCache.end())
     313            return it->second.get();
    313314
    314315        return jsStringSlowCase(exec, stringCache, stringImpl);
Note: See TracChangeset for help on using the changeset viewer.