Changeset 143279 in webkit


Ignore:
Timestamp:
Feb 18, 2013 5:39:12 PM (11 years ago)
Author:
ggaren@apple.com
Message:

Shrank the SourceProvider cache
​https://bugs.webkit.org/show_bug.cgi?id=110158

Reviewed by Oliver Hunt.

Source/JavaScriptCore:

CodeCache is now our primary source cache, so a long-lived SourceProvider
cache is a waste. I measured this as a 10MB Membuster win; with more
precise instrumentation, Andreas estimated it as up to 30MB.

I didn't eliminate the SourceProvider cache because it's still useful
in speeding up uncached parsing of scripts with large nested functions
(i.e., all scripts).

  • heap/Heap.cpp:

(JSC::Heap::collect): Discard all source provider caches after GC. This
is a convenient place to do so because it's reasonably soon after initial
parsing without being immediate.

  • parser/Parser.cpp:

(JSC::::Parser): Updated for interface change: The heap now owns the
source provider cache, since most SourceProviders are not expected to
have one by default, and the heap is responsible for throwing them away.

(JSC::::parseInner): No need to update statistics on cache size, since
we're going to throw it away no matter what.

(JSC::::parseFunctionInfo): Reduced the minimum function size to 16. This
is a 27% win on a new parsing micro-benchmark I've added. Now that the
cache is temporary, we don't have to worry so much about its memory
footprint.

  • parser/Parser.h:

(Parser): Updated for interface changes.

  • parser/SourceProvider.cpp:

(JSC::SourceProvider::SourceProvider):
(JSC::SourceProvider::~SourceProvider):

  • parser/SourceProvider.h:

(JSC):
(SourceProvider): SourceProvider doesn't own its cache anymore because
the cache is temporary.

  • parser/SourceProviderCache.cpp:

(JSC::SourceProviderCache::clear):
(JSC::SourceProviderCache::add):

  • parser/SourceProviderCache.h:

(JSC::SourceProviderCache::SourceProviderCache):
(SourceProviderCache):

  • parser/SourceProviderCacheItem.h:

(SourceProviderCacheItem): No need to update statistics on cache size,
since we're going to throw it away no matter what.

  • runtime/JSGlobalData.cpp:

(JSC::JSGlobalData::addSourceProviderCache):
(JSC):
(JSC::JSGlobalData::clearSourceProviderCaches):

  • runtime/JSGlobalData.h:

(JSC):
(JSGlobalData): Moved the cache here so it's easier to throw away.

Source/WebCore:

Test: fast/js/regress/nested-function-parsing.html

No need to keep statistics on cache size, since we're going to throw it
away no matter what.

  • WebCore.order:
  • bindings/js/CachedScriptSourceProvider.h:

(CachedScriptSourceProvider):
(WebCore::CachedScriptSourceProvider::CachedScriptSourceProvider):

  • loader/cache/CachedScript.cpp:

(WebCore::CachedScript::destroyDecodedData):
(WebCore):
(WebCore::CachedScript::reportMemoryUsage):

  • loader/cache/CachedScript.h:

(CachedScript):

LayoutTests:

New benchmark to show that a minimum size of 16 is better than 64.

  • fast/js/regress/nested-function-parsing-expected.txt: Added.
  • fast/js/regress/nested-function-parsing.html: Added.
  • fast/js/regress/script-tests/nested-function-parsing.js: Added.
