Changeset 143949 in webkit


Ignore:
Timestamp:
Feb 25, 2013 10:56:43 AM (11 years ago)
Author:
ggaren@apple.com
Message:

Do one lookup per code cache insertion instead of two
​https://bugs.webkit.org/show_bug.cgi?id=110674

Reviewed by Sam Weinig.

Deployed the idiomatic "add null value" trick to avoid a second hash
lookup when inserting an item.

  • runtime/CodeCache.cpp:

(JSC::CodeCacheMap::pruneSlowCase): Factored this into a helper function
to improve clarity and get some code off the hot path.

(JSC::CodeCache::getCodeBlock):
(JSC::CodeCache::getFunctionExecutableFromGlobalCode): Use the add() API
to avoid two hash lookups. Be sure to remove items if parsing fails,
otherwise we'll leave nulls in the table. (I'm guessing that caching parse
errors is not a win.)

  • runtime/CodeCache.h:

(JSC::SourceCodeValue::SourceCodeValue):
(CodeCacheMap):
(JSC::CodeCacheMap::add): Combined find() and set() into add().

(JSC::CodeCacheMap::remove):
(JSC::CodeCacheMap::age):
(JSC::CodeCacheMap::prune): Refactored to support above changes.

Location:
trunk/Source/JavaScriptCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r143909 r143949  
     12013-02-25  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Do one lookup per code cache insertion instead of two
     4        https://bugs.webkit.org/show_bug.cgi?id=110674
     5
     6        Reviewed by Sam Weinig.
     7
     8        Deployed the idiomatic "add null value" trick to avoid a second hash
     9        lookup when inserting an item.
     10
     11        * runtime/CodeCache.cpp:
     12        (JSC::CodeCacheMap::pruneSlowCase): Factored this into a helper function
     13        to improve clarity and get some code off the hot path.
     14
     15        (JSC::CodeCache::getCodeBlock):
     16        (JSC::CodeCache::getFunctionExecutableFromGlobalCode): Use the add() API
     17        to avoid two hash lookups. Be sure to remove items if parsing fails,
     18        otherwise we'll leave nulls in the table. (I'm guessing that caching parse
     19        errors is not a win.)
     20
     21        * runtime/CodeCache.h:
     22        (JSC::SourceCodeValue::SourceCodeValue):
     23        (CodeCacheMap):
     24        (JSC::CodeCacheMap::add): Combined find() and set() into add().
     25
     26        (JSC::CodeCacheMap::remove):
     27        (JSC::CodeCacheMap::age):
     28        (JSC::CodeCacheMap::prune): Refactored to support above changes.
     29
    1302013-02-25  Carlos Garcia Campos  <cgarcia@igalia.com>
    231

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

    r143759 r143949  
    3737namespace JSC {
    3838
     39void CodeCacheMap::pruneSlowCase()
     40{
     41    while (m_size >= m_capacity) {
     42        MapType::iterator it = m_map.begin();
     43        m_size -= it->key.length();
     44        m_map.remove(it);
     45    }
     46}
     47
    3948CodeCache::CodeCache()
    4049{
    … …  
    6170{
    6271    SourceCodeKey key = SourceCodeKey(source, String(), CacheTypes<UnlinkedCodeBlockType>::codeType, strictness);
    63     bool storeInCache = false;
    64     if (debuggerMode == DebuggerOff && profilerMode == ProfilerOff) {
    65         const Strong<JSCell>* result = m_sourceCode.find(key);
    66         if (result) {
    67             UnlinkedCodeBlockType* unlinkedCode = jsCast<UnlinkedCodeBlockType*>(result->get());
    68             unsigned firstLine = source.firstLine() + unlinkedCode->firstLine();
    69             executable->recordParse(unlinkedCode->codeFeatures(), unlinkedCode->hasCapturedVariables(), firstLine, firstLine + unlinkedCode->lineCount());
    70             return unlinkedCode;
    71         }
    72         storeInCache = true;
     72    CodeCacheMap::AddResult addResult = m_sourceCode.add(key, SourceCodeValue());
     73    bool canCache = debuggerMode == DebuggerOff && profilerMode == ProfilerOff;
     74    if (!addResult.isNewEntry && canCache) {
     75        UnlinkedCodeBlockType* unlinkedCode = jsCast<UnlinkedCodeBlockType*>(addResult.iterator->value.cell.get());
     76        unsigned firstLine = source.firstLine() + unlinkedCode->firstLine();
     77        executable->recordParse(unlinkedCode->codeFeatures(), unlinkedCode->hasCapturedVariables(), firstLine, firstLine + unlinkedCode->lineCount());
     78        return unlinkedCode;
    7379    }
    7480
    7581    typedef typename CacheTypes<UnlinkedCodeBlockType>::RootNode RootNode;
    7682    RefPtr<RootNode> rootNode = parse<RootNode>(&globalData, source, 0, Identifier(), strictness, JSParseProgramCode, error);
    77     if (!rootNode)
     83    if (!rootNode) {
     84        m_sourceCode.remove(addResult.iterator);
    7885        return 0;
     86    }
    7987    executable->recordParse(rootNode->features(), rootNode->hasCapturedVariables(), rootNode->lineNo(), rootNode->lastLine());
    8088
    … …  
    8492    error = generator->generate();
    8593    rootNode->destroyData();
    86     if (error.m_type != ParserError::ErrorNone)
     94    if (error.m_type != ParserError::ErrorNone) {
     95        m_sourceCode.remove(addResult.iterator);
    8796        return 0;
     97    }
    8898
    89     if (storeInCache)
    90         m_sourceCode.set(key, Strong<UnlinkedCodeBlock>(globalData, unlinkedCode));
     99    if (!canCache) {
     100        m_sourceCode.remove(addResult.iterator);
     101        return unlinkedCode;
     102    }
    91103
     104    addResult.iterator->value = SourceCodeValue(globalData, unlinkedCode, m_sourceCode.age());
    92105    return unlinkedCode;
    93106}
    … …  
    106119{
    107120    SourceCodeKey key = SourceCodeKey(source, name.string(), SourceCodeKey::FunctionType, JSParseNormal);
    108     const Strong<JSCell>* result = m_sourceCode.find(key);
    109     if (result)
    110         return jsCast<UnlinkedFunctionExecutable*>(result->get());
     121    CodeCacheMap::AddResult addResult = m_sourceCode.add(key, SourceCodeValue());
     122    if (!addResult.isNewEntry)
     123        return jsCast<UnlinkedFunctionExecutable*>(addResult.iterator->value.cell.get());
    111124
    112125    RefPtr<ProgramNode> program = parse<ProgramNode>(&globalData, source, 0, Identifier(), JSParseNormal, JSParseProgramCode, error);
    113126    if (!program) {
    114127        ASSERT(error.m_type != ParserError::ErrorNone);
     128        m_sourceCode.remove(addResult.iterator);
    115129        return 0;
    116130    }
    … …  
    130144    functionExecutable->m_nameValue.set(globalData, functionExecutable, jsString(&globalData, name.string()));
    131145
    132     m_sourceCode.set(key, Strong<UnlinkedFunctionExecutable>(globalData, functionExecutable));
     146    addResult.iterator->value = SourceCodeValue(globalData, functionExecutable, m_sourceCode.age());
    133147    return functionExecutable;
    134148}

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

    r143768 r143949  
    119119    }
    120120
    121     SourceCodeValue(const Strong<JSCell>& cell, int64_t age)
    122         : cell(cell)
     121    SourceCodeValue(JSGlobalData& globalData, JSCell* cell, int64_t age)
     122        : cell(globalData, cell)
    123123        , age(age)
    124124    {
    … …  
    130130
    131131class CodeCacheMap {
     132public:
    132133    typedef HashMap<SourceCodeKey, SourceCodeValue, SourceCodeKeyHash, SourceCodeKeyHashTraits> MapType;
    133134    typedef MapType::iterator iterator;
    134 
    135 public:
     135    typedef MapType::AddResult AddResult;
     136
    136137    enum { MinCacheCapacity = 1000000 }; // Size in characters
    137138
    … …  
    143144    }
    144145
    145     const Strong<JSCell>* find(const SourceCodeKey& key)
    146     {
    147         iterator it = m_map.find(key);
    148         if (it == m_map.end())
    149             return 0;
    150 
    151         int64_t age = m_age - it->value.age;
     146
     147    AddResult add(const SourceCodeKey& key, const SourceCodeValue& value)
     148    {
     149        prune();
     150
     151        AddResult addResult = m_map.add(key, value);
     152        if (addResult.isNewEntry) {
     153            m_size += key.length();
     154            m_age += key.length();
     155            return addResult;
     156        }
     157
     158        int64_t age = m_age - addResult.iterator->value.age;
    152159        if (age > m_capacity) {
    153160            // A requested object is older than the cache's capacity. We can
    … …  
    164171        }
    165172
    166         it->value.age = m_age;
     173        addResult.iterator->value.age = m_age;
    167174        m_age += key.length();
    168         return &it->value.cell;
    169     }
    170 
    171     void set(const SourceCodeKey& key, const Strong<JSCell>& value)
    172     {
    173         pruneIfNeeded();
    174 
    175         m_size += key.length();
    176         m_age += key.length();
    177         m_map.set(key, SourceCodeValue(value, m_age));
     175        return addResult;
     176    }
     177
     178    void remove(iterator it)
     179    {
     180        m_size -= it->key.length();
     181        m_map.remove(it);
    178182    }
    179183
    … …  
    184188        m_map.clear();
    185189    }
     190
     191    int64_t age() { return m_age; }
    186192
    187193private:
    … …  
    195201    static const int64_t oldObjectSamplingMultiplier = 32;
    196202
    197     void pruneIfNeeded()
    198     {
    199         while (m_size >= m_capacity) {
    200             MapType::iterator it = m_map.begin();
    201             m_size -= it->key.length();
    202             m_map.remove(it);
    203         }
     203    void pruneSlowCase();
     204    void prune()
     205    {
     206        if (m_size < m_capacity)
     207            return;
     208        pruneSlowCase();
    204209    }
    205210
Note: See TracChangeset for help on using the changeset viewer.