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

[wasm] Do not pass the error to callbacks

Instead of passing the error explicitly, make the callbacks get the
error from the CompilationState. This prepares a change to call the
callbacks asynchronously, because from the background we cannot
construct the final error message (because this requires access to the
wire bytes). Thus the callbacks will have to get the actual compile
error from the CompilationState from a foreground task if they need it.

R=mstarzinger@chromium.org

Bug: v8:8689
Change-Id: I22accabf895bf21fa7492e2f5cb8bac93237c765
Reviewed-on: https://chromium-review.googlesource.com/c/1445975
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59216}
parent be8c9e73
...@@ -98,7 +98,8 @@ enum class CompilationEvent : uint8_t { ...@@ -98,7 +98,8 @@ enum class CompilationEvent : uint8_t {
// This is the PIMPL interface to that private class. // This is the PIMPL interface to that private class.
class CompilationState { class CompilationState {
public: public:
using callback_t = std::function<void(CompilationEvent, const WasmError*)>; using callback_t = std::function<void(CompilationEvent)>;
~CompilationState(); ~CompilationState();
void CancelAndWait(); void CancelAndWait();
......
...@@ -216,7 +216,7 @@ class CompilationStateImpl { ...@@ -216,7 +216,7 @@ class CompilationStateImpl {
: func_index(func_index), error(std::move(error)) {} : func_index(func_index), error(std::move(error)) {}
}; };
void NotifyOnEvent(CompilationEvent event, const WasmError* error); void NotifyOnEvent(CompilationEvent event);
std::vector<std::unique_ptr<WasmCompilationUnit>>& finish_units() { std::vector<std::unique_ptr<WasmCompilationUnit>>& finish_units() {
return baseline_compilation_finished() ? tiering_finish_units_ return baseline_compilation_finished() ? tiering_finish_units_
...@@ -1081,7 +1081,7 @@ class AsyncCompileJob::CompilationStateCallback { ...@@ -1081,7 +1081,7 @@ class AsyncCompileJob::CompilationStateCallback {
public: public:
explicit CompilationStateCallback(AsyncCompileJob* job) : job_(job) {} explicit CompilationStateCallback(AsyncCompileJob* job) : job_(job) {}
void operator()(CompilationEvent event, const WasmError* error) { void operator()(CompilationEvent event) {
// This callback is only being called from a foreground task. // This callback is only being called from a foreground task.
switch (event) { switch (event) {
case CompilationEvent::kFinishedBaselineCompilation: case CompilationEvent::kFinishedBaselineCompilation:
...@@ -1099,14 +1099,12 @@ class AsyncCompileJob::CompilationStateCallback { ...@@ -1099,14 +1099,12 @@ class AsyncCompileJob::CompilationStateCallback {
break; break;
case CompilationEvent::kFailedCompilation: case CompilationEvent::kFailedCompilation:
DCHECK(!last_event_.has_value()); DCHECK(!last_event_.has_value());
DCHECK_NOT_NULL(error);
// Tier-up compilation should not fail if baseline compilation // Tier-up compilation should not fail if baseline compilation
// did not fail. // did not fail.
DCHECK(!Impl(job_->native_module_->compilation_state()) DCHECK(!Impl(job_->native_module_->compilation_state())
->baseline_compilation_finished()); ->baseline_compilation_finished());
// Note: The WasmError is copied here for use in the foreground task. job_->DoSync<CompileFailed, kUseExistingForegroundTask>();
job_->DoSync<CompileFailed, kUseExistingForegroundTask>(*error);
break; break;
default: default:
UNREACHABLE(); UNREACHABLE();
...@@ -1350,15 +1348,12 @@ class AsyncCompileJob::PrepareAndStartCompile : public CompileStep { ...@@ -1350,15 +1348,12 @@ class AsyncCompileJob::PrepareAndStartCompile : public CompileStep {
//========================================================================== //==========================================================================
class AsyncCompileJob::CompileFailed : public CompileStep { class AsyncCompileJob::CompileFailed : public CompileStep {
public: public:
explicit CompileFailed(WasmError error) : error_(std::move(error)) {}
void RunInForeground(AsyncCompileJob* job) override { void RunInForeground(AsyncCompileJob* job) override {
TRACE_COMPILE("(4b) Compilation Failed...\n"); TRACE_COMPILE("(4b) Compilation Failed...\n");
return job->AsyncCompileFailed("Async compilation failed", error_); WasmError error =
Impl(job->native_module_->compilation_state())->GetCompileError();
return job->AsyncCompileFailed("Async compilation failed", error);
} }
private:
WasmError error_;
}; };
void AsyncCompileJob::CompileWrappers() { void AsyncCompileJob::CompileWrappers() {
...@@ -1736,7 +1731,7 @@ void CompilationStateImpl::OnFinishedUnit(ExecutionTier tier, WasmCode* code) { ...@@ -1736,7 +1731,7 @@ void CompilationStateImpl::OnFinishedUnit(ExecutionTier tier, WasmCode* code) {
for (auto event : {CompilationEvent::kFinishedBaselineCompilation, for (auto event : {CompilationEvent::kFinishedBaselineCompilation,
CompilationEvent::kFinishedTopTierCompilation}) { CompilationEvent::kFinishedTopTierCompilation}) {
if (!events.contains(event)) continue; if (!events.contains(event)) continue;
NotifyOnEvent(event, nullptr); NotifyOnEvent(event);
} }
}; };
foreground_task_runner_->PostTask( foreground_task_runner_->PostTask(
...@@ -1845,17 +1840,14 @@ void CompilationStateImpl::SetError(uint32_t func_index, ...@@ -1845,17 +1840,14 @@ void CompilationStateImpl::SetError(uint32_t func_index,
compile_error.release(); compile_error.release();
// Schedule a foreground task to call the callback and notify users about the // Schedule a foreground task to call the callback and notify users about the
// compile error. // compile error.
foreground_task_runner_->PostTask( foreground_task_runner_->PostTask(MakeCancelableTask(
MakeCancelableTask(&foreground_task_manager_, [this] { &foreground_task_manager_,
WasmError error = GetCompileError(); [this] { NotifyOnEvent(CompilationEvent::kFailedCompilation); }));
NotifyOnEvent(CompilationEvent::kFailedCompilation, &error);
}));
} }
void CompilationStateImpl::NotifyOnEvent(CompilationEvent event, void CompilationStateImpl::NotifyOnEvent(CompilationEvent event) {
const WasmError* error) {
HandleScope scope(isolate_); HandleScope scope(isolate_);
for (auto& callback : callbacks_) callback(event, error); for (auto& callback : callbacks_) callback(event);
// If no more events are expected after this one, clear the callbacks to free // If no more events are expected after this one, clear the callbacks to free
// memory. We can safely do this here, as this method is only called from // memory. We can safely do this here, as this method is only called from
// foreground tasks. // foreground tasks.
......
...@@ -128,9 +128,8 @@ class TopTierCompiledCallback { ...@@ -128,9 +128,8 @@ class TopTierCompiledCallback {
: native_module_(std::move(native_module)), : native_module_(std::move(native_module)),
callback_(std::move(callback)) {} callback_(std::move(callback)) {}
void operator()(CompilationEvent event, const WasmError* error) const { void operator()(CompilationEvent event) const {
if (event != CompilationEvent::kFinishedTopTierCompilation) return; if (event != CompilationEvent::kFinishedTopTierCompilation) return;
DCHECK_NULL(error);
// If the native module is still alive, get back a shared ptr and call the // If the native module is still alive, get back a shared ptr and call the
// callback. // callback.
if (std::shared_ptr<NativeModule> native_module = native_module_.lock()) { if (std::shared_ptr<NativeModule> native_module = native_module_.lock()) {
......
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