Location:
trunk
Files:
3 added
17 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r143269 r143279  
     12013-02-18  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Shrank the SourceProvider cache
     4        https://bugs.webkit.org/show_bug.cgi?id=110158
     5
     6        Reviewed by Oliver Hunt.
     7
     8        New benchmark to show that a minimum size of 16 is better than 64.
     9
     10        * fast/js/regress/nested-function-parsing-expected.txt: Added.
     11        * fast/js/regress/nested-function-parsing.html: Added.
     12        * fast/js/regress/script-tests/nested-function-parsing.js: Added.
     13
    1142013-02-18  Filip Pizlo  <fpizlo@apple.com>
    215

  • trunk/Source/JavaScriptCore/ChangeLog

    r143276 r143279  
     12013-02-18  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Shrank the SourceProvider cache
     4        https://bugs.webkit.org/show_bug.cgi?id=110158
     5
     6        Reviewed by Oliver Hunt.
     7
     8        CodeCache is now our primary source cache, so a long-lived SourceProvider
     9        cache is a waste. I measured this as a 10MB Membuster win; with more
     10        precise instrumentation, Andreas estimated it as up to 30MB.
     11
     12        I didn't eliminate the SourceProvider cache because it's still useful
     13        in speeding up uncached parsing of scripts with large nested functions
     14        (i.e., all scripts).
     15
     16        * heap/Heap.cpp:
     17        (JSC::Heap::collect): Discard all source provider caches after GC. This
     18        is a convenient place to do so because it's reasonably soon after initial
     19        parsing without being immediate.
     20
     21        * parser/Parser.cpp:
     22        (JSC::::Parser): Updated for interface change: The heap now owns the
     23        source provider cache, since most SourceProviders are not expected to
     24        have one by default, and the heap is responsible for throwing them away.
     25
     26        (JSC::::parseInner): No need to update statistics on cache size, since
     27        we're going to throw it away no matter what.
     28
     29        (JSC::::parseFunctionInfo): Reduced the minimum function size to 16. This
     30        is a 27% win on a new parsing micro-benchmark I've added. Now that the
     31        cache is temporary, we don't have to worry so much about its memory
     32        footprint.
     33
     34        * parser/Parser.h:
     35        (Parser): Updated for interface changes.
     36
     37        * parser/SourceProvider.cpp:
     38        (JSC::SourceProvider::SourceProvider):
     39        (JSC::SourceProvider::~SourceProvider):
     40        * parser/SourceProvider.h:
     41        (JSC):
     42        (SourceProvider): SourceProvider doesn't own its cache anymore because
     43        the cache is temporary.
     44
     45        * parser/SourceProviderCache.cpp:
     46        (JSC::SourceProviderCache::clear):
     47        (JSC::SourceProviderCache::add):
     48        * parser/SourceProviderCache.h:
     49        (JSC::SourceProviderCache::SourceProviderCache):
     50        (SourceProviderCache):
     51        * parser/SourceProviderCacheItem.h:
     52        (SourceProviderCacheItem): No need to update statistics on cache size,
     53        since we're going to throw it away no matter what.
     54
     55        * runtime/JSGlobalData.cpp:
     56        (JSC::JSGlobalData::addSourceProviderCache):
     57        (JSC):
     58        (JSC::JSGlobalData::clearSourceProviderCaches):
     59        * runtime/JSGlobalData.h:
     60        (JSC):
     61        (JSGlobalData): Moved the cache here so it's easier to throw away.
     62
    1632013-02-18  Filip Pizlo  <fpizlo@apple.com>
    264

  • trunk/Source/JavaScriptCore/heap/Heap.cpp

    r141179 r143279  
    751751    }
    752752
     753    {
     754        GCPHASE(DeleteSourceProviderCaches);
     755        m_globalData->clearSourceProviderCaches();
     756    }
     757
    753758    if (sweepToggle == DoSweep) {
    754759        SamplingRegion samplingRegion("Garbage Collection: Sweeping");

  • trunk/Source/JavaScriptCore/parser/Parser.cpp

    r140945 r143279  
    8585    m_lexer->setCode(source, m_arena);
    8686
    87     m_functionCache = source.provider()->cache();
     87    m_functionCache = globalData->addSourceProviderCache(source.provider());
    8888    ScopeRef scope = pushScope();
    8989    if (parserMode == JSParseFunctionCode)
    … …  
    111111    String parseError = String();
    112112   
    113     unsigned oldFunctionCacheSize = m_functionCache ? m_functionCache->byteSize() : 0;
    114113    ASTBuilder context(const_cast<JSGlobalData*>(m_globalData), const_cast<SourceCode*>(m_source));
    115114    if (m_lexer->isReparsing())
    … …  
    127126    if (scope->shadowsArguments())
    128127        features |= ShadowsArgumentsFeature;
    129     unsigned functionCacheSize = m_functionCache ? m_functionCache->byteSize() : 0;
    130     if (functionCacheSize != oldFunctionCacheSize)
    131         m_lexer->sourceProvider()->notifyCacheSizeChanged(functionCacheSize - oldFunctionCacheSize);
    132128
    133129    didFinishParsing(sourceElements, context.varDeclarations(), context.funcDeclarations(), features,
    … …  
    864860    // Cache the tokenizer state and the function scope the first time the function is parsed.
    865861    // Any future reparsing can then skip the function.
    866     static const int minimumFunctionLengthToCache = 64;
     862    static const int minimumFunctionLengthToCache = 16;
    867863    OwnPtr<SourceProviderCacheItem> newInfo;
    868864    int functionLength = closeBracePos - openBracePos;
    … …  
    880876    matchOrFail(CLOSEBRACE);
    881877   
    882     if (newInfo) {
    883         unsigned approximateByteSize = newInfo->approximateByteSize();
    884         m_functionCache->add(openBracePos, newInfo.release(), approximateByteSize);
    885     }
     878    if (newInfo)
     879        m_functionCache->add(openBracePos, newInfo.release());
    886880   
    887881    next();

  • trunk/Source/JavaScriptCore/parser/Parser.h

    r143147 r143279  
    909909    int m_nonTrivialExpressionCount;
    910910    const Identifier* m_lastIdentifier;
    911     SourceProviderCache* m_functionCache;
     911    RefPtr<SourceProviderCache> m_functionCache;
    912912    SourceElements* m_sourceElements;
    913913    ParserArenaData<DeclarationStacks::VarStack>* m_varDeclarations;

  • trunk/Source/JavaScriptCore/parser/SourceProvider.cpp

    r143147 r143279  
    2727#include "SourceProvider.h"
    2828
    29 #include "SourceProviderCache.h"
    30 
    3129namespace JSC {
    3230
    33 SourceProvider::SourceProvider(const String& url, const TextPosition& startPosition, SourceProviderCache* cache)
     31SourceProvider::SourceProvider(const String& url, const TextPosition& startPosition)
    3432    : m_url(url)
    3533    , m_startPosition(startPosition)
    3634    , m_validated(false)
    37     , m_cache(cache ? cache : new SourceProviderCache)
    38     , m_cacheOwned(!cache)
    3935{
    4036}
    … …  
    4238SourceProvider::~SourceProvider()
    4339{
    44     if (m_cacheOwned)
    45         delete m_cache;
    4640}
    4741

  • trunk/Source/JavaScriptCore/parser/SourceProvider.h

    r143147 r143279  
    3838namespace JSC {
    3939
    40     class SourceProviderCache;
    41 
    4240    class SourceProvider : public RefCounted<SourceProvider> {
    4341    public:
    4442        static const intptr_t nullID = 1;
    4543       
    46         JS_EXPORT_PRIVATE SourceProvider(const String& url, const TextPosition& startPosition, SourceProviderCache* = 0);
     44        JS_EXPORT_PRIVATE SourceProvider(const String& url, const TextPosition& startPosition);
    4745
    4846        JS_EXPORT_PRIVATE virtual ~SourceProvider();
    … …  
    6765        void setValid() { m_validated = true; }
    6866
    69         SourceProviderCache* cache() const { return m_cache; }
    70         void notifyCacheSizeChanged(int delta) { if (!m_cacheOwned) cacheSizeChanged(delta); }
    71        
    7267    private:
    73         virtual void cacheSizeChanged(int delta) { UNUSED_PARAM(delta); }
    7468
    7569        String m_url;
    7670        TextPosition m_startPosition;
    7771        bool m_validated;
    78         SourceProviderCache* m_cache;
    79         bool m_cacheOwned;
    8072    };
    8173

  • trunk/Source/JavaScriptCore/parser/SourceProviderCache.cpp

    r104318 r143279  
    3737{
    3838    m_map.clear();
    39     m_contentByteSize = 0;
    4039}
    4140
    42 unsigned SourceProviderCache::byteSize() const
    43 {
    44     return m_contentByteSize + sizeof(*this) + m_map.capacity() * sizeof(SourceProviderCacheItem*);
    45 }
    46 
    47 void SourceProviderCache::add(int sourcePosition, PassOwnPtr<SourceProviderCacheItem> item, unsigned size)
     41void SourceProviderCache::add(int sourcePosition, PassOwnPtr<SourceProviderCacheItem> item)
    4842{
    4943    m_map.add(sourcePosition, item);
    50     m_contentByteSize += size;
    5144}
    5245

  • trunk/Source/JavaScriptCore/parser/SourceProviderCache.h

    r127484 r143279  
    3131#include <wtf/OwnPtr.h>
    3232#include <wtf/PassOwnPtr.h>
     33#include <wtf/RefCounted.h>
    3334
    3435namespace JSC {
    3536
    36 class SourceProviderCache {
     37class SourceProviderCache : public RefCounted<SourceProviderCache> {
    3738    WTF_MAKE_FAST_ALLOCATED;
    3839public:
    39     SourceProviderCache() : m_contentByteSize(0) {}
     40    SourceProviderCache() { }
    4041    JS_EXPORT_PRIVATE ~SourceProviderCache();
    4142
    4243    JS_EXPORT_PRIVATE void clear();
    43     JS_EXPORT_PRIVATE unsigned byteSize() const;
    44     void add(int sourcePosition, PassOwnPtr<SourceProviderCacheItem>, unsigned size);
     44    void add(int sourcePosition, PassOwnPtr<SourceProviderCacheItem>);
    4545    const SourceProviderCacheItem* get(int sourcePosition) const { return m_map.get(sourcePosition); }
    4646
    4747private:
    4848    HashMap<int, OwnPtr<SourceProviderCacheItem> > m_map;
    49     unsigned m_contentByteSize;
    5049};
    5150

  • trunk/Source/JavaScriptCore/parser/SourceProviderCacheItem.h

    r140945 r143279  
    5656    ~SourceProviderCacheItem();
    5757
    58     unsigned approximateByteSize() const
    59     {
    60         // The identifiers are uniqued strings so most likely there are few names that actually use any additional memory.
    61         static const unsigned assumedAverageIdentifierSize = sizeof(StringImpl*) + 2;
    62         unsigned size = sizeof(*this);
    63         size += usedVariablesCount * assumedAverageIdentifierSize;
    64         size += writtenVariablesCount * assumedAverageIdentifierSize;
    65         return size;
    66     }
    6758    JSToken closeBraceToken() const
    6859    {

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

    r141651 r143279  
    5858#include "RegExpCache.h"
    5959#include "RegExpObject.h"
     60#include "SourceProviderCache.h"
    6061#include "StrictEvalActivation.h"
    6162#include "StrongInlines.h"
    … …  
    457458}
    458459
     460SourceProviderCache* JSGlobalData::addSourceProviderCache(SourceProvider* sourceProvider)
     461{
     462    SourceProviderCacheMap::AddResult addResult = sourceProviderCacheMap.add(sourceProvider, 0);
     463    if (addResult.isNewEntry)
     464        addResult.iterator->value = adoptRef(new SourceProviderCache);
     465    return addResult.iterator->value.get();
     466}
     467
     468void JSGlobalData::clearSourceProviderCaches()
     469{
     470    sourceProviderCacheMap.clear();
     471}
     472
    459473struct StackPreservingRecompiler : public MarkedBlock::VoidFunctor {
    460474    HashSet<FunctionExecutable*> currentlyExecutingFunctions;

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

    r143247 r143279  
    8383    class ParserArena;
    8484    class RegExpCache;
     85    class SourceProvider;
     86    class SourceProviderCache;
    8587    class Stringifier;
    8688    class Structure;
    … …  
    304306#endif
    305307
     308        SourceProviderCache* addSourceProviderCache(SourceProvider*);
     309        void clearSourceProviderCaches();
     310
    306311        PrototypeMap prototypeMap;
    307312
    308313        OwnPtr<ParserArena> parserArena;
     314        typedef HashMap<RefPtr<SourceProvider>, RefPtr<SourceProviderCache> > SourceProviderCacheMap;
     315        SourceProviderCacheMap sourceProviderCacheMap;
    309316        OwnPtr<Keywords> keywords;
    310317        Interpreter* interpreter;

  • trunk/Source/WebCore/ChangeLog

    r143272 r143279  
     12013-02-18  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Shrank the SourceProvider cache
     4        https://bugs.webkit.org/show_bug.cgi?id=110158
     5
     6        Reviewed by Oliver Hunt.
     7
     8        Test: fast/js/regress/nested-function-parsing.html
     9
     10        No need to keep statistics on cache size, since we're going to throw it
     11        away no matter what.
     12
     13        * WebCore.order:
     14        * bindings/js/CachedScriptSourceProvider.h:
     15        (CachedScriptSourceProvider):
     16        (WebCore::CachedScriptSourceProvider::CachedScriptSourceProvider):
     17        * loader/cache/CachedScript.cpp:
     18        (WebCore::CachedScript::destroyDecodedData):
     19        (WebCore):
     20        (WebCore::CachedScript::reportMemoryUsage):
     21        * loader/cache/CachedScript.h:
     22        (CachedScript):
     23
    1242013-02-18  peavo@outlook.com  <peavo@outlook.com>
    225

  • trunk/Source/WebCore/WebCore.order

    r142977 r143279  
    40014001__ZN7WebCore14CachedResource12removeClientEPNS_20CachedResourceClientE
    40024002__ZNK7WebCore26CachedScriptSourceProvider4dataEv
    4003 __ZN7WebCore26CachedScriptSourceProvider16cacheSizeChangedEi
    4004 __ZN7WebCore12CachedScript30sourceProviderCacheSizeChangedEi
    40054003__ZN7WebCore17CheckboxInputType6createEPNS_16HTMLInputElementE
    40064004__ZNK7WebCore17CheckboxInputType15formControlTypeEv

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

    r128542 r143279  
    4747    const String& source() const { return m_cachedScript->script(); }
    4848
    49     virtual void cacheSizeChanged(int delta)
    50     {
    51         m_cachedScript->sourceProviderCacheSizeChanged(delta);
    52     }
    53 
    5449private:
    5550    CachedScriptSourceProvider(CachedScript* cachedScript)
    56         : SourceProvider(cachedScript->response().url(), TextPosition::minimumPosition(), cachedScript->sourceProviderCache())
     51        : SourceProvider(cachedScript->response().url(), TextPosition::minimumPosition())
    5752        , m_cachedScript(cachedScript)
    5853    {

  • trunk/Source/WebCore/loader/cache/CachedScript.cpp

    r143147 r143279  
    3737#include "WebCoreMemoryInstrumentation.h"
    3838#include <wtf/Vector.h>
    39 
    40 #if USE(JSC) 
    41 #include <parser/SourceProvider.h>
    42 #include <parser/SourceProviderCache.h>
    43 #endif
    4439
    4540namespace WebCore {
    … …  
    10297{
    10398    m_script = String();
    104     unsigned extraSize = 0;
    105 #if USE(JSC)
    106     if (m_sourceProviderCache && m_clients.isEmpty())
    107         m_sourceProviderCache->clear();
    108 
    109     extraSize = m_sourceProviderCache ? m_sourceProviderCache->byteSize() : 0;
    110 #endif
    111     setDecodedSize(extraSize);
     99    setDecodedSize(0);
    112100    if (!MemoryCache::shouldMakeResourcePurgeableOnEviction() && isSafeToMakePurgeable())
    113101        makePurgeable(true);
    114102}
    115 
    116 #if USE(JSC)
    117 JSC::SourceProviderCache* CachedScript::sourceProviderCache() const
    118 {   
    119     if (!m_sourceProviderCache)
    120         m_sourceProviderCache = adoptPtr(new JSC::SourceProviderCache);
    121     return m_sourceProviderCache.get();
    122 }
    123 
    124 void CachedScript::sourceProviderCacheSizeChanged(int delta)
    125 {
    126     setDecodedSize(decodedSize() + delta);
    127 }
    128 #endif
    129103
    130104#if ENABLE(NOSNIFF)
    … …  
    141115    info.addMember(m_script, "script");
    142116    info.addMember(m_decoder, "decoder");
    143 #if USE(JSC)
    144     info.addMember(m_sourceProviderCache, "sourceProviderCache");
    145 #endif
    146117}
    147118

  • trunk/Source/WebCore/loader/cache/CachedScript.h

    r142683 r143279  
    2929#include "CachedResource.h"
    3030
    31 #if USE(JSC)
    32 namespace JSC {
    33     class SourceProviderCache;
    34 }
    35 #endif
    36 
    3731namespace WebCore {
    3832
    … …  
    5347
    5448        virtual void destroyDecodedData();
    55 #if USE(JSC)       
    56         // Allows JSC to cache additional information about the source.
    57         JSC::SourceProviderCache* sourceProviderCache() const;
    58         void sourceProviderCacheSizeChanged(int delta);
    59 #endif
    6049#if ENABLE(NOSNIFF)
    6150        bool mimeTypeAllowedByNosniff() const;
    … …  
    6958        String m_script;
    7059        RefPtr<TextResourceDecoder> m_decoder;
    71 #if USE(JSC)       
    72         mutable OwnPtr<JSC::SourceProviderCache> m_sourceProviderCache;
    73 #endif
    7460    };
    7561}
Note: See TracChangeset for help on using the changeset viewer.