Commit 269ea5ae authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

[wasm] Remove AsyncCompileJob even if a foreground task is pending

We have complicated logic in place to ensure that an {AsyncCompileJob}
is not removed as long as a foreground task is still pending.
This CL changes that to just cancel the pending foreground task and
remove the {AsyncCompileJob} immediately.

R=mstarzinger@chromium.org

Bug: chromium:869420
Change-Id: Ia064dae4a0e31416675e5d77c46879254fc817c4
Reviewed-on: https://chromium-review.googlesource.com/1158578Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54846}
parent 2f696693
......@@ -2123,15 +2123,8 @@ void AsyncCompileJob::Start() {
void AsyncCompileJob::Abort() {
background_task_manager_.CancelAndWait();
if (native_module_) native_module_->compilation_state()->Abort();
if (num_pending_foreground_tasks_ == 0) {
// No task is pending, we can just remove the AsyncCompileJob.
isolate_->wasm_engine()->RemoveCompileJob(this);
} else {
// There is still a compilation task in the task queue. We enter the
// AbortCompilation state and wait for this compilation task to abort the
// AsyncCompileJob.
NextStep<AbortCompilation>();
}
CancelPendingForegroundTask();
isolate_->wasm_engine()->RemoveCompileJob(this);
}
class AsyncStreamingProcessor final : public StreamingProcessor {
......@@ -2217,9 +2210,9 @@ class AsyncCompileJob::CompileStep {
void Run(bool on_foreground) {
if (on_foreground) {
DCHECK_NOT_NULL(job_->pending_foreground_task_);
job_->pending_foreground_task_ = nullptr;
HandleScope scope(job_->isolate_);
--job_->num_pending_foreground_tasks_;
DCHECK_EQ(0, job_->num_pending_foreground_tasks_);
SaveContext saved_context(job_->isolate_);
job_->isolate_->set_context(*job_->native_context_);
RunInForeground();
......@@ -2249,18 +2242,41 @@ class AsyncCompileJob::CompileTask : public CancelableTask {
job_(job),
on_foreground_(on_foreground) {}
void RunInternal() override { job_->step_->Run(on_foreground_); }
void RunInternal() final {
if (job_) job_->step_->Run(on_foreground_);
}
void Cancel() {
DCHECK_NOT_NULL(job_);
job_ = nullptr;
}
private:
// {job_} will be cleared to cancel a pending task.
AsyncCompileJob* job_;
bool on_foreground_;
};
void AsyncCompileJob::StartForegroundTask() {
++num_pending_foreground_tasks_;
DCHECK_EQ(1, num_pending_foreground_tasks_);
DCHECK_NULL(pending_foreground_task_);
auto new_task = base::make_unique<CompileTask>(this, true);
pending_foreground_task_ = new_task.get();
foreground_task_runner_->PostTask(std::move(new_task));
}
foreground_task_runner_->PostTask(base::make_unique<CompileTask>(this, true));
void AsyncCompileJob::ExecuteForegroundTaskImmediately() {
DCHECK_NULL(pending_foreground_task_);
auto new_task = base::make_unique<CompileTask>(this, true);
pending_foreground_task_ = new_task.get();
new_task->Run();
}
void AsyncCompileJob::CancelPendingForegroundTask() {
if (!pending_foreground_task_) return;
pending_foreground_task_->Cancel();
pending_foreground_task_ = nullptr;
}
template <typename Step, typename... Args>
......@@ -2421,17 +2437,14 @@ class AsyncCompileJob::PrepareAndStartCompile : public CompileStep {
}
return;
case CompilationEvent::kFinishedTopTierCompilation:
// It is only safe to remove the AsyncCompileJob if no
// foreground task is currently pending, and no finisher is
// outstanding (streaming compilation).
if (job->num_pending_foreground_tasks_ == 0 &&
job->outstanding_finishers_.load() == 0) {
job->isolate_->wasm_engine()->RemoveCompileJob(job);
} else {
// If a foreground task was pending or a finsher was pending,
// we will rely on FinishModule to remove the job.
// If a foreground task or a finisher is pending, we rely on
// FinishModule to remove the job.
if (job->pending_foreground_task_ ||
job->outstanding_finishers_.load() > 0) {
job->tiering_completed_ = true;
return;
}
job->isolate_->wasm_engine()->RemoveCompileJob(job);
return;
case CompilationEvent::kFailedCompilation: {
// Tier-up compilation should not fail if baseline compilation
......@@ -2532,13 +2545,6 @@ class AsyncCompileJob::FinishModule : public CompileStep {
}
};
class AsyncCompileJob::AbortCompilation : public CompileStep {
void RunInForeground() override {
TRACE_COMPILE("Abort asynchronous compilation ...\n");
job_->isolate_->wasm_engine()->RemoveCompileJob(job_);
}
};
AsyncStreamingProcessor::AsyncStreamingProcessor(AsyncCompileJob* job)
: job_(job), compilation_unit_builder_(nullptr) {}
......@@ -2557,7 +2563,7 @@ void AsyncStreamingProcessor::FinishAsyncCompileJobWithError(ResultBase error) {
if (job_->native_module_) {
job_->native_module_->compilation_state()->Abort();
if (job_->num_pending_foreground_tasks_ == 0) {
if (job_->pending_foreground_task_ == nullptr) {
job_->DoSync<AsyncCompileJob::DecodeFail>(std::move(result));
} else {
job_->NextStep<AsyncCompileJob::DecodeFail>(std::move(result));
......@@ -2631,12 +2637,8 @@ bool AsyncStreamingProcessor::ProcessCodeSectionHeader(size_t functions_count,
}
job_->NextStep<AsyncCompileJob::PrepareAndStartCompile>(false);
// Execute the PrepareAndStartCompile step immediately and not in a separate
// task. The step expects to be run on a separate foreground thread though, so
// we to increment {num_pending_foreground_tasks_} to look like one.
++job_->num_pending_foreground_tasks_;
DCHECK_EQ(1, job_->num_pending_foreground_tasks_);
constexpr bool on_foreground = true;
job_->step_->Run(on_foreground);
// task.
job_->ExecuteForegroundTaskImmediately();
job_->native_module_->compilation_state()->SetNumberOfFunctionsToCompile(
functions_count);
......
......@@ -87,6 +87,7 @@ class AsyncCompileJob {
std::shared_ptr<StreamingDecoder> CreateStreamingDecoder();
void Abort();
void CancelPendingForegroundTask();
Isolate* isolate() const { return isolate_; }
......@@ -101,7 +102,6 @@ class AsyncCompileJob {
class CompileFailed;
class CompileWrappers;
class FinishModule;
class AbortCompilation;
class UpdateToTopTierCompiledCode;
const std::shared_ptr<Counters>& async_counters() const {
......@@ -116,6 +116,7 @@ class AsyncCompileJob {
void AsyncCompileSucceeded(Handle<WasmModuleObject> result);
void StartForegroundTask();
void ExecuteForegroundTaskImmediately();
void StartBackgroundTask();
......@@ -169,8 +170,8 @@ class AsyncCompileJob {
return outstanding_finishers_.fetch_sub(1) == 1;
}
// Counts the number of pending foreground tasks.
int32_t num_pending_foreground_tasks_ = 0;
// A reference to a pending foreground task, or {nullptr} if none is pending.
CompileTask* pending_foreground_task_ = nullptr;
// The AsyncCompileJob owns the StreamingDecoder because the StreamingDecoder
// contains data which is needed by the AsyncCompileJob for streaming
......
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