Diff
Modified: branches/dfgFourthTier/Source/_javascript_Core/ChangeLog (149517 => 149518)
--- branches/dfgFourthTier/Source/_javascript_Core/ChangeLog 2013-05-03 05:01:03 UTC (rev 149517)
+++ branches/dfgFourthTier/Source/_javascript_Core/ChangeLog 2013-05-03 06:46:52 UTC (rev 149518)
@@ -1,5 +1,35 @@
2013-05-02 Filip Pizlo <[email protected]>
+ 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 <[email protected]>
+
fourthTier: DFG tries to ref/deref StringImpls in a ton of places
https://bugs.webkit.org/show_bug.cgi?id=115300
Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGGraph.cpp (149517 => 149518)
--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGGraph.cpp 2013-05-03 05:01:03 UTC (rev 149517)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGGraph.cpp 2013-05-03 06:46:52 UTC (rev 149518)
@@ -47,7 +47,7 @@
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)
Modified: branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGJITCompiler.cpp (149517 => 149518)
--- branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGJITCompiler.cpp 2013-05-03 05:01:03 UTC (rev 149517)
+++ branches/dfgFourthTier/Source/_javascript_Core/dfg/DFGJITCompiler.cpp 2013-05-03 06:46:52 UTC (rev 149518)
@@ -286,8 +286,10 @@
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;
@@ -386,8 +388,10 @@
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());
Modified: branches/dfgFourthTier/Source/_javascript_Core/jit/JIT.cpp (149517 => 149518)
--- branches/dfgFourthTier/Source/_javascript_Core/jit/JIT.cpp 2013-05-03 05:01:03 UTC (rev 149517)
+++ branches/dfgFourthTier/Source/_javascript_Core/jit/JIT.cpp 2013-05-03 06:46:52 UTC (rev 149518)
@@ -649,7 +649,10 @@
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);
}
@@ -839,8 +842,10 @@
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();
Modified: branches/dfgFourthTier/Source/_javascript_Core/profiler/ProfilerBytecodes.h (149517 => 149518)
--- branches/dfgFourthTier/Source/_javascript_Core/profiler/ProfilerBytecodes.h 2013-05-03 05:01:03 UTC (rev 149517)
+++ branches/dfgFourthTier/Source/_javascript_Core/profiler/ProfilerBytecodes.h 2013-05-03 06:46:52 UTC (rev 149518)
@@ -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>
Modified: branches/dfgFourthTier/Source/_javascript_Core/profiler/ProfilerDatabase.cpp (149517 => 149518)
--- branches/dfgFourthTier/Source/_javascript_Core/profiler/ProfilerDatabase.cpp 2013-05-03 05:01:03 UTC (rev 149517)
+++ branches/dfgFourthTier/Source/_javascript_Core/profiler/ProfilerDatabase.cpp 2013-05-03 06:46:52 UTC (rev 149518)
@@ -44,6 +44,8 @@
Bytecodes* Database::ensureBytecodesFor(CodeBlock* codeBlock)
{
+ Locker locker(m_lock);
+
codeBlock = codeBlock->baselineVersion();
HashMap<CodeBlock*, Bytecodes*>::iterator iter = m_bytecodesMap.find(codeBlock);
@@ -60,21 +62,18 @@
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
{
JSObject* result = constructEmptyObject(exec);
Modified: branches/dfgFourthTier/Source/_javascript_Core/profiler/ProfilerDatabase.h (149517 => 149518)
--- branches/dfgFourthTier/Source/_javascript_Core/profiler/ProfilerDatabase.h 2013-05-03 05:01:03 UTC (rev 149517)
+++ branches/dfgFourthTier/Source/_javascript_Core/profiler/ProfilerDatabase.h 2013-05-03 06:46:52 UTC (rev 149518)
@@ -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
@@ -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 {
@@ -48,8 +49,7 @@
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
@@ -66,11 +66,25 @@
JS_EXPORT_PRIVATE bool save(const char* filename) const;
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;
+
VM& m_vm;
SegmentedVector<Bytecodes> m_bytecodes;
HashMap<CodeBlock*, Bytecodes*> m_bytecodesMap;
Vector<RefPtr<Compilation> > m_compilations;
+ Lock m_lock;
};
} } // namespace JSC::Profiler
