Commit 55cf8267 authored by ggaren@apple.com's avatar ggaren@apple.com

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.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@143949 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 3e229631
2013-02-25 Geoffrey Garen <ggaren@apple.com>
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.
2013-02-25 Carlos Garcia Campos <cgarcia@igalia.com>
[BlackBerry][ARM] Fix cast-align warnings in JavaScriptCore
......
......@@ -36,6 +36,15 @@
namespace JSC {
void CodeCacheMap::pruneSlowCase()
{
while (m_size >= m_capacity) {
MapType::iterator it = m_map.begin();
m_size -= it->key.length();
m_map.remove(it);
}
}
CodeCache::CodeCache()
{
}
......@@ -60,22 +69,21 @@ template <class UnlinkedCodeBlockType, class ExecutableType>
UnlinkedCodeBlockType* CodeCache::getCodeBlock(JSGlobalData& globalData, ExecutableType* executable, const SourceCode& source, JSParserStrictness strictness, DebuggerMode debuggerMode, ProfilerMode profilerMode, ParserError& error)
{
SourceCodeKey key = SourceCodeKey(source, String(), CacheTypes<UnlinkedCodeBlockType>::codeType, strictness);
bool storeInCache = false;
if (debuggerMode == DebuggerOff && profilerMode == ProfilerOff) {
const Strong<JSCell>* result = m_sourceCode.find(key);
if (result) {
UnlinkedCodeBlockType* unlinkedCode = jsCast<UnlinkedCodeBlockType*>(result->get());
unsigned firstLine = source.firstLine() + unlinkedCode->firstLine();
executable->recordParse(unlinkedCode->codeFeatures(), unlinkedCode->hasCapturedVariables(), firstLine, firstLine + unlinkedCode->lineCount());
return unlinkedCode;
}
storeInCache = true;
CodeCacheMap::AddResult addResult = m_sourceCode.add(key, SourceCodeValue());
bool canCache = debuggerMode == DebuggerOff && profilerMode == ProfilerOff;
if (!addResult.isNewEntry && canCache) {
UnlinkedCodeBlockType* unlinkedCode = jsCast<UnlinkedCodeBlockType*>(addResult.iterator->value.cell.get());
unsigned firstLine = source.firstLine() + unlinkedCode->firstLine();
executable->recordParse(unlinkedCode->codeFeatures(), unlinkedCode->hasCapturedVariables(), firstLine, firstLine + unlinkedCode->lineCount());
return unlinkedCode;
}
typedef typename CacheTypes<UnlinkedCodeBlockType>::RootNode RootNode;
RefPtr<RootNode> rootNode = parse<RootNode>(&globalData, source, 0, Identifier(), strictness, JSParseProgramCode, error);
if (!rootNode)
if (!rootNode) {
m_sourceCode.remove(addResult.iterator);
return 0;
}
executable->recordParse(rootNode->features(), rootNode->hasCapturedVariables(), rootNode->lineNo(), rootNode->lastLine());
UnlinkedCodeBlockType* unlinkedCode = UnlinkedCodeBlockType::create(&globalData, executable->executableInfo());
......@@ -83,12 +91,17 @@ UnlinkedCodeBlockType* CodeCache::getCodeBlock(JSGlobalData& globalData, Executa
OwnPtr<BytecodeGenerator> generator(adoptPtr(new BytecodeGenerator(globalData, rootNode.get(), unlinkedCode, debuggerMode, profilerMode)));
error = generator->generate();
rootNode->destroyData();
if (error.m_type != ParserError::ErrorNone)
if (error.m_type != ParserError::ErrorNone) {
m_sourceCode.remove(addResult.iterator);
return 0;
}
if (storeInCache)
m_sourceCode.set(key, Strong<UnlinkedCodeBlock>(globalData, unlinkedCode));
if (!canCache) {
m_sourceCode.remove(addResult.iterator);
return unlinkedCode;
}
addResult.iterator->value = SourceCodeValue(globalData, unlinkedCode, m_sourceCode.age());
return unlinkedCode;
}
......@@ -105,13 +118,14 @@ UnlinkedEvalCodeBlock* CodeCache::getEvalCodeBlock(JSGlobalData& globalData, Eva
UnlinkedFunctionExecutable* CodeCache::getFunctionExecutableFromGlobalCode(JSGlobalData& globalData, const Identifier& name, const SourceCode& source, ParserError& error)
{
SourceCodeKey key = SourceCodeKey(source, name.string(), SourceCodeKey::FunctionType, JSParseNormal);
const Strong<JSCell>* result = m_sourceCode.find(key);
if (result)
return jsCast<UnlinkedFunctionExecutable*>(result->get());
CodeCacheMap::AddResult addResult = m_sourceCode.add(key, SourceCodeValue());
if (!addResult.isNewEntry)
return jsCast<UnlinkedFunctionExecutable*>(addResult.iterator->value.cell.get());
RefPtr<ProgramNode> program = parse<ProgramNode>(&globalData, source, 0, Identifier(), JSParseNormal, JSParseProgramCode, error);
if (!program) {
ASSERT(error.m_type != ParserError::ErrorNone);
m_sourceCode.remove(addResult.iterator);
return 0;
}
......@@ -129,7 +143,7 @@ UnlinkedFunctionExecutable* CodeCache::getFunctionExecutableFromGlobalCode(JSGlo
UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&globalData, source, body);
functionExecutable->m_nameValue.set(globalData, functionExecutable, jsString(&globalData, name.string()));
m_sourceCode.set(key, Strong<UnlinkedFunctionExecutable>(globalData, functionExecutable));
addResult.iterator->value = SourceCodeValue(globalData, functionExecutable, m_sourceCode.age());
return functionExecutable;
}
......
......@@ -118,8 +118,8 @@ struct SourceCodeValue {
{
}
SourceCodeValue(const Strong<JSCell>& cell, int64_t age)
: cell(cell)
SourceCodeValue(JSGlobalData& globalData, JSCell* cell, int64_t age)
: cell(globalData, cell)
, age(age)
{
}
......@@ -129,10 +129,11 @@ struct SourceCodeValue {
};
class CodeCacheMap {
public:
typedef HashMap<SourceCodeKey, SourceCodeValue, SourceCodeKeyHash, SourceCodeKeyHashTraits> MapType;
typedef MapType::iterator iterator;
typedef MapType::AddResult AddResult;
public:
enum { MinCacheCapacity = 1000000 }; // Size in characters
CodeCacheMap()
......@@ -142,13 +143,19 @@ public:
{
}
const Strong<JSCell>* find(const SourceCodeKey& key)
AddResult add(const SourceCodeKey& key, const SourceCodeValue& value)
{
iterator it = m_map.find(key);
if (it == m_map.end())
return 0;
prune();
AddResult addResult = m_map.add(key, value);
if (addResult.isNewEntry) {
m_size += key.length();
m_age += key.length();
return addResult;
}
int64_t age = m_age - it->value.age;
int64_t age = m_age - addResult.iterator->value.age;
if (age > m_capacity) {
// A requested object is older than the cache's capacity. We can
// infer that requested objects are subject to high eviction probability,
......@@ -163,18 +170,15 @@ public:
m_capacity = MinCacheCapacity;
}
it->value.age = m_age;
addResult.iterator->value.age = m_age;
m_age += key.length();
return &it->value.cell;
return addResult;
}
void set(const SourceCodeKey& key, const Strong<JSCell>& value)
void remove(iterator it)
{
pruneIfNeeded();
m_size += key.length();
m_age += key.length();
m_map.set(key, SourceCodeValue(value, m_age));
m_size -= it->key.length();
m_map.remove(it);
}
void clear()
......@@ -184,6 +188,8 @@ public:
m_map.clear();
}
int64_t age() { return m_age; }
private:
// This constant factor biases cache capacity toward recent activity. We
// want to adapt to changing workloads.
......@@ -194,13 +200,12 @@ private:
// sample them, so we need to extrapolate from the ones we do sample.
static const int64_t oldObjectSamplingMultiplier = 32;
void pruneIfNeeded()
void pruneSlowCase();
void prune()
{
while (m_size >= m_capacity) {
MapType::iterator it = m_map.begin();
m_size -= it->key.length();
m_map.remove(it);
}
if (m_size < m_capacity)
return;
pruneSlowCase();
}
MapType m_map;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment