Commit 424e7a06 authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

[wasm][cleanup] Avoid data race on has_priority_ field

After https://crrev.com/c/2529140, the actual data race should already
be fixed. This CL updates documentation (by moving the field to the
fields protected by {mutex_}), and updates {SetHighPriority} to also
take the mutex. This change is not strictly necessary, because this
method is only called right after creating the object, so no other
threads have access to it yet. But relying on that seems brittle, and
moving the initialization to the constructor is a bigger refactoring
that I don't consider worth it at the moment. The whole priority
management will probably be refactored again soon anyway.

R=ahaas@chromium.org

Bug: v8:11141
Change-Id: I496b619d551aeb584bd6e777c04ed4df076c3ae9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2529143Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71113}
parent 44d5a2a3
...@@ -595,7 +595,10 @@ class CompilationStateImpl { ...@@ -595,7 +595,10 @@ class CompilationStateImpl {
void WaitForCompilationEvent(CompilationEvent event); void WaitForCompilationEvent(CompilationEvent event);
void SetHighPriority() { has_priority_ = true; } void SetHighPriority() {
base::MutexGuard guard(&mutex_);
has_priority_ = true;
}
bool failed() const { bool failed() const {
return compile_failed_.load(std::memory_order_relaxed); return compile_failed_.load(std::memory_order_relaxed);
...@@ -665,8 +668,6 @@ class CompilationStateImpl { ...@@ -665,8 +668,6 @@ class CompilationStateImpl {
std::vector<std::shared_ptr<JSToWasmWrapperCompilationUnit>> std::vector<std::shared_ptr<JSToWasmWrapperCompilationUnit>>
js_to_wasm_wrapper_units_; js_to_wasm_wrapper_units_;
bool has_priority_ = false;
// This mutex protects all information of this {CompilationStateImpl} which is // This mutex protects all information of this {CompilationStateImpl} which is
// being accessed concurrently. // being accessed concurrently.
mutable base::Mutex mutex_; mutable base::Mutex mutex_;
...@@ -674,6 +675,8 @@ class CompilationStateImpl { ...@@ -674,6 +675,8 @@ class CompilationStateImpl {
////////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////////////
// Protected by {mutex_}: // Protected by {mutex_}:
bool has_priority_ = false;
std::shared_ptr<JobHandle> current_compile_job_; std::shared_ptr<JobHandle> current_compile_job_;
// Features detected to be used in this module. Features can be detected // Features detected to be used in this module. Features can be detected
......
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