Commit 4bba8c0b authored by oliver@apple.com's avatar oliver@apple.com

fourthTier: Profiler should be thread-safe

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

Reviewed by Geoffrey Garen.

Change the Profiler::Database API for Compilation creation so that we don't add
it to the Database until it's completely constructed. This prevents the Database
from seeing Compilations that are being concurrently constructed.

Change the Profiler::Database itself to do locking for creation of Bytecodes and
for modifying the map. This map may be consulted by both the main thread and the
concurrent thread.

* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::Graph):
* dfg/DFGJITCompiler.cpp:
(JSC::DFG::JITCompiler::link):
(JSC::DFG::JITCompiler::linkFunction):
* jit/JIT.cpp:
(JSC::JIT::privateCompile):
* profiler/ProfilerBytecodes.h:
* profiler/ProfilerDatabase.cpp:
(JSC::Profiler::Database::ensureBytecodesFor):
(JSC::Profiler::Database::notifyDestruction):
(JSC::Profiler::Database::addCompilation):
* profiler/ProfilerDatabase.h:
(Database):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@153143 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 02039469
2013-05-02 Filip Pizlo <fpizlo@apple.com>
fourthTier: Profiler should be thread-safe
https://bugs.webkit.org/show_bug.cgi?id=115445
Reviewed by Geoffrey Garen.
Change the Profiler::Database API for Compilation creation so that we don't add
it to the Database until it's completely constructed. This prevents the Database
from seeing Compilations that are being concurrently constructed.
Change the Profiler::Database itself to do locking for creation of Bytecodes and
for modifying the map. This map may be consulted by both the main thread and the
concurrent thread.
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::Graph):
* dfg/DFGJITCompiler.cpp:
(JSC::DFG::JITCompiler::link):
(JSC::DFG::JITCompiler::linkFunction):
* jit/JIT.cpp:
(JSC::JIT::privateCompile):
* profiler/ProfilerBytecodes.h:
* profiler/ProfilerDatabase.cpp:
(JSC::Profiler::Database::ensureBytecodesFor):
(JSC::Profiler::Database::notifyDestruction):
(JSC::Profiler::Database::addCompilation):
* profiler/ProfilerDatabase.h:
(Database):
2013-05-02 Filip Pizlo <fpizlo@apple.com>
fourthTier: DFG tries to ref/deref StringImpls in a ton of places
......
......@@ -47,7 +47,7 @@ static const char* dfgOpNames[] = {
Graph::Graph(VM& vm, CodeBlock* codeBlock, unsigned osrEntryBytecodeIndex, const Operands<JSValue>& mustHandleValues)
: m_vm(vm)
, m_codeBlock(codeBlock)
, m_compilation(vm.m_perBytecodeProfiler ? vm.m_perBytecodeProfiler->newCompilation(codeBlock, Profiler::DFG) : 0)
, m_compilation(vm.m_perBytecodeProfiler ? adoptRef(new Profiler::Compilation(vm.m_perBytecodeProfiler->ensureBytecodesFor(codeBlock), Profiler::DFG)) : 0)
, m_profiledBlock(codeBlock->alternative())
, m_allocator(vm.m_dfgState->m_allocator)
, m_hasArguments(false)
......
......@@ -286,8 +286,10 @@ bool JITCompiler::link(RefPtr<JSC::JITCode>& entry)
if (shouldShowDisassembly())
m_disassembler->dump(linkBuffer);
if (m_graph.m_compilation)
if (m_graph.m_compilation) {
m_disassembler->reportToProfiler(m_graph.m_compilation.get(), linkBuffer);
m_vm->m_perBytecodeProfiler->addCompilation(m_graph.m_compilation);
}
m_jitCode->initializeCodeRef(linkBuffer.finalizeCodeWithoutDisassembly());
entry = m_jitCode;
......@@ -389,8 +391,10 @@ bool JITCompiler::linkFunction(RefPtr<JSC::JITCode>& entry, MacroAssemblerCodePt
if (shouldShowDisassembly())
m_disassembler->dump(linkBuffer);
if (m_graph.m_compilation)
if (m_graph.m_compilation) {
m_disassembler->reportToProfiler(m_graph.m_compilation.get(), linkBuffer);
m_vm->m_perBytecodeProfiler->addCompilation(m_graph.m_compilation);
}
entryWithArityCheck = linkBuffer.locationOf(m_arityCheck);
m_jitCode->initializeCodeRef(linkBuffer.finalizeCodeWithoutDisassembly());
......
......@@ -594,7 +594,10 @@ PassRefPtr<JITCode> JIT::privateCompile(CodePtr* functionEntryArityCheck, JITCom
if (Options::showDisassembly() || m_vm->m_perBytecodeProfiler)
m_disassembler = adoptPtr(new JITDisassembler(m_codeBlock));
if (m_vm->m_perBytecodeProfiler) {
m_compilation = m_vm->m_perBytecodeProfiler->newCompilation(m_codeBlock, Profiler::Baseline);
m_compilation = adoptRef(
new Profiler::Compilation(
m_vm->m_perBytecodeProfiler->ensureBytecodesFor(m_codeBlock),
Profiler::Baseline));
m_compilation->addProfiledBytecodes(*m_vm->m_perBytecodeProfiler, m_codeBlock);
}
......@@ -784,8 +787,10 @@ PassRefPtr<JITCode> JIT::privateCompile(CodePtr* functionEntryArityCheck, JITCom
if (Options::showDisassembly())
m_disassembler->dump(patchBuffer);
if (m_compilation)
if (m_compilation) {
m_disassembler->reportToProfiler(m_compilation.get(), patchBuffer);
m_vm->m_perBytecodeProfiler->addCompilation(m_compilation);
}
CodeRef result = patchBuffer.finalizeCodeWithoutDisassembly();
......
......@@ -29,6 +29,7 @@
#include "CodeBlockHash.h"
#include "JSCJSValue.h"
#include "ProfilerBytecodeSequence.h"
#include <wtf/ByteSpinLock.h>
#include <wtf/PrintStream.h>
#include <wtf/text/WTFString.h>
......
......@@ -60,6 +60,8 @@ Database::~Database()
Bytecodes* Database::ensureBytecodesFor(CodeBlock* codeBlock)
{
Locker locker(m_lock);
codeBlock = codeBlock->baselineVersion();
HashMap<CodeBlock*, Bytecodes*>::iterator iter = m_bytecodesMap.find(codeBlock);
......@@ -76,19 +78,16 @@ Bytecodes* Database::ensureBytecodesFor(CodeBlock* codeBlock)
void Database::notifyDestruction(CodeBlock* codeBlock)
{
Locker locker(m_lock);
m_bytecodesMap.remove(codeBlock);
}
PassRefPtr<Compilation> Database::newCompilation(Bytecodes* bytecodes, CompilationKind kind)
void Database::addCompilation(PassRefPtr<Compilation> compilation)
{
RefPtr<Compilation> compilation = adoptRef(new Compilation(bytecodes, kind));
ASSERT(!isCompilationThread());
m_compilations.append(compilation);
return compilation.release();
}
PassRefPtr<Compilation> Database::newCompilation(CodeBlock* codeBlock, CompilationKind kind)
{
return newCompilation(ensureBytecodesFor(codeBlock), kind);
}
JSValue Database::toJS(ExecState* exec) const
......
......@@ -35,6 +35,7 @@
#include <wtf/Noncopyable.h>
#include <wtf/PassRefPtr.h>
#include <wtf/SegmentedVector.h>
#include <wtf/ThreadingPrimitives.h>
#include <wtf/text/WTFString.h>
namespace JSC { namespace Profiler {
......@@ -50,8 +51,7 @@ public:
Bytecodes* ensureBytecodesFor(CodeBlock*);
void notifyDestruction(CodeBlock*);
PassRefPtr<Compilation> newCompilation(CodeBlock*, CompilationKind);
PassRefPtr<Compilation> newCompilation(Bytecodes*, CompilationKind);
void addCompilation(PassRefPtr<Compilation>);
// Converts the database to a JavaScript object that is suitable for JSON stringification.
// Note that it's probably a good idea to use an ExecState* associated with a global
......@@ -70,6 +70,20 @@ public:
void registerToSaveAtExit(const char* filename);
private:
// Use a full-blown adaptive mutex because:
// - There is only one ProfilerDatabase per VM. The size overhead of the system's
// mutex is negligible if you only have one of them.
// - It's locked infrequently - once per bytecode generation, compilation, and
// code block collection - so the fact that the fast path still requires a
// function call is neglible.
// - It tends to be held for a while. Currently, we hold it while generating
// Profiler::Bytecodes for a CodeBlock. That's uncommon and shouldn't affect
// performance, but if we did have contention, we would want a sensible,
// power-aware backoff. An adaptive mutex will do this as a matter of course,
// but a spinlock won't.
typedef Mutex Lock;
typedef MutexLocker Locker;
void addDatabaseToAtExit();
void removeDatabaseFromAtExit();
......@@ -85,6 +99,7 @@ private:
bool m_shouldSaveAtExit;
CString m_atExitSaveFilename;
Database* m_nextRegisteredDatabase;
Lock m_lock;
};
} } // namespace JSC::Profiler
......
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