Skip to content

Commit 596d55a

Browse files
jupvfrancoCommit Bot
jupvfranco
authored and
Commit Bot
committed
Deoptimization and multithreading.
When using Lockers and Unlockers it is possible to create a scenario where multiple threads point to the same optimized code object. When that happens, if one of the threads triggers deoptimization, then the stack replacement needs to happen in the stacks of all threads. With this CL, the deoptimizer visits all threads to do so. The CL also adds three tests where V8 used to crash due to this issue. Bug: v8:6563 Change-Id: I74e9af472d4833aa8d13e579df45133791f6a503 Reviewed-on: https://chromium-review.googlesource.com/670783 Reviewed-by: Jaroslav Sevcik <[email protected]> Commit-Queue: Juliana Patricia Vicente Franco <[email protected]> Cr-Commit-Position: refs/heads/master@{#48060}
1 parent 8c89502 commit 596d55a

File tree

3 files changed

+291
-44
lines changed

3 files changed

+291
-44
lines changed

‎src/deoptimizer.cc‎

Lines changed: 54 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -144,15 +144,59 @@ void Deoptimizer::GenerateDeoptimizationEntries(MacroAssembler* masm,
144144
generator.Generate();
145145
}
146146

147+
namespace {
148+
class ActivationsFinder : public ThreadVisitor {
149+
public:
150+
explicit ActivationsFinder(std::set<Code*>* codes,
151+
Code* topmost_optimized_code,
152+
bool safe_to_deopt_topmost_optimized_code)
153+
: codes_(codes) {
154+
#ifdef DEBUG
155+
topmost_ = topmost_optimized_code;
156+
safe_to_deopt_ = safe_to_deopt_topmost_optimized_code;
157+
#endif
158+
}
159+
160+
// Find the frames with activations of codes marked for deoptimization, search
161+
// for the trampoline to the deoptimizer call respective to each code, and use
162+
// it to replace the current pc on the stack.
163+
void VisitThread(Isolate* isolate, ThreadLocalTop* top) {
164+
for (StackFrameIterator it(isolate, top); !it.done(); it.Advance()) {
165+
if (it.frame()->type() == StackFrame::OPTIMIZED) {
166+
Code* code = it.frame()->LookupCode();
167+
if (code->kind() == Code::OPTIMIZED_FUNCTION &&
168+
code->marked_for_deoptimization()) {
169+
codes_->erase(code);
170+
// Obtain the trampoline to the deoptimizer call.
171+
SafepointEntry safepoint = code->GetSafepointEntry(it.frame()->pc());
172+
int trampoline_pc = safepoint.trampoline_pc();
173+
DCHECK_IMPLIES(code == topmost_, safe_to_deopt_);
174+
// Replace the current pc on the stack with the trampoline.
175+
it.frame()->set_pc(code->instruction_start() + trampoline_pc);
176+
}
177+
}
178+
}
179+
}
180+
181+
private:
182+
std::set<Code*>* codes_;
183+
184+
#ifdef DEBUG
185+
Code* topmost_;
186+
bool safe_to_deopt_;
187+
#endif
188+
};
189+
} // namespace
190+
147191
// Move marked code from the optimized code list to the deoptimized code list,
148192
// and replace pc on the stack for codes marked for deoptimization.
149193
void Deoptimizer::DeoptimizeMarkedCodeForContext(Context* context) {
150194
DisallowHeapAllocation no_allocation;
151195

152196
Isolate* isolate = context->GetHeap()->isolate();
153-
#ifdef DEBUG
154197
Code* topmost_optimized_code = NULL;
155198
bool safe_to_deopt_topmost_optimized_code = false;
199+
#ifdef DEBUG
156200
// Make sure all activations of optimized code can deopt at their current PC.
157201
// The topmost optimized code has special handling because it cannot be
158202
// deoptimized due to weak object dependency.
@@ -227,28 +271,16 @@ void Deoptimizer::DeoptimizeMarkedCodeForContext(Context* context) {
227271
element = next;
228272
}
229273

230-
// Find the frames with activations of codes marked for deoptimization, search
231-
// for the trampoline to the deoptimizer call respective to each code, and use
232-
// it to replace the current pc on the stack.
233-
for (StackFrameIterator it(isolate, isolate->thread_local_top()); !it.done();
234-
it.Advance()) {
235-
if (it.frame()->type() == StackFrame::OPTIMIZED) {
236-
Code* code = it.frame()->LookupCode();
237-
if (code->kind() == Code::OPTIMIZED_FUNCTION &&
238-
code->marked_for_deoptimization()) {
239-
codes.erase(code);
240-
// Obtain the trampoline to the deoptimizer call.
241-
SafepointEntry safepoint = code->GetSafepointEntry(it.frame()->pc());
242-
int trampoline_pc = safepoint.trampoline_pc();
243-
DCHECK_IMPLIES(code == topmost_optimized_code,
244-
safe_to_deopt_topmost_optimized_code);
245-
// Replace the current pc on the stack with the trampoline.
246-
it.frame()->set_pc(code->instruction_start() + trampoline_pc);
247-
}
248-
}
249-
}
274+
ActivationsFinder visitor(&codes, topmost_optimized_code,
275+
safe_to_deopt_topmost_optimized_code);
276+
// Iterate over the stack of this thread.
277+
visitor.VisitThread(isolate, isolate->thread_local_top());
278+
// In addition to iterate over the stack of this thread, we also
279+
// need to consider all the other threads as they may also use
280+
// the code currently beings deoptimized.
281+
isolate->thread_manager()->IterateArchivedThreads(&visitor);
250282

251-
// If there's no activation of a code in the stack then we can remove its
283+
// If there's no activation of a code in any stack then we can remove its
252284
// deoptimization data. We do this to ensure that Code objects that will be
253285
// unlinked won't be kept alive.
254286
std::set<Code*>::iterator it;

‎src/runtime/runtime-compiler.cc‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -137,28 +137,6 @@ RUNTIME_FUNCTION(Runtime_NotifyStubFailure) {
137137
return isolate->heap()->undefined_value();
138138
}
139139

140-
class ActivationsFinder : public ThreadVisitor {
141-
public:
142-
Code* code_;
143-
bool has_code_activations_;
144-
145-
explicit ActivationsFinder(Code* code)
146-
: code_(code), has_code_activations_(false) {}
147-
148-
void VisitThread(Isolate* isolate, ThreadLocalTop* top) {
149-
JavaScriptFrameIterator it(isolate, top);
150-
VisitFrames(&it);
151-
}
152-
153-
void VisitFrames(JavaScriptFrameIterator* it) {
154-
for (; !it->done(); it->Advance()) {
155-
JavaScriptFrame* frame = it->frame();
156-
if (code_->contains(frame->pc())) has_code_activations_ = true;
157-
}
158-
}
159-
};
160-
161-
162140
RUNTIME_FUNCTION(Runtime_NotifyDeoptimized) {
163141
HandleScope scope(isolate);
164142
DCHECK_EQ(1, args.length());

‎test/cctest/test-lockers.cc‎

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,243 @@ using ::v8::String;
5454
using ::v8::Value;
5555
using ::v8::V8;
5656

57+
namespace {
58+
59+
class DeoptimizeCodeThread : public v8::base::Thread {
60+
public:
61+
DeoptimizeCodeThread(v8::Isolate* isolate, v8::Local<v8::Context> context,
62+
const char* trigger)
63+
: Thread(Options("DeoptimizeCodeThread")),
64+
isolate_(isolate),
65+
context_(isolate, context),
66+
source_(trigger) {}
67+
68+
void Run() {
69+
v8::Locker locker(isolate_);
70+
isolate_->Enter();
71+
v8::HandleScope handle_scope(isolate_);
72+
v8::Local<v8::Context> context =
73+
v8::Local<v8::Context>::New(isolate_, context_);
74+
v8::Context::Scope context_scope(context);
75+
CHECK_EQ(isolate_, v8::Isolate::GetCurrent());
76+
// This code triggers deoptimization of some function that will be
77+
// used in a different thread.
78+
CompileRun(source_);
79+
isolate_->Exit();
80+
}
81+
82+
private:
83+
v8::Isolate* isolate_;
84+
Persistent<v8::Context> context_;
85+
// The code that triggers the deoptimization.
86+
const char* source_;
87+
};
88+
89+
void UnlockForDeoptimization(const v8::FunctionCallbackInfo<v8::Value>& args) {
90+
v8::Isolate* isolate = v8::Isolate::GetCurrent();
91+
// Gets the pointer to the thread that will trigger the deoptimization of the
92+
// code.
93+
DeoptimizeCodeThread* deoptimizer =
94+
reinterpret_cast<DeoptimizeCodeThread*>(isolate->GetData(0));
95+
{
96+
// Exits and unlocks the isolate.
97+
isolate->Exit();
98+
v8::Unlocker unlocker(isolate);
99+
// Starts the deoptimizing thread.
100+
deoptimizer->Start();
101+
// Waits for deoptimization to finish.
102+
deoptimizer->Join();
103+
}
104+
// The deoptimizing thread has finished its work, and the isolate
105+
// will now be used by the current thread.
106+
isolate->Enter();
107+
}
108+
109+
void UnlockForDeoptimizationIfReady(
110+
const v8::FunctionCallbackInfo<v8::Value>& args) {
111+
v8::Isolate* isolate = v8::Isolate::GetCurrent();
112+
bool* ready_to_deoptimize = reinterpret_cast<bool*>(isolate->GetData(1));
113+
if (*ready_to_deoptimize) {
114+
// The test should enter here only once, so put the flag back to false.
115+
*ready_to_deoptimize = false;
116+
// Gets the pointer to the thread that will trigger the deoptimization of
117+
// the code.
118+
DeoptimizeCodeThread* deoptimizer =
119+
reinterpret_cast<DeoptimizeCodeThread*>(isolate->GetData(0));
120+
{
121+
// Exits and unlocks the thread.
122+
isolate->Exit();
123+
v8::Unlocker unlocker(isolate);
124+
// Starts the thread that deoptimizes the function.
125+
deoptimizer->Start();
126+
// Waits for the deoptimizing thread to finish.
127+
deoptimizer->Join();
128+
}
129+
// The deoptimizing thread has finished its work, and the isolate
130+
// will now be used by the current thread.
131+
isolate->Enter();
132+
}
133+
}
134+
} // namespace
135+
136+
TEST(LazyDeoptimizationMultithread) {
137+
i::FLAG_allow_natives_syntax = true;
138+
v8::Isolate::CreateParams create_params;
139+
create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
140+
v8::Isolate* isolate = v8::Isolate::New(create_params);
141+
{
142+
v8::Locker locker(isolate);
143+
v8::Isolate::Scope isolate_scope(isolate);
144+
v8::HandleScope scope(isolate);
145+
v8::Local<v8::Context> context = v8::Context::New(isolate);
146+
const char* trigger_deopt = "obj = { y: 0, x: 1 };";
147+
148+
// We use the isolate to pass arguments to the UnlockForDeoptimization
149+
// function. Namely, we pass a pointer to the deoptimizing thread.
150+
DeoptimizeCodeThread deoptimize_thread(isolate, context, trigger_deopt);
151+
isolate->SetData(0, &deoptimize_thread);
152+
v8::Context::Scope context_scope(context);
153+
154+
// Create the function templace for C++ code that is invoked from
155+
// JavaScript code.
156+
Local<v8::FunctionTemplate> fun_templ =
157+
v8::FunctionTemplate::New(isolate, UnlockForDeoptimization);
158+
Local<Function> fun = fun_templ->GetFunction(context).ToLocalChecked();
159+
CHECK(context->Global()
160+
->Set(context, v8_str("unlock_for_deoptimization"), fun)
161+
.FromJust());
162+
163+
// Optimizes a function f, which will be deoptimized in another
164+
// thread.
165+
CompileRun(
166+
"var b = false; var obj = { x: 1 };"
167+
"function f() { g(); return obj.x; }"
168+
"function g() { if (b) { unlock_for_deoptimization(); } }"
169+
"%NeverOptimizeFunction(g);"
170+
"f(); f(); %OptimizeFunctionOnNextCall(f);"
171+
"f();");
172+
173+
// Trigger the unlocking.
174+
Local<Value> v = CompileRun("b = true; f();");
175+
176+
// Once the isolate has been unlocked, the thread will wait for the
177+
// other thread to finish its task. Once this happens, this thread
178+
// continues with its execution, that is, with the execution of the
179+
// function g, which then returns to f. The function f should have
180+
// also been deoptimized. If the replacement did not happen on this
181+
// thread's stack, then the test will fail here.
182+
CHECK(v->IsNumber());
183+
CHECK_EQ(1, static_cast<int>(v->NumberValue(context).FromJust()));
184+
}
185+
isolate->Dispose();
186+
}
187+
188+
TEST(LazyDeoptimizationMultithreadWithNatives) {
189+
i::FLAG_allow_natives_syntax = true;
190+
v8::Isolate::CreateParams create_params;
191+
create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
192+
v8::Isolate* isolate = v8::Isolate::New(create_params);
193+
{
194+
v8::Locker locker(isolate);
195+
v8::Isolate::Scope isolate_scope(isolate);
196+
v8::HandleScope scope(isolate);
197+
v8::Local<v8::Context> context = v8::Context::New(isolate);
198+
const char* trigger_deopt = "%DeoptimizeFunction(f);";
199+
200+
// We use the isolate to pass arguments to the UnlockForDeoptimization
201+
// function. Namely, we pass a pointer to the deoptimizing thread.
202+
DeoptimizeCodeThread deoptimize_thread(isolate, context, trigger_deopt);
203+
isolate->SetData(0, &deoptimize_thread);
204+
bool ready_to_deopt = false;
205+
isolate->SetData(1, &ready_to_deopt);
206+
v8::Context::Scope context_scope(context);
207+
208+
// Create the function templace for C++ code that is invoked from
209+
// JavaScript code.
210+
Local<v8::FunctionTemplate> fun_templ =
211+
v8::FunctionTemplate::New(isolate, UnlockForDeoptimizationIfReady);
212+
Local<Function> fun = fun_templ->GetFunction(context).ToLocalChecked();
213+
CHECK(context->Global()
214+
->Set(context, v8_str("unlock_for_deoptimization"), fun)
215+
.FromJust());
216+
217+
// Optimizes a function f, which will be deoptimized in another
218+
// thread.
219+
CompileRun(
220+
"var obj = { x: 1 };"
221+
"function f() { g(); return obj.x;}"
222+
"function g() { "
223+
" unlock_for_deoptimization(); }"
224+
"%NeverOptimizeFunction(g);"
225+
"f(); f(); %OptimizeFunctionOnNextCall(f);");
226+
227+
// Trigger the unlocking.
228+
ready_to_deopt = true;
229+
isolate->SetData(1, &ready_to_deopt);
230+
Local<Value> v = CompileRun("f();");
231+
232+
// Once the isolate has been unlocked, the thread will wait for the
233+
// other thread to finish its task. Once this happens, this thread
234+
// continues with its execution, that is, with the execution of the
235+
// function g, which then returns to f. The function f should have
236+
// also been deoptimized. Otherwise, the test will fail here.
237+
CHECK(v->IsNumber());
238+
CHECK_EQ(1, static_cast<int>(v->NumberValue(context).FromJust()));
239+
}
240+
isolate->Dispose();
241+
}
242+
243+
TEST(EagerDeoptimizationMultithread) {
244+
i::FLAG_allow_natives_syntax = true;
245+
v8::Isolate::CreateParams create_params;
246+
create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
247+
v8::Isolate* isolate = v8::Isolate::New(create_params);
248+
{
249+
v8::Locker locker(isolate);
250+
v8::Isolate::Scope isolate_scope(isolate);
251+
v8::HandleScope scope(isolate);
252+
v8::Local<v8::Context> context = v8::Context::New(isolate);
253+
const char* trigger_deopt = "f({y: 0, x: 1});";
254+
255+
// We use the isolate to pass arguments to the UnlockForDeoptimization
256+
// function. Namely, we pass a pointer to the deoptimizing thread.
257+
DeoptimizeCodeThread deoptimize_thread(isolate, context, trigger_deopt);
258+
isolate->SetData(0, &deoptimize_thread);
259+
bool ready_to_deopt = false;
260+
isolate->SetData(1, &ready_to_deopt);
261+
v8::Context::Scope context_scope(context);
262+
263+
// Create the function templace for C++ code that is invoked from
264+
// JavaScript code.
265+
Local<v8::FunctionTemplate> fun_templ =
266+
v8::FunctionTemplate::New(isolate, UnlockForDeoptimizationIfReady);
267+
Local<Function> fun = fun_templ->GetFunction(context).ToLocalChecked();
268+
CHECK(context->Global()
269+
->Set(context, v8_str("unlock_for_deoptimization"), fun)
270+
.FromJust());
271+
272+
// Optimizes a function f, which will be deoptimized by another thread.
273+
CompileRun(
274+
"function f(obj) { unlock_for_deoptimization(); return obj.x; }"
275+
"f({x: 1}); f({x: 1});"
276+
"%OptimizeFunctionOnNextCall(f);"
277+
"f({x: 1});");
278+
279+
// Trigger the unlocking.
280+
ready_to_deopt = true;
281+
isolate->SetData(1, &ready_to_deopt);
282+
Local<Value> v = CompileRun("f({x: 1});");
283+
284+
// Once the isolate has been unlocked, the thread will wait for the
285+
// other thread to finish its task. Once this happens, this thread
286+
// continues with its execution, that is, with the execution of the
287+
// function g, which then returns to f. The function f should have
288+
// also been deoptimized. Otherwise, the test will fail here.
289+
CHECK(v->IsNumber());
290+
CHECK_EQ(1, static_cast<int>(v->NumberValue(context).FromJust()));
291+
}
292+
isolate->Dispose();
293+
}
57294

58295
// Migrating an isolate
59296
class KangarooThread : public v8::base::Thread {

0 commit comments

Comments
 (0)