Skip to content

Commit efc8e5b

Browse files
saambaratisaambarati
committed
[ Catalina Release JSC] A large number of JSC test appear to be flaky failing
https://bugs.webkit.org/show_bug.cgi?id=225998 <rdar://problem/78235001> Reviewed by Yusuke Suzuki. JSTests: * stress/dont-link-virtual-calls-on-compiler-thread.js: Added. Source/JavaScriptCore: This patch is fixing some fallout from moving JIT::link() to a background thread: 1. We can't shrink the CodeBlock's constant pool on a background thread since we read from it without grabbing a lock on the main thread (when reading things off the stack in slow path calls). 2. We can't create GCAwareJITStubRoutines on the compilation thread, since creating a GCAwareJITStubRoutines adds to a global hash table inside Heap. This means that we have to do that step of emitting virtual calls for eval when we're finalizing code on the main thread. This patch also makes it so that a baseline JIT compilation thread is correctly marked as such. * heap/JITStubRoutineSet.cpp: (JSC::JITStubRoutineSet::add): * jit/AssemblyHelpers.cpp: (JSC::AssemblyHelpers::emitUnlinkedVirtualCall): (JSC::AssemblyHelpers::emitVirtualCall): * jit/AssemblyHelpers.h: * jit/JIT.cpp: (JSC::JIT::link): (JSC::JIT::finalizeOnMainThread): * jit/JIT.h: * jit/JITCall.cpp: (JSC::JIT::compileCallEvalSlowCase): * jit/JITCall32_64.cpp: (JSC::JIT::compileCallEvalSlowCase): * jit/JITWorklist.cpp: Canonical link: https://commits.webkit.org/237992@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@277850 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 60dd2d1 commit efc8e5b

13 files changed

Lines changed: 107 additions & 10 deletions

‎JSTests/ChangeLog‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
2021-05-20 Saam Barati <sbarati@apple.com>
2+
3+
[ Catalina Release JSC] A large number of JSC test appear to be flaky failing
4+
https://bugs.webkit.org/show_bug.cgi?id=225998
5+
<rdar://problem/78235001>
6+
7+
Reviewed by Yusuke Suzuki.
8+
9+
* stress/dont-link-virtual-calls-on-compiler-thread.js: Added.
10+
111
2021-05-19 Robin Morisset <rmorisset@apple.com>
212

313
Fix typo in AirUseCounts

‎JSTests/stress/dont-link-virtual-calls-on-compiler-thread.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
//@ skip if $architecture != "arm64" and $architecture != "x86-64"
2+
3+
let s = `
4+
for (let i = 0; i < 10000; i++) {
5+
eval?.('eval("function f() {}");');
6+
}
7+
`;
8+
9+
for (let i = 0; i < 5; ++i)
10+
runString(s);

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,40 @@
1+
2021-05-20 Saam Barati <sbarati@apple.com>
2+
3+
[ Catalina Release JSC] A large number of JSC test appear to be flaky failing
4+
https://bugs.webkit.org/show_bug.cgi?id=225998
5+
<rdar://problem/78235001>
6+
7+
Reviewed by Yusuke Suzuki.
8+
9+
This patch is fixing some fallout from moving JIT::link() to a background
10+
thread:
11+
1. We can't shrink the CodeBlock's constant pool on a background thread
12+
since we read from it without grabbing a lock on the main thread (when
13+
reading things off the stack in slow path calls).
14+
2. We can't create GCAwareJITStubRoutines on the compilation thread, since
15+
creating a GCAwareJITStubRoutines adds to a global hash table inside Heap. This
16+
means that we have to do that step of emitting virtual calls for eval when
17+
we're finalizing code on the main thread.
18+
19+
This patch also makes it so that a baseline JIT compilation thread is
20+
correctly marked as such.
21+
22+
* heap/JITStubRoutineSet.cpp:
23+
(JSC::JITStubRoutineSet::add):
24+
* jit/AssemblyHelpers.cpp:
25+
(JSC::AssemblyHelpers::emitUnlinkedVirtualCall):
26+
(JSC::AssemblyHelpers::emitVirtualCall):
27+
* jit/AssemblyHelpers.h:
28+
* jit/JIT.cpp:
29+
(JSC::JIT::link):
30+
(JSC::JIT::finalizeOnMainThread):
31+
* jit/JIT.h:
32+
* jit/JITCall.cpp:
33+
(JSC::JIT::compileCallEvalSlowCase):
34+
* jit/JITCall32_64.cpp:
35+
(JSC::JIT::compileCallEvalSlowCase):
36+
* jit/JITWorklist.cpp:
37+
138
2021-05-20 Saam Barati <sbarati@apple.com>
239

340
Make polymorphic calls play nice with Data Call ICs

‎Source/JavaScriptCore/assembler/LinkBuffer.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,6 @@ void LinkBuffer::performFinalization()
476476
task->run(*this);
477477

478478
#ifndef NDEBUG
479-
ASSERT(m_isJumpIsland || !isCompilationThread());
480479
ASSERT(!m_completed);
481480
ASSERT(isValid());
482481
m_completed = true;

‎Source/JavaScriptCore/heap/JITStubRoutineSet.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ JITStubRoutineSet::~JITStubRoutineSet()
5757

