Commit fd564737 authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

[wasm] Abort compilation from background tasks

This removes another liability of the finisher: to abort compilation
and publish errors once an error state has been set by a background
compile unit.
This CL makes background threads set the error state directly and
schedule a foreground task to actually publish the error (e.g. via the
promise).

R=mstarzinger@chromium.org

Bug: v8:7921
Change-Id: I7a6a7ca4f235c2ad374b6ffc434eb6ac7d5f54ae
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/1307425Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57135}
parent af120db4
...@@ -94,7 +94,6 @@ class CompilationStateImpl { ...@@ -94,7 +94,6 @@ class CompilationStateImpl {
bool HasCompilationUnitToFinish(); bool HasCompilationUnitToFinish();
void PublishError();
void OnFinishedUnit(); void OnFinishedUnit();
void ScheduleUnitForFinishing(std::unique_ptr<WasmCompilationUnit> unit, void ScheduleUnitForFinishing(std::unique_ptr<WasmCompilationUnit> unit,
ExecutionTier mode); ExecutionTier mode);
...@@ -115,7 +114,7 @@ class CompilationStateImpl { ...@@ -115,7 +114,7 @@ class CompilationStateImpl {
bool failed() const { bool failed() const {
base::MutexGuard guard(&mutex_); base::MutexGuard guard(&mutex_);
return failed_; return compile_error_ != nullptr;
} }
bool baseline_compilation_finished() const { bool baseline_compilation_finished() const {
...@@ -182,7 +181,6 @@ class CompilationStateImpl { ...@@ -182,7 +181,6 @@ class CompilationStateImpl {
std::vector<std::unique_ptr<WasmCompilationUnit>> tiering_compilation_units_; std::vector<std::unique_ptr<WasmCompilationUnit>> tiering_compilation_units_;
bool finisher_is_running_ = false; bool finisher_is_running_ = false;
bool failed_ = false; // TODO(clemensh): Remove; derive from compile_error_.
size_t num_background_tasks_ = 0; size_t num_background_tasks_ = 0;
std::unique_ptr<CompilationError> compile_error_; std::unique_ptr<CompilationError> compile_error_;
...@@ -841,11 +839,8 @@ class FinishCompileTask : public CancelableTask { ...@@ -841,11 +839,8 @@ class FinishCompileTask : public CancelableTask {
break; break;
} }
if (unit->failed()) { DCHECK_IMPLIES(unit->failed(), compilation_state_->failed());
compilation_state_->PublishError(); if (unit->failed()) break;
compilation_state_->SetFinisherIsRunning(false);
break;
}
WasmCode* result = unit->result(); WasmCode* result = unit->result();
...@@ -3004,14 +2999,6 @@ bool CompilationStateImpl::HasCompilationUnitToFinish() { ...@@ -3004,14 +2999,6 @@ bool CompilationStateImpl::HasCompilationUnitToFinish() {
return !finish_units().empty(); return !finish_units().empty();
} }
void CompilationStateImpl::PublishError() {
DCHECK_NOT_NULL(compile_error_);
DCHECK(compile_error_->result.failed());
Abort();
VoidResult error_result = GetCompileError();
NotifyOnEvent(CompilationEvent::kFailedCompilation, &error_result);
}
void CompilationStateImpl::OnFinishedUnit() { void CompilationStateImpl::OnFinishedUnit() {
DCHECK_GT(outstanding_units_, 0); DCHECK_GT(outstanding_units_, 0);
--outstanding_units_; --outstanding_units_;
...@@ -3052,7 +3039,7 @@ void CompilationStateImpl::ScheduleUnitForFinishing( ...@@ -3052,7 +3039,7 @@ void CompilationStateImpl::ScheduleUnitForFinishing(
baseline_finish_units_.push_back(std::move(unit)); baseline_finish_units_.push_back(std::move(unit));
} }
if (!finisher_is_running_ && !failed_) { if (!finisher_is_running_ && !compile_error_) {
ScheduleFinisherTask(); ScheduleFinisherTask();
// We set the flag here so that not more than one finisher is started. // We set the flag here so that not more than one finisher is started.
finisher_is_running_ = true; finisher_is_running_ = true;
...@@ -3082,7 +3069,7 @@ void CompilationStateImpl::RestartBackgroundTasks(size_t max) { ...@@ -3082,7 +3069,7 @@ void CompilationStateImpl::RestartBackgroundTasks(size_t max) {
{ {
base::MutexGuard guard(&mutex_); base::MutexGuard guard(&mutex_);
// No need to restart tasks if compilation already failed. // No need to restart tasks if compilation already failed.
if (failed_) return; if (compile_error_) return;
DCHECK_LE(num_background_tasks_, max_background_tasks_); DCHECK_LE(num_background_tasks_, max_background_tasks_);
if (num_background_tasks_ == max_background_tasks_) return; if (num_background_tasks_ == max_background_tasks_) return;
...@@ -3122,7 +3109,10 @@ void CompilationStateImpl::ScheduleFinisherTask() { ...@@ -3122,7 +3109,10 @@ void CompilationStateImpl::ScheduleFinisherTask() {
void CompilationStateImpl::Abort() { void CompilationStateImpl::Abort() {
{ {
base::MutexGuard guard(&mutex_); base::MutexGuard guard(&mutex_);
failed_ = true; if (!compile_error_) {
compile_error_ = base::make_unique<CompilationError>(
0, VoidResult::Error(0, "Compilation aborted"));
}
} }
background_task_manager_.CancelAndWait(); background_task_manager_.CancelAndWait();
} }
...@@ -3135,6 +3125,16 @@ void CompilationStateImpl::SetError(uint32_t func_index, ...@@ -3135,6 +3125,16 @@ void CompilationStateImpl::SetError(uint32_t func_index,
if (compile_error_) return; if (compile_error_) return;
compile_error_ = compile_error_ =
base::make_unique<CompilationError>(func_index, error_result); base::make_unique<CompilationError>(func_index, error_result);
// Schedule a foreground task to call the callback and notify users about the
// compile error.
foreground_task_runner_->PostTask(
MakeCancelableLambdaTask(&foreground_task_manager_, [this] {
// This is only being called from foreground tasks.
base::MutexGuard guard(&mutex_);
HandleScope scope(isolate_);
VoidResult error_result = GetCompileError();
NotifyOnEvent(CompilationEvent::kFailedCompilation, &error_result);
}));
} }
void CompilationStateImpl::NotifyOnEvent(CompilationEvent event, void CompilationStateImpl::NotifyOnEvent(CompilationEvent event,
......
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