From 0b3f82b5c7410b525f7c3df9300e5cb7d563c079 Mon Sep 17 00:00:00 2001 From: Santiago Aboy Solanes Date: Wed, 24 Feb 2021 16:21:20 +0000 Subject: [PATCH] [cleanup] Improve the concurrent SharedMutexes comments and naming Rename string_access to internalized_string_access and transition_array_access to full_transition_array_access. Bug: v8:7790 Change-Id: I225f90d4c0d6ddac1f15796848428bc962a2420f Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2718145 Reviewed-by: Georg Neis Reviewed-by: Jakob Gruber Commit-Queue: Santiago Aboy Solanes Cr-Commit-Position: refs/heads/master@{#73044} --- src/execution/isolate.h | 25 +++++++++++++++---------- src/execution/local-isolate.h | 4 +++- src/objects/string-inl.h | 5 +++-- src/objects/string.cc | 4 ++-- src/objects/transitions.cc | 6 +++--- 5 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/execution/isolate.h b/src/execution/isolate.h index 6c458860e8..978092621e 100644 --- a/src/execution/isolate.h +++ b/src/execution/isolate.h @@ -631,20 +631,25 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { // Mutex for serializing access to break control structures. base::RecursiveMutex* break_access() { return &break_access_; } - // Shared mutex for allowing concurrent read/writes to FeedbackVectors. + // Shared mutex for allowing thread-safe concurrent reads of FeedbackVectors. base::SharedMutex* feedback_vector_access() { return &feedback_vector_access_; } - // Shared mutex for allowing concurrent read/writes to Strings. - base::SharedMutex* string_access() { return &string_access_; } - - // Shared mutex for allowing concurrent read/writes to TransitionArrays. - base::SharedMutex* transition_array_access() { - return &transition_array_access_; + // Shared mutex for allowing thread-safe concurrent reads of + // InternalizedStrings. + base::SharedMutex* internalized_string_access() { + return &internalized_string_access_; } - // Shared mutex for allowing concurrent read/writes to SharedFunctionInfos. + // Shared mutex for allowing thread-safe concurrent reads of TransitionArrays + // of kind kFullTransitionArray. + base::SharedMutex* full_transition_array_access() { + return &full_transition_array_access_; + } + + // Shared mutex for allowing thread-safe concurrent reads of + // SharedFunctionInfos. base::SharedMutex* shared_function_info_access() { return &shared_function_info_access_; } @@ -1805,8 +1810,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { std::shared_ptr async_counters_; base::RecursiveMutex break_access_; base::SharedMutex feedback_vector_access_; - base::SharedMutex string_access_; - base::SharedMutex transition_array_access_; + base::SharedMutex internalized_string_access_; + base::SharedMutex full_transition_array_access_; base::SharedMutex shared_function_info_access_; Logger* logger_ = nullptr; StubCache* load_stub_cache_ = nullptr; diff --git a/src/execution/local-isolate.h b/src/execution/local-isolate.h index c55f8dc65e..6d054b0cef 100644 --- a/src/execution/local-isolate.h +++ b/src/execution/local-isolate.h @@ -53,7 +53,9 @@ class V8_EXPORT_PRIVATE LocalIsolate final : private HiddenLocalFactory { inline Object root(RootIndex index) const; StringTable* string_table() const { return isolate_->string_table(); } - base::SharedMutex* string_access() { return isolate_->string_access(); } + base::SharedMutex* internalized_string_access() { + return isolate_->internalized_string_access(); + } v8::internal::LocalFactory* factory() { // Upcast to the privately inherited base-class using c-style casts to avoid diff --git a/src/objects/string-inl.h b/src/objects/string-inl.h index 5be0141ab8..cead7e51d1 100644 --- a/src/objects/string-inl.h +++ b/src/objects/string-inl.h @@ -38,14 +38,15 @@ class V8_NODISCARD SharedStringAccessGuardIfNeeded { // from a background thread. explicit SharedStringAccessGuardIfNeeded(LocalIsolate* local_isolate) { if (IsNeeded(local_isolate)) { - mutex_guard.emplace(local_isolate->string_access()); + mutex_guard.emplace(local_isolate->internalized_string_access()); } } // Slow version which gets the isolate from the String. explicit SharedStringAccessGuardIfNeeded(String str) { Isolate* isolate = GetIsolateIfNeeded(str); - if (isolate != nullptr) mutex_guard.emplace(isolate->string_access()); + if (isolate != nullptr) + mutex_guard.emplace(isolate->internalized_string_access()); } static SharedStringAccessGuardIfNeeded NotNeeded() { diff --git a/src/objects/string.cc b/src/objects/string.cc index b9413618ee..9c1a61bdc9 100644 --- a/src/objects/string.cc +++ b/src/objects/string.cc @@ -171,7 +171,7 @@ bool String::MakeExternal(v8::String::ExternalStringResource* resource) { } base::SharedMutexGuard shared_mutex_guard( - isolate->string_access()); + isolate->internalized_string_access()); // Morph the string to an external string by replacing the map and // reinitializing the fields. This won't work if the space the existing // string occupies is too small for a regular external string. Instead, we @@ -249,7 +249,7 @@ bool String::MakeExternal(v8::String::ExternalOneByteStringResource* resource) { } base::SharedMutexGuard shared_mutex_guard( - isolate->string_access()); + isolate->internalized_string_access()); // Morph the string to an external string by replacing the map and // reinitializing the fields. This won't work if the space the existing // string occupies is too small for a regular external string. Instead, we diff --git a/src/objects/transitions.cc b/src/objects/transitions.cc index 75ca763a1b..e4fb97bea1 100644 --- a/src/objects/transitions.cc +++ b/src/objects/transitions.cc @@ -145,7 +145,7 @@ void TransitionsAccessor::Insert(Handle name, Handle target, // If an existing entry was found, overwrite it and return. if (index != kNotFound) { base::SharedMutexGuard shared_mutex_guard( - isolate_->transition_array_access()); + isolate_->full_transition_array_access()); array.SetRawTarget(index, HeapObjectReference::Weak(*target)); return; } @@ -158,7 +158,7 @@ void TransitionsAccessor::Insert(Handle name, Handle target, // If there is enough capacity, insert new entry into the existing array. if (new_nof <= array.Capacity()) { base::SharedMutexGuard shared_mutex_guard( - isolate_->transition_array_access()); + isolate_->full_transition_array_access()); array.SetNumberOfTransitions(new_nof); for (int i = number_of_transitions; i > insertion_index; --i) { array.SetKey(i, array.GetKey(i - 1)); @@ -231,7 +231,7 @@ Map TransitionsAccessor::SearchTransition(Name name, PropertyKind kind, } case kFullTransitionArray: { base::SharedMutexGuardIf scope( - isolate_->transition_array_access(), concurrent_access_); + isolate_->full_transition_array_access(), concurrent_access_); return transitions().SearchAndGetTarget(kind, name, attributes); } }