5858
void JITStubRoutineSet::add(GCAwareJITStubRoutine* routine)
5959
{
60+
RELEASE_ASSERT(!isCompilationThread());
6061
ASSERT(!routine->m_isJettisoned);
6162

6263
m_routines.append(Routine {

‎Source/JavaScriptCore/jit/AssemblyHelpers.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -643,11 +643,16 @@ void AssemblyHelpers::restoreCalleeSavesFromEntryFrameCalleeSavesBuffer(EntryFra
643643
#endif
644644
}
645645

646-
void AssemblyHelpers::emitVirtualCall(VM& vm, JSGlobalObject* globalObject, CallLinkInfo* info)
646+
AssemblyHelpers::Call AssemblyHelpers::emitUnlinkedVirtualCall(JSGlobalObject* globalObject, CallLinkInfo* info)
647647
{
648648
move(TrustedImmPtr(info), GPRInfo::regT2);
649649
move(TrustedImmPtr(globalObject), GPRInfo::regT3);
650-
Call call = nearCall();
650+
return nearCall();
651+
}
652+
653+
void AssemblyHelpers::emitVirtualCall(VM& vm, JSGlobalObject* globalObject, CallLinkInfo* info)
654+
{
655+
Call call = emitUnlinkedVirtualCall(globalObject, info);
651656
addLinkTask(
652657
[=, &vm] (LinkBuffer& linkBuffer) {
653658
MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunk = virtualThunkFor(vm, *info);

‎Source/JavaScriptCore/jit/AssemblyHelpers.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1828,6 +1828,7 @@ class AssemblyHelpers : public MacroAssembler {
18281828
functor(TypeofType::Undefined, true);
18291829
}
18301830

1831+
Call emitUnlinkedVirtualCall(JSGlobalObject*, CallLinkInfo*);
18311832
void emitVirtualCall(VM&, JSGlobalObject*, CallLinkInfo*);
18321833

18331834
void makeSpaceOnStackForCCall();

‎Source/JavaScriptCore/jit/JIT.cpp‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -977,11 +977,6 @@ void JIT::link()
977977
patchBuffer, JSEntryPtrTag,
978978
"Baseline JIT code for %s", toCString(CodeBlockWithJITType(m_codeBlock, JITType::BaselineJIT)).data());
979979

980-
{
981-
ConcurrentJSLocker locker(m_codeBlock->m_lock);
982-
m_codeBlock->shrinkToFit(locker, CodeBlock::ShrinkMode::LateShrink);
983-
}
984-
985980
MacroAssemblerCodePtr<JSEntryPtrTag> withArityCheck = patchBuffer.locationOf<JSEntryPtrTag>(m_arityCheck);
986981
m_jitCode = adoptRef(*new DirectJITCode(result, withArityCheck, JITType::BaselineJIT));
987982

@@ -996,6 +991,20 @@ CompilationResult JIT::finalizeOnMainThread()
996991
if (!m_jitCode)
997992
return CompilationFailed;
998993

994+
for (auto pair : m_virtualCalls) {
995+
auto callLocation = m_linkBuffer->locationOfNearCall<JITThunkPtrTag>(pair.first);
996+
997+
CallLinkInfo& info = pair.second;
998+
MacroAssemblerCodeRef<JITStubRoutinePtrTag> virtualThunk = virtualThunkFor(*m_vm, info);
999+
info.setSlowStub(GCAwareJITStubRoutine::create(virtualThunk, *m_vm));
1000+
MacroAssembler::repatchNearCall(callLocation, CodeLocationLabel<JITStubRoutinePtrTag>(virtualThunk.code()));
1001+
}
1002+
1003+
{
1004+
ConcurrentJSLocker locker(m_codeBlock->m_lock);
1005+
m_codeBlock->shrinkToFit(locker, CodeBlock::ShrinkMode::LateShrink);
1006+
}
1007+
9991008
for (size_t i = 0; i < m_codeBlock->numberOfExceptionHandlers(); ++i) {
10001009
HandlerInfo& handler = m_codeBlock->exceptionHandler(i);
10011010
// FIXME: <rdar://problem/39433318>.

‎Source/JavaScriptCore/jit/JIT.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,8 @@ namespace JSC {
10561056
BytecodeIndex m_loopOSREntryBytecodeIndex;
10571057

10581058
RefPtr<DirectJITCode> m_jitCode;
1059+
1060+
Vector<std::pair<Call, CallLinkInfo&>> m_virtualCalls;
10591061
};
10601062

10611063
} // namespace JSC

‎Source/JavaScriptCore/jit/JITCall.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ void JIT::compileCallEvalSlowCase(const Instruction* instruction, Vector<SlowCas
156156
addPtr(TrustedImm32(registerOffset * sizeof(Register) + sizeof(CallerFrameAndPC)), callFrameRegister, stackPointerRegister);
157157

158158
load64(Address(stackPointerRegister, sizeof(Register) * CallFrameSlot::callee - sizeof(CallerFrameAndPC)), regT0);
159-
emitVirtualCall(vm(), m_codeBlock->globalObject(), info);
159+
m_virtualCalls.append(std::pair<Call, CallLinkInfo&> { emitUnlinkedVirtualCall(m_codeBlock->globalObject(), info), *info });
160160
addPtr(TrustedImm32(stackPointerOffsetFor(m_codeBlock) * sizeof(Register)), callFrameRegister, stackPointerRegister);
161161
checkStackPointerAlignment();
162162

0 commit comments

Comments
 (0)