Code cache should be explicit about what it caches

https://bugs.webkit.org/show_bug.cgi?id=110039

Reviewed by Oliver Hunt.

This patch makes the code cache more explicit in two ways:

(1) The cache caches top-level scripts. Any sub-functions executed as a
part of a script are cached with it and evicted with it.

This simplifies things by eliminating out-of-band sub-function tracking,
and fixes pathological cases where functions for live scripts would be
evicted in favor of functions for dead scripts, and/or high probability
functions executed early in script lifetime would be evicted in favor of
low probability functions executed late in script lifetime, due to LRU.

Statistical data from general browsing and PLT confirms that caching
functions independently of scripts is not profitable.

(2) The cache tracks script size, not script count.

This reduces the worst-case cache size by a factor of infinity.

Script size is a reasonable first-order estimate of in-memory footprint
for a cached script because there are no syntactic constructs that have
super-linear memory footprint.

* bytecode/UnlinkedCodeBlock.cpp:
(JSC::generateFunctionCodeBlock): Moved this function out of the cache
because it does not consult the cache, and is not managed by it.

(JSC::UnlinkedFunctionExecutable::visitChildren): Visit our code blocks
because they are strong references now, rather than weak, a la (1).

(JSC::UnlinkedFunctionExecutable::codeBlockFor): Updated for interface changes.

* bytecode/UnlinkedCodeBlock.h:
(UnlinkedFunctionExecutable):
(UnlinkedFunctionCodeBlock): Strong now, not weak, a la (1).

* runtime/CodeCache.cpp:
(JSC::CodeCache::CodeCache):
* runtime/CodeCache.h:
(JSC::SourceCodeKey::length):
(SourceCodeKey):
(CodeCacheMap):
(JSC::CodeCacheMap::CodeCacheMap):
(JSC::CodeCacheMap::find):
(JSC::CodeCacheMap::set):
(JSC::CodeCacheMap::clear):
(CodeCache):
(JSC::CodeCache::clear): Removed individual function tracking, due to (1).
Added explicit character counting, for (2).

You might think 16000000 characters is a lot. It is. But this patch
didn't establish that limit -- it just took the existing limit and
made it more visible. I intend to reduce the size of the cache in a
future patch.


Canonical link: https://commits.webkit.org/128334@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@143133 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,64 @@
+2013-02-16  Geoffrey Garen  <ggaren@apple.com>
+
+        Code cache should be explicit about what it caches
+        https://bugs.webkit.org/show_bug.cgi?id=110039
+
+        Reviewed by Oliver Hunt.
+
+        This patch makes the code cache more explicit in two ways:
+
+        (1) The cache caches top-level scripts. Any sub-functions executed as a
+        part of a script are cached with it and evicted with it.
+
+        This simplifies things by eliminating out-of-band sub-function tracking,
+        and fixes pathological cases where functions for live scripts would be
+        evicted in favor of functions for dead scripts, and/or high probability
+        functions executed early in script lifetime would be evicted in favor of
+        low probability functions executed late in script lifetime, due to LRU.
+
+        Statistical data from general browsing and PLT confirms that caching
+        functions independently of scripts is not profitable.
+
+        (2) The cache tracks script size, not script count.
+
+        This reduces the worst-case cache size by a factor of infinity.
+
+        Script size is a reasonable first-order estimate of in-memory footprint 
+        for a cached script because there are no syntactic constructs that have
+        super-linear memory footprint.
+
+        * bytecode/UnlinkedCodeBlock.cpp:
+        (JSC::generateFunctionCodeBlock): Moved this function out of the cache
+        because it does not consult the cache, and is not managed by it.
+
+        (JSC::UnlinkedFunctionExecutable::visitChildren): Visit our code blocks
+        because they are strong references now, rather than weak, a la (1).
+
+        (JSC::UnlinkedFunctionExecutable::codeBlockFor): Updated for interface changes.
+
+        * bytecode/UnlinkedCodeBlock.h:
+        (UnlinkedFunctionExecutable):
+        (UnlinkedFunctionCodeBlock): Strong now, not weak, a la (1).
+
+        * runtime/CodeCache.cpp:
+        (JSC::CodeCache::CodeCache):
+        * runtime/CodeCache.h:
+        (JSC::SourceCodeKey::length):
+        (SourceCodeKey):
+        (CodeCacheMap):
+        (JSC::CodeCacheMap::CodeCacheMap):
+        (JSC::CodeCacheMap::find):
+        (JSC::CodeCacheMap::set):
+        (JSC::CodeCacheMap::clear):
+        (CodeCache):
+        (JSC::CodeCache::clear): Removed individual function tracking, due to (1).
+        Added explicit character counting, for (2).
+
+        You might think 16000000 characters is a lot. It is. But this patch
+        didn't establish that limit -- it just took the existing limit and
+        made it more visible. I intend to reduce the size of the cache in a
+        future patch.
+
 2013-02-16  Filip Pizlo  <fpizlo@apple.com>
 
         Remove support for bytecode comments, since it doesn't build, and hasn't been used in a while.

Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp

