diff --git a/src/api.cc b/src/api.cc index c311731ccd..634ad37115 100644 --- a/src/api.cc +++ b/src/api.cc @@ -8629,7 +8629,7 @@ int Isolate::ContextDisposedNotification(bool dependant_context) { if (!dependant_context) { // We left the current context, we can abort all WebAssembly compilations on // that isolate. - isolate->wasm_engine()->AbortCompileJobsOnIsolate(isolate); + isolate->wasm_engine()->DeleteCompileJobsOnIsolate(isolate); } // TODO(ahaas): move other non-heap activity out of the heap call. return isolate->heap()->NotifyContextDisposed(dependant_context); diff --git a/src/isolate.cc b/src/isolate.cc index e3cc8de9e7..d472b4f97f 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -2839,7 +2839,7 @@ void Isolate::Deinit() { debug()->Unload(); - wasm_engine()->AbortCompileJobsOnIsolate(this); + wasm_engine()->DeleteCompileJobsOnIsolate(this); if (concurrent_recompilation_enabled()) { optimizing_compile_dispatcher_->Stop(); diff --git a/src/wasm/compilation-environment.h b/src/wasm/compilation-environment.h index c5f1e85976..82db9da5e3 100644 --- a/src/wasm/compilation-environment.h +++ b/src/wasm/compilation-environment.h @@ -93,7 +93,7 @@ class CompilationState { using callback_t = std::function; ~CompilationState(); - void AbortCompilation(); + void CancelAndWait(); void SetError(uint32_t func_index, const ResultBase& error_result); diff --git a/src/wasm/module-compiler.cc b/src/wasm/module-compiler.cc index 5ba2b717b2..f54c837b07 100644 --- a/src/wasm/module-compiler.cc +++ b/src/wasm/module-compiler.cc @@ -67,7 +67,7 @@ class CompilationStateImpl { // Cancel all background compilation and wait for all tasks to finish. Call // this before destructing this object. - void AbortCompilation(); + void CancelAndWait(); // Set the number of compilations unit expected to be executed. Needs to be // set before {AddCompilationUnits} is run, which triggers background @@ -83,6 +83,9 @@ class CompilationStateImpl { std::vector>& baseline_units, std::vector>& tiering_units); std::unique_ptr GetNextCompilationUnit(); + std::unique_ptr GetNextExecutedUnit(); + + bool HasCompilationUnitToFinish(); void OnFinishedUnit(ExecutionTier); void ScheduleCodeLogging(WasmCode*); @@ -90,6 +93,10 @@ class CompilationStateImpl { void OnBackgroundTaskStopped(const WasmFeatures& detected); void PublishDetectedFeatures(Isolate* isolate, const WasmFeatures& detected); void RestartBackgroundTasks(size_t max = std::numeric_limits::max()); + // Only one foreground thread (finisher) is allowed to run at a time. + // {SetFinisherIsRunning} returns whether the flag changed its state. + bool SetFinisherIsRunning(bool value); + void ScheduleFinisherTask(); void Abort(); @@ -207,6 +214,11 @@ class CompilationStateImpl { void NotifyOnEvent(CompilationEvent event, const VoidResult* error_result); + std::vector>& finish_units() { + return baseline_compilation_finished() ? tiering_finish_units_ + : baseline_finish_units_; + } + // TODO(mstarzinger): Get rid of the Isolate field to make sure the // {CompilationStateImpl} can be shared across multiple Isolates. Isolate* const isolate_; @@ -233,8 +245,12 @@ class CompilationStateImpl { std::vector> baseline_compilation_units_; std::vector> tiering_compilation_units_; + bool finisher_is_running_ = false; size_t num_background_tasks_ = 0; + std::vector> baseline_finish_units_; + std::vector> tiering_finish_units_; + // Features detected to be used in this module. Features can be detected // as a module is being compiled. WasmFeatures detected_features_ = kNoWasmFeatures; @@ -473,7 +489,7 @@ const CompilationStateImpl* Impl(const CompilationState* compilation_state) { CompilationState::~CompilationState() { Impl(this)->~CompilationStateImpl(); } -void CompilationState::AbortCompilation() { Impl(this)->AbortCompilation(); } +void CompilationState::CancelAndWait() { Impl(this)->CancelAndWait(); } void CompilationState::SetError(uint32_t func_index, const ResultBase& error_result) { @@ -660,8 +676,15 @@ bool in_bounds(uint32_t offset, size_t size, size_t upper) { using WasmInstanceMap = IdentityMap, FreeStoreAllocationPolicy>; +double MonotonicallyIncreasingTimeInMs() { + return V8::GetCurrentPlatform()->MonotonicallyIncreasingTime() * + base::Time::kMillisecondsPerSecond; +} + // Run by each compilation task and by the main thread (i.e. in both -// foreground and background threads). +// foreground and background threads). The no_finisher_callback is called +// within the result_mutex_ lock when no finishing task is running, i.e. when +// the finisher_is_running_ flag is not set. bool FetchAndExecuteCompilationUnit(CompilationEnv* env, CompilationStateImpl* compilation_state, WasmFeatures* detected, @@ -698,6 +721,15 @@ void InitializeCompilationUnits(NativeModule* native_module, builder.Commit(); } +void FinishCompilationUnits(CompilationStateImpl* compilation_state) { + TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm"), "FinishCompilationUnits"); + while (!compilation_state->failed()) { + std::unique_ptr unit = + compilation_state->GetNextExecutedUnit(); + if (unit == nullptr) break; + } +} + void CompileInParallel(Isolate* isolate, NativeModule* native_module) { // Data structures for the parallel compilation. @@ -710,6 +742,12 @@ void CompileInParallel(Isolate* isolate, NativeModule* native_module) { // the background threads. // 2) The background threads and the main thread pick one compilation unit at // a time and execute the parallel phase of the compilation unit. + // 3) After the parallel phase of all compilation units has started, the + // main thread continues to finish all compilation units as long as + // baseline-compilation units are left to be processed. + // 4) If tier-up is enabled, the main thread restarts background tasks + // that take care of compiling and finishing the top-tier compilation + // units. // Turn on the {CanonicalHandleScope} so that the background threads can // use the node cache. @@ -717,6 +755,10 @@ void CompileInParallel(Isolate* isolate, NativeModule* native_module) { CompilationStateImpl* compilation_state = Impl(native_module->compilation_state()); + // Make sure that no foreground task is spawned for finishing + // the compilation units. This foreground thread will be + // responsible for finishing compilation. + compilation_state->SetFinisherIsRunning(true); uint32_t num_wasm_functions = native_module->num_functions() - native_module->num_imported_functions(); compilation_state->SetNumberOfFunctionsToCompile(num_wasm_functions); @@ -732,19 +774,38 @@ void CompileInParallel(Isolate* isolate, NativeModule* native_module) { // a time and execute the parallel phase of the compilation unit. WasmFeatures detected_features; CompilationEnv env = native_module->CreateCompilationEnv(); - // TODO(wasm): This might already execute TurboFan units on the main thread, - // while waiting for baseline compilation to finish. This can introduce - // additional delay. - // TODO(wasm): This is a busy-wait loop once all units have started executing - // in background threads. Replace by a semaphore / barrier. - while (!compilation_state->failed() && + while (FetchAndExecuteCompilationUnit(&env, compilation_state, + &detected_features, + isolate->counters()) && !compilation_state->baseline_compilation_finished()) { - FetchAndExecuteCompilationUnit(&env, compilation_state, &detected_features, - isolate->counters()); + // TODO(clemensh): Refactor ownership of the AsyncCompileJob and remove + // this. + FinishCompilationUnits(compilation_state); + + if (compilation_state->failed()) break; + } + + while (!compilation_state->failed()) { + // 3) After the parallel phase of all compilation units has started, the + // main thread continues to finish compilation units as long as + // baseline compilation units are left to be processed. If compilation + // already failed, all background tasks have already been canceled + // in {FinishCompilationUnits}, and there are no units to finish. + FinishCompilationUnits(compilation_state); + + if (compilation_state->baseline_compilation_finished()) break; } // Publish features from the foreground and background tasks. compilation_state->PublishDetectedFeatures(isolate, detected_features); + + // 4) If tiering-compilation is enabled, we need to set the finisher + // to false, such that the background threads will spawn a foreground + // thread to finish the top-tier compilation units. + if (!compilation_state->failed() && + compilation_state->compile_mode() == CompileMode::kTiering) { + compilation_state->SetFinisherIsRunning(false); + } } void CompileSequentially(Isolate* isolate, NativeModule* native_module, @@ -842,6 +903,62 @@ void CompileNativeModule(Isolate* isolate, ErrorThrower* thrower, } } +// The runnable task that finishes compilation in foreground (e.g. updating +// the NativeModule, the code table, etc.). +class FinishCompileTask : public CancelableTask { + public: + explicit FinishCompileTask(CompilationStateImpl* compilation_state, + CancelableTaskManager* task_manager) + : CancelableTask(task_manager), compilation_state_(compilation_state) {} + + void RunInternal() override { + Isolate* isolate = compilation_state_->isolate(); + HandleScope scope(isolate); + SaveContext saved_context(isolate); + isolate->set_context(Context()); + + TRACE_COMPILE("(4a) Finishing compilation units...\n"); + if (compilation_state_->failed()) { + compilation_state_->SetFinisherIsRunning(false); + return; + } + + // We execute for 1 ms and then reschedule the task, same as the GC. + double deadline = MonotonicallyIncreasingTimeInMs() + 1.0; + while (true) { + compilation_state_->RestartBackgroundTasks(); + + std::unique_ptr unit = + compilation_state_->GetNextExecutedUnit(); + + if (unit == nullptr) { + // It might happen that a background task just scheduled a unit to be + // finished, but did not start a finisher task since the flag was still + // set. Check for this case, and continue if there is more work. + compilation_state_->SetFinisherIsRunning(false); + if (compilation_state_->HasCompilationUnitToFinish() && + compilation_state_->SetFinisherIsRunning(true)) { + continue; + } + break; + } + + if (compilation_state_->failed()) break; + + if (deadline < MonotonicallyIncreasingTimeInMs()) { + // We reached the deadline. We reschedule this task and return + // immediately. Since we rescheduled this task already, we do not set + // the FinisherIsRunning flag to false. + compilation_state_->ScheduleFinisherTask(); + return; + } + } + } + + private: + CompilationStateImpl* compilation_state_; +}; + // The runnable task that performs compilations in the background. class BackgroundCompileTask : public CancelableTask { public: @@ -2296,18 +2413,14 @@ void AsyncCompileJob::Start() { } void AsyncCompileJob::Abort() { - background_task_manager_.CancelAndWait(); - if (native_module_) Impl(native_module_->compilation_state())->Abort(); - // Tell the streaming decoder that compilation aborted. - // TODO(ahaas): Is this notification really necessary? Check - // https://crbug.com/888170. - if (stream_) stream_->NotifyCompilationEnded(); - CancelPendingForegroundTask(); + // Removing this job will trigger the destructor, which will cancel all + // compilation. + isolate_->wasm_engine()->RemoveCompileJob(this); } class AsyncStreamingProcessor final : public StreamingProcessor { public: - explicit AsyncStreamingProcessor(std::shared_ptr job); + explicit AsyncStreamingProcessor(AsyncCompileJob* job); bool ProcessModuleHeader(Vector bytes, uint32_t offset) override; @@ -2339,22 +2452,28 @@ class AsyncStreamingProcessor final : public StreamingProcessor { void CommitCompilationUnits(); ModuleDecoder decoder_; - std::shared_ptr job_; + AsyncCompileJob* job_; std::unique_ptr compilation_unit_builder_; uint32_t next_function_ = 0; }; std::shared_ptr AsyncCompileJob::CreateStreamingDecoder() { DCHECK_NULL(stream_); - stream_ = std::make_shared( - base::make_unique(shared_from_this())); + stream_.reset( + new StreamingDecoder(base::make_unique(this))); return stream_; } AsyncCompileJob::~AsyncCompileJob() { background_task_manager_.CancelAndWait(); + if (native_module_) Impl(native_module_->compilation_state())->Abort(); + // Tell the streaming decoder that the AsyncCompileJob is not available + // anymore. + // TODO(ahaas): Is this notification really necessary? Check + // https://crbug.com/888170. + if (stream_) stream_->NotifyCompilationEnded(); + CancelPendingForegroundTask(); for (auto d : deferred_handles_) delete d; - isolate_->wasm_engine()->RemoveCompileJob(this); } void AsyncCompileJob::PrepareRuntimeObjects( @@ -2424,19 +2543,19 @@ void AsyncCompileJob::FinishCompile(bool compile_wrappers) { } void AsyncCompileJob::AsyncCompileFailed(Handle error_reason) { - if (stream_) stream_->NotifyCompilationEnded(); + // {job} keeps the {this} pointer alive. + std::shared_ptr job = + isolate_->wasm_engine()->RemoveCompileJob(this); resolver_->OnCompilationFailed(error_reason); } void AsyncCompileJob::AsyncCompileSucceeded(Handle result) { - if (stream_) stream_->NotifyCompilationEnded(); resolver_->OnCompilationSucceeded(result); } class AsyncCompileJob::CompilationStateCallback { public: - explicit CompilationStateCallback(std::shared_ptr job) - : job_(std::move(job)) {} + explicit CompilationStateCallback(AsyncCompileJob* job) : job_(job) {} void operator()(CompilationEvent event, const ResultBase* error_result) { // This callback is only being called from a foreground task. @@ -2451,10 +2570,21 @@ class AsyncCompileJob::CompilationStateCallback { break; case CompilationEvent::kFinishedTopTierCompilation: DCHECK_EQ(CompilationEvent::kFinishedBaselineCompilation, last_event_); + // 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_->isolate_->wasm_engine()->RemoveCompileJob(job_); + } break; case CompilationEvent::kFailedCompilation: DCHECK(!last_event_.has_value()); DCHECK_NOT_NULL(error_result); + // Tier-up compilation should not fail if baseline compilation + // did not fail. + DCHECK(!Impl(job_->native_module_->compilation_state()) + ->baseline_compilation_finished()); + { SaveContext saved_context(job_->isolate()); job_->isolate()->set_context(*job_->native_context_); @@ -2462,7 +2592,11 @@ class AsyncCompileJob::CompilationStateCallback { thrower.CompileFailed(*error_result); Handle error = thrower.Reify(); - job_->AsyncCompileFailed(error); + DeferredHandleScope deferred(job_->isolate()); + error = handle(*error, job_->isolate()); + job_->deferred_handles_.push_back(deferred.Detach()); + + job_->DoSync(error); } break; @@ -2475,7 +2609,7 @@ class AsyncCompileJob::CompilationStateCallback { } private: - std::shared_ptr job_; + AsyncCompileJob* job_; #ifdef DEBUG base::Optional last_event_; #endif @@ -2487,7 +2621,7 @@ class AsyncCompileJob::CompileStep { public: virtual ~CompileStep() = default; - void Run(const std::shared_ptr& job, bool on_foreground) { + void Run(AsyncCompileJob* job, bool on_foreground) { if (on_foreground) { HandleScope scope(job->isolate_); SaveContext saved_context(job->isolate_); @@ -2498,12 +2632,8 @@ class AsyncCompileJob::CompileStep { } } - virtual void RunInForeground(const std::shared_ptr&) { - UNREACHABLE(); - } - virtual void RunInBackground(const std::shared_ptr&) { - UNREACHABLE(); - } + virtual void RunInForeground(AsyncCompileJob*) { UNREACHABLE(); } + virtual void RunInBackground(AsyncCompileJob*) { UNREACHABLE(); } }; class AsyncCompileJob::CompileTask : public CancelableTask { @@ -2515,7 +2645,7 @@ class AsyncCompileJob::CompileTask : public CancelableTask { // their own task manager. : CancelableTask(on_foreground ? job->isolate_->cancelable_task_manager() : &job->background_task_manager_), - job_(job->shared_from_this()), + job_(job), on_foreground_(on_foreground) {} ~CompileTask() override { @@ -2537,10 +2667,9 @@ class AsyncCompileJob::CompileTask : public CancelableTask { } private: - // Compile tasks (foreground and background) keep the {AsyncCompileJob} alive - // via shared_ptr. It will be cleared to cancel a pending task. - std::shared_ptr job_; - const bool on_foreground_; + // {job_} will be cleared to cancel a pending task. + AsyncCompileJob* job_; + bool on_foreground_; void ResetPendingForegroundTask() const { DCHECK_EQ(this, job_->pending_foreground_task_); @@ -2615,7 +2744,7 @@ class AsyncCompileJob::DecodeModule : public AsyncCompileJob::CompileStep { public: explicit DecodeModule(Counters* counters) : counters_(counters) {} - void RunInBackground(const std::shared_ptr& job) override { + void RunInBackground(AsyncCompileJob* job) override { ModuleResult result; { DisallowHandleAllocation no_handle; @@ -2651,10 +2780,11 @@ class AsyncCompileJob::DecodeFail : public CompileStep { private: ModuleResult result_; - void RunInForeground(const std::shared_ptr& job) override { + void RunInForeground(AsyncCompileJob* job) override { TRACE_COMPILE("(1b) Decoding failed.\n"); ErrorThrower thrower(job->isolate_, "AsyncCompile"); thrower.CompileFailed("Wasm decoding failed", result_); + // {job_} is deleted in AsyncCompileFailed, therefore the {return}. return job->AsyncCompileFailed(thrower.Reify()); } }; @@ -2672,7 +2802,7 @@ class AsyncCompileJob::PrepareAndStartCompile : public CompileStep { std::shared_ptr module_; bool start_compilation_; - void RunInForeground(const std::shared_ptr& job) override { + void RunInForeground(AsyncCompileJob* job) override { TRACE_COMPILE("(2) Prepare and start compile...\n"); // Make sure all compilation tasks stopped running. Decoding (async step) @@ -2708,13 +2838,30 @@ class AsyncCompileJob::PrepareAndStartCompile : public CompileStep { } }; +//========================================================================== +// Step 4b (sync): Compilation failed. Reject Promise. +//========================================================================== +class AsyncCompileJob::CompileFailed : public CompileStep { + public: + explicit CompileFailed(Handle error_reason) + : error_reason_(error_reason) {} + + void RunInForeground(AsyncCompileJob* job) override { + TRACE_COMPILE("(4b) Compilation Failed...\n"); + return job->AsyncCompileFailed(error_reason_); + } + + private: + Handle error_reason_; +}; + //========================================================================== // Step 5 (sync): Compile JS->wasm wrappers. //========================================================================== class AsyncCompileJob::CompileWrappers : public CompileStep { // TODO(wasm): Compile all wrappers here, including the start function wrapper // and the wrappers for the function table elements. - void RunInForeground(const std::shared_ptr& job) override { + void RunInForeground(AsyncCompileJob* job) override { TRACE_COMPILE("(5) Compile wrappers...\n"); // Compile JS->wasm wrappers for exported functions. CompileJsToWasmWrappers( @@ -2728,20 +2875,33 @@ class AsyncCompileJob::CompileWrappers : public CompileStep { // Step 6 (sync): Finish the module and resolve the promise. //========================================================================== class AsyncCompileJob::FinishModule : public CompileStep { - void RunInForeground(const std::shared_ptr& job) override { + void RunInForeground(AsyncCompileJob* job) override { TRACE_COMPILE("(6) Finish module...\n"); job->AsyncCompileSucceeded(job->module_object_); + + size_t num_functions = job->native_module_->num_functions() - + job->native_module_->num_imported_functions(); + auto* compilation_state = Impl(job->native_module_->compilation_state()); + if (compilation_state->compile_mode() == CompileMode::kRegular || + num_functions == 0) { + // If we do not tier up, the async compile job is done here and + // can be deleted. + job->isolate_->wasm_engine()->RemoveCompileJob(job); + return; + } + DCHECK_EQ(CompileMode::kTiering, compilation_state->compile_mode()); + if (compilation_state->baseline_compilation_finished()) { + job->isolate_->wasm_engine()->RemoveCompileJob(job); + } } }; -AsyncStreamingProcessor::AsyncStreamingProcessor( - std::shared_ptr job) +AsyncStreamingProcessor::AsyncStreamingProcessor(AsyncCompileJob* job) : decoder_(job->enabled_features_), - job_(std::move(job)), + job_(job), compilation_unit_builder_(nullptr) {} void AsyncStreamingProcessor::FinishAsyncCompileJobWithError(ResultBase error) { - TRACE_STREAMING("Finish streaming with error.\n"); DCHECK(error.failed()); // Make sure all background tasks stopped executing before we change the state // of the AsyncCompileJob to DecodeFail. @@ -2954,14 +3114,13 @@ CompilationStateImpl::CompilationStateImpl(internal::Isolate* isolate, } CompilationStateImpl::~CompilationStateImpl() { - // {AbortCompilation} must have been called. DCHECK(background_task_manager_.canceled()); DCHECK(foreground_task_manager_.canceled()); CompilationError* error = compile_error_.load(std::memory_order_acquire); if (error != nullptr) delete error; } -void CompilationStateImpl::AbortCompilation() { +void CompilationStateImpl::CancelAndWait() { background_task_manager_.CancelAndWait(); foreground_task_manager_.CancelAndWait(); } @@ -3023,10 +3182,26 @@ CompilationStateImpl::GetNextCompilationUnit() { return std::unique_ptr(); } +std::unique_ptr +CompilationStateImpl::GetNextExecutedUnit() { + std::vector>& units = finish_units(); + base::MutexGuard guard(&mutex_); + if (units.empty()) return {}; + std::unique_ptr ret = std::move(units.back()); + units.pop_back(); + return ret; +} + +bool CompilationStateImpl::HasCompilationUnitToFinish() { + return !finish_units().empty(); +} + void CompilationStateImpl::OnFinishedUnit(ExecutionTier tier) { // This mutex guarantees that events happen in the right order. base::MutexGuard guard(&mutex_); + if (failed()) return; + // If we are *not* compiling in tiering mode, then all units are counted as // baseline units. bool is_tiering_mode = compile_mode_ == CompileMode::kTiering; @@ -3136,8 +3311,19 @@ void CompilationStateImpl::RestartBackgroundTasks(size_t max) { } } +bool CompilationStateImpl::SetFinisherIsRunning(bool value) { + base::MutexGuard guard(&mutex_); + if (finisher_is_running_ == value) return false; + finisher_is_running_ = value; + return true; +} + +void CompilationStateImpl::ScheduleFinisherTask() { + foreground_task_runner_->PostTask( + base::make_unique(this, &foreground_task_manager_)); +} + void CompilationStateImpl::Abort() { - printf("Abort: %p\n", this); SetError(0, VoidResult::Error(0, "Compilation aborted")); background_task_manager_.CancelAndWait(); // No more callbacks after abort. Don't free the std::function objects here, diff --git a/src/wasm/module-compiler.h b/src/wasm/module-compiler.h index d20d0d969a..6101898157 100644 --- a/src/wasm/module-compiler.h +++ b/src/wasm/module-compiler.h @@ -65,11 +65,8 @@ Address CompileLazy(Isolate*, NativeModule*, uint32_t func_index); // allocates on the V8 heap (e.g. creating the module object) must be a // foreground task. All other tasks (e.g. decoding and validating, the majority // of the work of compilation) can be background tasks. -// AsyncCompileJobs are stored in shared_ptr by all tasks which need to keep -// them alive. {std::enable_shared_from_this} allows to regain a shared_ptr from -// a raw ptr. // TODO(wasm): factor out common parts of this with the synchronous pipeline. -class AsyncCompileJob : public std::enable_shared_from_this { +class AsyncCompileJob { public: AsyncCompileJob(Isolate* isolate, const WasmFeatures& enabled_features, std::unique_ptr bytes_copy, size_t length, @@ -95,6 +92,7 @@ class AsyncCompileJob : public std::enable_shared_from_this { class DecodeModule; // Step 1 (async) class DecodeFail; // Step 1b (sync) class PrepareAndStartCompile; // Step 2 (sync) + class CompileFailed; // Step 4b (sync) class CompileWrappers; // Step 5 (sync) class FinishModule; // Step 6 (sync) diff --git a/src/wasm/streaming-decoder.cc b/src/wasm/streaming-decoder.cc index 71fe118e62..7910cb31cf 100644 --- a/src/wasm/streaming-decoder.cc +++ b/src/wasm/streaming-decoder.cc @@ -103,10 +103,8 @@ void StreamingDecoder::Finish() { void StreamingDecoder::Abort() { TRACE_STREAMING("Abort\n"); if (!ok()) return; // Failed already. - // Move the processor out of the unique_ptr field first, to avoid recursive - // calls to {OnAbort}. - std::unique_ptr processor = std::move(processor_); - processor->OnAbort(); + processor_->OnAbort(); + Fail(); } void StreamingDecoder::SetModuleCompiledCallback( diff --git a/src/wasm/wasm-code-manager.cc b/src/wasm/wasm-code-manager.cc index e8b632c90a..af1c7b229e 100644 --- a/src/wasm/wasm-code-manager.cc +++ b/src/wasm/wasm-code-manager.cc @@ -869,7 +869,7 @@ NativeModule::~NativeModule() { TRACE_HEAP("Deleting native module: %p\n", reinterpret_cast(this)); // Cancel all background compilation before resetting any field of the // NativeModule or freeing anything. - compilation_state_->AbortCompilation(); + compilation_state_->CancelAndWait(); code_manager_->FreeNativeModule(this); } diff --git a/src/wasm/wasm-engine.cc b/src/wasm/wasm-engine.cc index bd9af9d699..c6e57cd241 100644 --- a/src/wasm/wasm-engine.cc +++ b/src/wasm/wasm-engine.cc @@ -24,7 +24,7 @@ WasmEngine::WasmEngine() WasmEngine::~WasmEngine() { // All AsyncCompileJobs have been canceled. - DCHECK(async_compile_jobs_.empty()); + DCHECK(jobs_.empty()); // All Isolates have been deregistered. DCHECK(isolates_.empty()); } @@ -212,7 +212,7 @@ void WasmEngine::AsyncCompile( std::unique_ptr copy(new byte[bytes.length()]); memcpy(copy.get(), bytes.start(), bytes.length()); - std::shared_ptr job = CreateAsyncCompileJob( + AsyncCompileJob* job = CreateAsyncCompileJob( isolate, enabled, std::move(copy), bytes.length(), handle(isolate->context(), isolate), std::move(resolver)); job->Start(); @@ -221,7 +221,7 @@ void WasmEngine::AsyncCompile( std::shared_ptr WasmEngine::StartStreamingCompilation( Isolate* isolate, const WasmFeatures& enabled, Handle context, std::shared_ptr resolver) { - std::shared_ptr job = + AsyncCompileJob* job = CreateAsyncCompileJob(isolate, enabled, std::unique_ptr(nullptr), 0, context, std::move(resolver)); return job->CreateStreamingDecoder(); @@ -278,49 +278,48 @@ CodeTracer* WasmEngine::GetCodeTracer() { return code_tracer_.get(); } -std::shared_ptr WasmEngine::CreateAsyncCompileJob( +AsyncCompileJob* WasmEngine::CreateAsyncCompileJob( Isolate* isolate, const WasmFeatures& enabled, std::unique_ptr bytes_copy, size_t length, Handle context, std::shared_ptr resolver) { - std::shared_ptr job = - std::make_shared(isolate, enabled, std::move(bytes_copy), - length, context, std::move(resolver)); + AsyncCompileJob* job = + new AsyncCompileJob(isolate, enabled, std::move(bytes_copy), length, + context, std::move(resolver)); + // Pass ownership to the unique_ptr in {jobs_}. base::MutexGuard guard(&mutex_); - async_compile_jobs_.insert({job.get(), job}); + jobs_[job] = std::unique_ptr(job); return job; } -void WasmEngine::RemoveCompileJob(AsyncCompileJob* job) { +std::unique_ptr WasmEngine::RemoveCompileJob( + AsyncCompileJob* job) { base::MutexGuard guard(&mutex_); - auto item = async_compile_jobs_.find(job); - DCHECK(item != async_compile_jobs_.end()); - async_compile_jobs_.erase(item); + auto item = jobs_.find(job); + DCHECK(item != jobs_.end()); + std::unique_ptr result = std::move(item->second); + jobs_.erase(item); + return result; } bool WasmEngine::HasRunningCompileJob(Isolate* isolate) { base::MutexGuard guard(&mutex_); DCHECK_EQ(1, isolates_.count(isolate)); - for (auto& entry : async_compile_jobs_) { + for (auto& entry : jobs_) { if (entry.first->isolate() == isolate) return true; } return false; } -void WasmEngine::AbortCompileJobsOnIsolate(Isolate* isolate) { - // Copy out to a vector first, since abortion can free {AsyncCompileJob}s and - // thus modify the {async_compile_jobs_} set. - std::vector> compile_jobs_to_abort; - { - base::MutexGuard guard(&mutex_); - DCHECK_EQ(1, isolates_.count(isolate)); - for (auto& job_entry : async_compile_jobs_) { - if (job_entry.first->isolate() != isolate) continue; - if (auto shared_job = job_entry.second.lock()) { - compile_jobs_to_abort.emplace_back(std::move(shared_job)); - } +void WasmEngine::DeleteCompileJobsOnIsolate(Isolate* isolate) { + base::MutexGuard guard(&mutex_); + DCHECK_EQ(1, isolates_.count(isolate)); + for (auto it = jobs_.begin(); it != jobs_.end();) { + if (it->first->isolate() == isolate) { + it = jobs_.erase(it); + } else { + ++it; } } - for (auto job : compile_jobs_to_abort) job->Abort(); } void WasmEngine::AddIsolate(Isolate* isolate) { diff --git a/src/wasm/wasm-engine.h b/src/wasm/wasm-engine.h index 01a0a15e08..4aa9331268 100644 --- a/src/wasm/wasm-engine.h +++ b/src/wasm/wasm-engine.h @@ -131,16 +131,16 @@ class V8_EXPORT_PRIVATE WasmEngine { CodeTracer* GetCodeTracer(); // Remove {job} from the list of active compile jobs. - void RemoveCompileJob(AsyncCompileJob* job); + std::unique_ptr RemoveCompileJob(AsyncCompileJob* job); // Returns true if at least one AsyncCompileJob that belongs to the given // Isolate is currently running. bool HasRunningCompileJob(Isolate* isolate); - // Aborts all AsyncCompileJobs that belong to the given Isolate. All + // Deletes all AsyncCompileJobs that belong to the given Isolate. All // compilation is aborted, no more callbacks will be triggered. This is used // for tearing down an isolate, or to clean it up to be reused. - void AbortCompileJobsOnIsolate(Isolate* isolate); + void DeleteCompileJobsOnIsolate(Isolate* isolate); // Manage the set of Isolates that use this WasmEngine. void AddIsolate(Isolate* isolate); @@ -155,7 +155,7 @@ class V8_EXPORT_PRIVATE WasmEngine { static std::shared_ptr GetWasmEngine(); private: - std::shared_ptr CreateAsyncCompileJob( + AsyncCompileJob* CreateAsyncCompileJob( Isolate* isolate, const WasmFeatures& enabled, std::unique_ptr bytes_copy, size_t length, Handle context, @@ -172,11 +172,9 @@ class V8_EXPORT_PRIVATE WasmEngine { ////////////////////////////////////////////////////////////////////////////// // Protected by {mutex_}: - // Keep weak_ptrs to the AsyncCompileJob so we can detect the intermediate - // state where the refcount already dropped to zero (and the weak_ptr is - // cleared) but the destructor did not run to completion yet. - std::unordered_map> - async_compile_jobs_; + // We use an AsyncCompileJob as the key for itself so that we can delete the + // job from the map when it is finished. + std::unordered_map> jobs_; std::unique_ptr compilation_stats_; std::unique_ptr code_tracer_;