Commit 605f9875 authored by Jakob Gruber's avatar Jakob Gruber Committed by Commit Bot

[compiler] Add the MapUpdater lock

It's locked exclusively in the MapUpdater API methods, and locked
shared in ComputePropertyAccessInfo (CPAI).

This lock is a step towards running CPAI on background threads. The
simple lock portion is landed separately in this CL to get an early
signal on potential lock overhead perf impact.

The lock is implemented and used very conservatively at the moment:

- it's a single global lock (and not e.g. per-map).
- it's locked for the entire method call duration (instead of only in
  relevant parts).

Both points can potentially be improved in the future.

Bug: v8:7790
Change-Id: I073423497e01b4901101973387a19962f953a576
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2797286Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73773}
parent 232bf1ff
......@@ -699,6 +699,10 @@ PropertyAccessInfo AccessInfoFactory::ComputePropertyAccessInfo(
Handle<Map> map, Handle<Name> name, AccessMode access_mode) const {
CHECK(name->IsUniqueName());
base::SharedMutexGuardIf<base::kShared> mutex_guard(
isolate()->map_updater_access(), should_lock_mutex());
MapUpdaterMutexDepthScope mumd_scope(this);
if (access_mode == AccessMode::kHas && !map->IsJSReceiverMap()) {
return PropertyAccessInfo::Invalid(zone());
}
......
......@@ -313,6 +313,28 @@ class AccessInfoFactory final {
Handle<Name> name, InternalIndex* index_out,
PropertyDetails* details_out) const;
bool should_lock_mutex() const { return map_updater_mutex_depth_ == 0; }
class MapUpdaterMutexDepthScope final {
public:
explicit MapUpdaterMutexDepthScope(const AccessInfoFactory* ptr)
: ptr_(ptr),
initial_map_updater_mutex_depth_(ptr->map_updater_mutex_depth_) {
ptr_->map_updater_mutex_depth_++;
}
~MapUpdaterMutexDepthScope() {
ptr_->map_updater_mutex_depth_--;
DCHECK_EQ(initial_map_updater_mutex_depth_,
ptr_->map_updater_mutex_depth_);
USE(initial_map_updater_mutex_depth_);
}
private:
const AccessInfoFactory* const ptr_;
const int initial_map_updater_mutex_depth_;
};
CompilationDependencies* dependencies() const { return dependencies_; }
JSHeapBroker* broker() const { return broker_; }
Isolate* isolate() const;
......@@ -323,6 +345,12 @@ class AccessInfoFactory final {
TypeCache const* const type_cache_;
Zone* const zone_;
// ComputePropertyAccessInfo can be called recursively, thus we need to
// emulate a recursive mutex. This field holds the locking depth, i.e. how
// many times the mutex has been recursively locked. Only the outermost
// locker actually locks underneath.
mutable int map_updater_mutex_depth_ = 0;
// TODO(nicohartmann@): Move to public
AccessInfoFactory(const AccessInfoFactory&) = delete;
AccessInfoFactory& operator=(const AccessInfoFactory&) = delete;
......
......@@ -656,6 +656,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
return &shared_function_info_access_;
}
base::SharedMutex* map_updater_access() { return &map_updater_access_; }
// The isolate's string table.
StringTable* string_table() { return string_table_.get(); }
......@@ -1840,6 +1842,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
base::SharedMutex internalized_string_access_;
base::SharedMutex full_transition_array_access_;
base::SharedMutex shared_function_info_access_;
base::SharedMutex map_updater_access_;
Logger* logger_ = nullptr;
StubCache* load_stub_cache_ = nullptr;
StubCache* store_stub_cache_ = nullptr;
......
......@@ -117,6 +117,10 @@ Handle<Map> MapUpdater::ReconfigureToDataField(InternalIndex descriptor,
DCHECK_EQ(kInitialized, state_);
DCHECK(descriptor.is_found());
DCHECK(!old_map_->is_dictionary_map());
base::SharedMutexGuard<base::kExclusive> mutex_guard(
isolate_->map_updater_access());
modified_descriptor_ = descriptor;
new_kind_ = kData;
new_attributes_ = attributes;
......@@ -200,6 +204,10 @@ Handle<Map> MapUpdater::ReconfigureToDataField(InternalIndex descriptor,
Handle<Map> MapUpdater::ReconfigureElementsKind(ElementsKind elements_kind) {
DCHECK_EQ(kInitialized, state_);
base::SharedMutexGuard<base::kExclusive> mutex_guard(
isolate_->map_updater_access());
new_elements_kind_ = elements_kind;
is_transitionable_fast_elements_kind_ =
IsTransitionableFastElementsKind(new_elements_kind_);
......@@ -217,6 +225,9 @@ Handle<Map> MapUpdater::Update() {
DCHECK_EQ(kInitialized, state_);
DCHECK(old_map_->is_deprecated());
base::SharedMutexGuard<base::kExclusive> mutex_guard(
isolate_->map_updater_access());
if (FindRootMap() == kEnd) return result_map_;
if (FindTargetMap() == kEnd) return result_map_;
if (ConstructNewMap() == kAtIntegrityLevelSource) {
......
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