@@ -46,6 +46,29 @@ const ClassInfo UnlinkedProgramCodeBlock::s_info = { "UnlinkedProgramCodeBlock",
 const ClassInfo UnlinkedEvalCodeBlock::s_info = { "UnlinkedEvalCodeBlock", &Base::s_info, 0, 0, CREATE_METHOD_TABLE(UnlinkedEvalCodeBlock) };
 const ClassInfo UnlinkedFunctionCodeBlock::s_info = { "UnlinkedFunctionCodeBlock", &Base::s_info, 0, 0, CREATE_METHOD_TABLE(UnlinkedFunctionCodeBlock) };
 
+static UnlinkedFunctionCodeBlock* generateFunctionCodeBlock(JSGlobalData& globalData, UnlinkedFunctionExecutable* executable, const SourceCode& source, CodeSpecializationKind kind, DebuggerMode debuggerMode, ProfilerMode profilerMode, ParserError& error)
+{
+    RefPtr<FunctionBodyNode> body = parse<FunctionBodyNode>(&globalData, source, executable->parameters(), executable->name(), executable->isInStrictContext() ? JSParseStrict : JSParseNormal, JSParseFunctionCode, error);
+
+    if (!body) {
+        ASSERT(error.m_type != ParserError::ErrorNone);
+        return 0;
+    }
+
+    if (executable->forceUsesArguments())
+        body->setUsesArguments();
+    body->finishParsing(executable->parameters(), executable->name(), executable->functionNameIsInScopeToggle());
+    executable->recordParse(body->features(), body->hasCapturedVariables(), body->lineNo(), body->lastLine());
+
+    UnlinkedFunctionCodeBlock* result = UnlinkedFunctionCodeBlock::create(&globalData, FunctionCode, ExecutableInfo(body->needsActivation(), body->usesEval(), body->isStrictMode(), kind == CodeForConstruct));
+    OwnPtr<BytecodeGenerator> generator(adoptPtr(new BytecodeGenerator(globalData, body.get(), result, debuggerMode, profilerMode)));
+    error = generator->generate();
+    body->destroyData();
+    if (error.m_type != ParserError::ErrorNone)
+        return 0;
+    return result;
+}
+
 unsigned UnlinkedCodeBlock::addOrFindConstant(JSValue v)
 {
     unsigned numberOfConstants = numberOfConstantRegisters();
@@ -82,6 +105,8 @@ void UnlinkedFunctionExecutable::visitChildren(JSCell* cell, SlotVisitor& visito
     COMPILE_ASSERT(StructureFlags & OverridesVisitChildren, OverridesVisitChildrenWithoutSettingFlag);
     ASSERT(thisObject->structure()->typeInfo().overridesVisitChildren());
     Base::visitChildren(thisObject, visitor);
+    visitor.append(&thisObject->m_codeBlockForCall);
+    visitor.append(&thisObject->m_codeBlockForConstruct);
     visitor.append(&thisObject->m_nameValue);
     visitor.append(&thisObject->m_symbolTableForCall);
     visitor.append(&thisObject->m_symbolTableForConstruct);
@@ -112,31 +137,27 @@ UnlinkedFunctionCodeBlock* UnlinkedFunctionExecutable::codeBlockFor(JSGlobalData
 {
     switch (specializationKind) {
     case CodeForCall:
-        if (UnlinkedFunctionCodeBlock* codeBlock = m_codeBlockForCall.get()) {
-            globalData.codeCache()->usedFunctionCode(globalData, codeBlock);
+        if (UnlinkedFunctionCodeBlock* codeBlock = m_codeBlockForCall.get())
             return codeBlock;
-        }
         break;
     case CodeForConstruct:
-        if (UnlinkedFunctionCodeBlock* codeBlock = m_codeBlockForConstruct.get()) {
-            globalData.codeCache()->usedFunctionCode(globalData, codeBlock);
+        if (UnlinkedFunctionCodeBlock* codeBlock = m_codeBlockForConstruct.get())
             return codeBlock;
-        }
         break;
     }
 
-    UnlinkedFunctionCodeBlock* result = globalData.codeCache()->getFunctionCodeBlock(globalData, this, source, specializationKind, debuggerMode, profilerMode, error);
+    UnlinkedFunctionCodeBlock* result = generateFunctionCodeBlock(globalData, this, source, specializationKind, debuggerMode, profilerMode, error);
 
     if (error.m_type != ParserError::ErrorNone)
         return 0;
 
     switch (specializationKind) {
     case CodeForCall:
-        m_codeBlockForCall = PassWeak<UnlinkedFunctionCodeBlock>(result);
+        m_codeBlockForCall.set(globalData, this, result);
         m_symbolTableForCall.set(globalData, this, result->symbolTable());
         break;
     case CodeForConstruct:
-        m_codeBlockForConstruct = PassWeak<UnlinkedFunctionCodeBlock>(result);
+        m_codeBlockForConstruct.set(globalData, this, result);
         m_symbolTableForConstruct.set(globalData, this, result->symbolTable());
         break;
     }

Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h

@@ -139,8 +139,8 @@ class UnlinkedFunctionExecutable : public JSCell {
 
 private:
     UnlinkedFunctionExecutable(JSGlobalData*, Structure*, const SourceCode&, FunctionBodyNode*);
-    Weak<UnlinkedFunctionCodeBlock> m_codeBlockForCall;
-    Weak<UnlinkedFunctionCodeBlock> m_codeBlockForConstruct;
+    WriteBarrier<UnlinkedFunctionCodeBlock> m_codeBlockForCall;
+    WriteBarrier<UnlinkedFunctionCodeBlock> m_codeBlockForConstruct;
 
     unsigned m_numCapturedVariables : 29;
     bool m_forceUsesArguments : 1;
@@ -675,17 +675,14 @@ class UnlinkedEvalCodeBlock : public UnlinkedGlobalCodeBlock {
 };
 
 class UnlinkedFunctionCodeBlock : public UnlinkedCodeBlock {
-private:
-    friend class CodeCache;
-
+public:
     static UnlinkedFunctionCodeBlock* create(JSGlobalData* globalData, CodeType codeType, const ExecutableInfo& info)
     {
         UnlinkedFunctionCodeBlock* instance = new (NotNull, allocateCell<UnlinkedFunctionCodeBlock>(globalData->heap)) UnlinkedFunctionCodeBlock(globalData, globalData->unlinkedFunctionCodeBlockStructure.get(), codeType, info);
         instance->finishCreation(*globalData);
         return instance;
     }
 
-public:
     typedef UnlinkedCodeBlock Base;
     static void destroy(JSCell*);
 

Source/JavaScriptCore/runtime/CodeCache.cpp

@@ -37,6 +37,7 @@
 namespace JSC {
 
 CodeCache::CodeCache()
+    : m_sourceCode(CacheSize)
 {
 }
 
@@ -102,35 +103,6 @@ UnlinkedEvalCodeBlock* CodeCache::getEvalCodeBlock(JSGlobalData& globalData, Eva
     return getCodeBlock<UnlinkedEvalCodeBlock>(globalData, executable, source, strictness, debuggerMode, profilerMode, error);
 }
 
-UnlinkedFunctionCodeBlock* CodeCache::generateFunctionCodeBlock(JSGlobalData& globalData, UnlinkedFunctionExecutable* executable, const SourceCode& source, CodeSpecializationKind kind, DebuggerMode debuggerMode, ProfilerMode profilerMode, ParserError& error)
-{
-    RefPtr<FunctionBodyNode> body = parse<FunctionBodyNode>(&globalData, source, executable->parameters(), executable->name(), executable->isInStrictContext() ? JSParseStrict : JSParseNormal, JSParseFunctionCode, error);
-
-    if (!body) {
-        ASSERT(error.m_type != ParserError::ErrorNone);
-        return 0;
-    }
-
-    if (executable->forceUsesArguments())
-        body->setUsesArguments();
-    body->finishParsing(executable->parameters(), executable->name(), executable->functionNameIsInScopeToggle());
-    executable->recordParse(body->features(), body->hasCapturedVariables(), body->lineNo(), body->lastLine());
-
-    UnlinkedFunctionCodeBlock* result = UnlinkedFunctionCodeBlock::create(&globalData, FunctionCode, ExecutableInfo(body->needsActivation(), body->usesEval(), body->isStrictMode(), kind == CodeForConstruct));
-    OwnPtr<BytecodeGenerator> generator(adoptPtr(new BytecodeGenerator(globalData, body.get(), result, debuggerMode, profilerMode)));
-    error = generator->generate();
-    body->destroyData();
-    if (error.m_type != ParserError::ErrorNone)
-        return 0;
-    m_recentlyUsedFunctions.set(result, Strong<UnlinkedFunctionCodeBlock>(globalData, result));
-    return result;
-}
-
-UnlinkedFunctionCodeBlock* CodeCache::getFunctionCodeBlock(JSGlobalData& globalData, UnlinkedFunctionExecutable* executable, const SourceCode& source, CodeSpecializationKind kind, DebuggerMode debuggerMode, ProfilerMode profilerMode, ParserError& error)
-{
-    return generateFunctionCodeBlock(globalData, executable, source, kind, debuggerMode, profilerMode, error);
-}
-
 UnlinkedFunctionExecutable* CodeCache::getFunctionExecutableFromGlobalCode(JSGlobalData& globalData, const Identifier& name, const SourceCode& source, ParserError& error)
 {
     SourceCodeKey key = SourceCodeKey(source, name.string(), SourceCodeKey::FunctionCallType, JSParseNormal);
@@ -162,9 +134,4 @@ UnlinkedFunctionExecutable* CodeCache::getFunctionExecutableFromGlobalCode(JSGlo
     return functionExecutable;
 }
 
-void CodeCache::usedFunctionCode(JSGlobalData& globalData, UnlinkedFunctionCodeBlock* codeBlock)
-{
-    m_recentlyUsedFunctions.set(codeBlock, Strong<UnlinkedFunctionCodeBlock>(globalData, codeBlock));
-}
-
 }

Source/JavaScriptCore/runtime/CodeCache.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012 Apple Inc. All Rights Reserved.
+ * Copyright (C) 2012, 2013 Apple Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -53,34 +53,6 @@ struct ParserError;
 class SourceCode;
 class SourceProvider;
 
-template <int CacheSize, typename KeyType, typename EntryType, typename HashArg = typename DefaultHash<KeyType>::Hash, typename KeyTraitsArg = HashTraits<KeyType> >
-class CacheMap {
-    typedef HashMap<KeyType, EntryType, HashArg, KeyTraitsArg> MapType;
-    typedef typename MapType::iterator iterator;
-
-public:
-    const EntryType* find(const KeyType& key)
-    {
-        iterator result = m_map.find(key);
-        if (result == m_map.end())
-            return 0;
-        return &result->value;
-    }
-
-    void set(const KeyType& key, const EntryType& value)
-    {
-        if (m_map.size() >= CacheSize)
-            m_map.remove(m_map.begin());
-
-        m_map.set(key, value);
-    }
-
-    void clear() { m_map.clear(); }
-
-private:
-    MapType m_map;
-};
-
 class SourceCodeKey {
 public:
     enum CodeType { EvalType, ProgramType, FunctionCallType, FunctionConstructType };
@@ -106,6 +78,8 @@ class SourceCodeKey {
 
     unsigned hash() const { return m_sourceString.impl()->hash(); }
 
+    size_t length() const { return m_sourceString.length(); }
+
     bool isNull() const { return m_sourceString.isNull(); }
 
     bool operator==(const SourceCodeKey& other) const
@@ -132,37 +106,73 @@ struct SourceCodeKeyHashTraits : SimpleClassHashTraits<SourceCodeKey> {
     static bool isEmptyValue(const SourceCodeKey& sourceCodeKey) { return sourceCodeKey.isNull(); }
 };
 
+class CodeCacheMap {
+    typedef HashMap<SourceCodeKey, Strong<JSCell>, SourceCodeKeyHash, SourceCodeKeyHashTraits> MapType;
+    typedef typename MapType::iterator iterator;
+
+public:
+    CodeCacheMap(size_t capacity)
+        : m_size(0)
+        , m_capacity(capacity)
+    {
+    }
+
+    const Strong<JSCell>* find(const SourceCodeKey& key)
+    {
+        iterator result = m_map.find(key);
+        if (result == m_map.end())
+            return 0;
+        return &result->value;
+    }
+
+    void set(const SourceCodeKey& key, const Strong<JSCell>& value)
+    {
+        while (m_size >= m_capacity) {
+            MapType::iterator it = m_map.begin();
+            m_size -= it->key.length();
+            m_map.remove(it);
+        }
+
+        m_size += key.length();
+        m_map.set(key, value);
+    }
+
+    void clear()
+    {
+        m_size = 0;
+        m_map.clear();
+    }
+
+private:
+    MapType m_map;
+    size_t m_size;
+    size_t m_capacity;
+};
+
+// Caches top-level code such as <script>, eval(), new Function, and JSEvaluateScript().
 class CodeCache {
 public:
     static PassOwnPtr<CodeCache> create() { return adoptPtr(new CodeCache); }
 
     UnlinkedProgramCodeBlock* getProgramCodeBlock(JSGlobalData&, ProgramExecutable*, const SourceCode&, JSParserStrictness, DebuggerMode, ProfilerMode, ParserError&);
     UnlinkedEvalCodeBlock* getEvalCodeBlock(JSGlobalData&, EvalExecutable*, const SourceCode&, JSParserStrictness, DebuggerMode, ProfilerMode, ParserError&);
-    UnlinkedFunctionCodeBlock* getFunctionCodeBlock(JSGlobalData&, UnlinkedFunctionExecutable*, const SourceCode&, CodeSpecializationKind, DebuggerMode, ProfilerMode, ParserError&);
     UnlinkedFunctionExecutable* getFunctionExecutableFromGlobalCode(JSGlobalData&, const Identifier&, const SourceCode&, ParserError&);
-    void usedFunctionCode(JSGlobalData&, UnlinkedFunctionCodeBlock*);
     ~CodeCache();
 
     void clear()
     {
         m_sourceCode.clear();
-        m_recentlyUsedFunctions.clear();
     }
 
 private:
     CodeCache();
 
-    UnlinkedFunctionCodeBlock* generateFunctionCodeBlock(JSGlobalData&, UnlinkedFunctionExecutable*, const SourceCode&, CodeSpecializationKind, DebuggerMode, ProfilerMode, ParserError&);
-
-    template <class UnlinkedCodeBlockType, class ExecutableType> inline UnlinkedCodeBlockType* getCodeBlock(JSGlobalData&, ExecutableType*, const SourceCode&, JSParserStrictness, DebuggerMode, ProfilerMode, ParserError&);
+    template <class UnlinkedCodeBlockType, class ExecutableType> 
+    UnlinkedCodeBlockType* getCodeBlock(JSGlobalData&, ExecutableType*, const SourceCode&, JSParserStrictness, DebuggerMode, ProfilerMode, ParserError&);
 
-    enum {
-        MaxRootEntries = 1280, // Top-level code such as <script>, eval(), JSEvaluateScript(), etc.
-        MaxChildFunctionEntries = MaxRootEntries * 8 // Sampling shows that each root holds about 6 functions. 8 is enough to usually cache all the child functions for each top-level entry.
-    };
+    enum { CacheSize = 16000000 }; // Size in characters
 
-    CacheMap<MaxRootEntries, SourceCodeKey, Strong<JSCell>, SourceCodeKeyHash, SourceCodeKeyHashTraits> m_sourceCode;
-    CacheMap<MaxChildFunctionEntries, UnlinkedFunctionCodeBlock*, Strong<UnlinkedFunctionCodeBlock> > m_recentlyUsedFunctions;
+    CodeCacheMap m_sourceCode;
 };
 
 }