mirror of
https://github.com/dart-lang/sdk
synced 2024-09-19 14:43:32 +00:00
[vm/concurrency] Avoid holding the program lock when installing new TTS
When intalling a new TTS stub two fields have to be updated: - AbstractType::type_test_stub - AbstractType::type_test_stub_entry_point_ Right now the code takes the program lock when updating those two fields, which is very heavy weight, since this happens very often and therefore the lock will be contended. Instead we'll solve the write-write races by ensuring they are applied using atomics in (any) order - though ensuring that they will be in sync. It is fine for them to be out-of sync for a short time, as long as we're guaranteed that whenever we hit a gc safepoint they are in sync (because GC might collect old TTS stubs that are not referenced anymore - i.e. we need to avoid situations where the cached entrypoint is old but the code pointer is new) There can be multiple mutators computing and installing a TTS at the same time, they will try to install in sequence. => This reduces the average waiting time on write access to program lock by 3x Fixes https://github.com/dart-lang/sdk/issues/46124 Issue https://github.com/dart-lang/sdk/issues/46252 TEST=Existing test coverage. Change-Id: Id1ed29adeb26fa93494e8710206fd680d427bd34 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/205520 Commit-Queue: Martin Kustermann <kustermann@google.com> Reviewed-by: Slava Egorov <vegorov@google.com>
This commit is contained in:
parent
ff4b86d2da
commit
5bd31fbd56
|
@ -3907,7 +3907,7 @@ class TypeDeserializationCluster
|
|||
for (intptr_t id = start_index_; id < stop_index_; id++) {
|
||||
type ^= refs.At(id);
|
||||
stub = TypeTestingStubGenerator::DefaultCodeForType(type);
|
||||
type.SetTypeTestingStub(stub);
|
||||
type.InitializeTypeTestingStubNonAtomic(stub);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -4032,7 +4032,7 @@ class FunctionTypeDeserializationCluster
|
|||
for (intptr_t id = start_index_; id < stop_index_; id++) {
|
||||
type ^= refs.At(id);
|
||||
stub = TypeTestingStubGenerator::DefaultCodeForType(type);
|
||||
type.SetTypeTestingStub(stub);
|
||||
type.InitializeTypeTestingStubNonAtomic(stub);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -4116,7 +4116,7 @@ class TypeRefDeserializationCluster : public DeserializationCluster {
|
|||
for (intptr_t id = start_index_; id < stop_index_; id++) {
|
||||
type_ref ^= refs.At(id);
|
||||
stub = TypeTestingStubGenerator::DefaultCodeForType(type_ref);
|
||||
type_ref.SetTypeTestingStub(stub);
|
||||
type_ref.InitializeTypeTestingStubNonAtomic(stub);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -4242,7 +4242,7 @@ class TypeParameterDeserializationCluster
|
|||
for (intptr_t id = start_index_; id < stop_index_; id++) {
|
||||
type_param ^= refs.At(id);
|
||||
stub = TypeTestingStubGenerator::DefaultCodeForType(type_param);
|
||||
type_param.SetTypeTestingStub(stub);
|
||||
type_param.InitializeTypeTestingStubNonAtomic(stub);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1249,10 +1249,10 @@ void Object::FinishInit(IsolateGroup* isolate_group) {
|
|||
Code& code = Code::Handle();
|
||||
|
||||
code = TypeTestingStubGenerator::DefaultCodeForType(*dynamic_type_);
|
||||
dynamic_type_->SetTypeTestingStub(code);
|
||||
dynamic_type_->InitializeTypeTestingStubNonAtomic(code);
|
||||
|
||||
code = TypeTestingStubGenerator::DefaultCodeForType(*void_type_);
|
||||
void_type_->SetTypeTestingStub(code);
|
||||
void_type_->InitializeTypeTestingStubNonAtomic(code);
|
||||
}
|
||||
|
||||
void Object::Cleanup() {
|
||||
|
@ -10256,7 +10256,7 @@ FunctionTypePtr FunctionType::New(intptr_t num_parent_type_arguments,
|
|||
result.SetHash(0);
|
||||
result.StoreNonPointer(&result.untag()->type_state_,
|
||||
UntaggedType::kAllocated);
|
||||
result.SetTypeTestingStub(
|
||||
result.InitializeTypeTestingStubNonAtomic(
|
||||
Code::Handle(Z, TypeTestingStubGenerator::DefaultCodeForType(result)));
|
||||
return result.ptr();
|
||||
}
|
||||
|
@ -20278,6 +20278,32 @@ const char* AbstractType::ToCString() const {
|
|||
}
|
||||
|
||||
void AbstractType::SetTypeTestingStub(const Code& stub) const {
|
||||
if (stub.IsNull()) {
|
||||
InitializeTypeTestingStubNonAtomic(stub);
|
||||
return;
|
||||
}
|
||||
|
||||
auto& old = Code::Handle(Thread::Current()->zone());
|
||||
while (true) {
|
||||
// We load the old TTS and it's entrypoint.
|
||||
old = untag()->type_test_stub<std::memory_order_acquire>();
|
||||
uword old_entry_point = old.IsNull() ? 0 : old.EntryPoint();
|
||||
|
||||
// If we can successfully update the entrypoint of the TTS, we will
|
||||
// unconditionally also set the [Code] of the TTS.
|
||||
//
|
||||
// Any competing writer would do the same, lose the compare-exchange, loop
|
||||
// around and continue loading the old [Code] TTS and continue to lose the
|
||||
// race until we have finally also updated the [Code] TTS.
|
||||
if (untag()->type_test_stub_entry_point_.compare_exchange_strong(
|
||||
old_entry_point, stub.EntryPoint())) {
|
||||
untag()->set_type_test_stub<std::memory_order_release>(stub.ptr());
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void AbstractType::InitializeTypeTestingStubNonAtomic(const Code& stub) const {
|
||||
if (stub.IsNull()) {
|
||||
// This only happens during bootstrapping when creating Type objects before
|
||||
// we have the instructions.
|
||||
|
@ -20286,9 +20312,6 @@ void AbstractType::SetTypeTestingStub(const Code& stub) const {
|
|||
untag()->set_type_test_stub(stub.ptr());
|
||||
return;
|
||||
}
|
||||
|
||||
Thread* thread = Thread::Current();
|
||||
SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock());
|
||||
StoreNonPointer(&untag()->type_test_stub_entry_point_, stub.EntryPoint());
|
||||
untag()->set_type_test_stub(stub.ptr());
|
||||
}
|
||||
|
@ -20448,7 +20471,7 @@ TypePtr Type::ToNullability(Nullability value, Heap::Space space) const {
|
|||
type ^= Object::Clone(*this, space);
|
||||
type.set_nullability(value);
|
||||
type.SetHash(0);
|
||||
type.SetTypeTestingStub(
|
||||
type.InitializeTypeTestingStubNonAtomic(
|
||||
Code::Handle(TypeTestingStubGenerator::DefaultCodeForType(type)));
|
||||
if (IsCanonical()) {
|
||||
// Object::Clone does not clone canonical bit.
|
||||
|
@ -20470,7 +20493,7 @@ FunctionTypePtr FunctionType::ToNullability(Nullability value,
|
|||
type ^= Object::Clone(*this, space);
|
||||
type.set_nullability(value);
|
||||
type.SetHash(0);
|
||||
type.SetTypeTestingStub(
|
||||
type.InitializeTypeTestingStubNonAtomic(
|
||||
Code::Handle(TypeTestingStubGenerator::DefaultCodeForType(type)));
|
||||
if (IsCanonical()) {
|
||||
// Object::Clone does not clone canonical bit.
|
||||
|
@ -21076,7 +21099,7 @@ TypePtr Type::New(const Class& clazz,
|
|||
UntaggedType::kAllocated);
|
||||
result.set_nullability(nullability);
|
||||
|
||||
result.SetTypeTestingStub(
|
||||
result.InitializeTypeTestingStubNonAtomic(
|
||||
Code::Handle(Z, TypeTestingStubGenerator::DefaultCodeForType(result)));
|
||||
return result.ptr();
|
||||
}
|
||||
|
@ -21390,7 +21413,7 @@ AbstractTypePtr TypeRef::InstantiateFrom(
|
|||
ASSERT(!instantiated_ref_type.IsTypeRef());
|
||||
instantiated_type_ref.set_type(instantiated_ref_type);
|
||||
|
||||
instantiated_type_ref.SetTypeTestingStub(Code::Handle(
|
||||
instantiated_type_ref.InitializeTypeTestingStubNonAtomic(Code::Handle(
|
||||
TypeTestingStubGenerator::DefaultCodeForType(instantiated_type_ref)));
|
||||
return instantiated_type_ref.ptr();
|
||||
}
|
||||
|
@ -21477,7 +21500,7 @@ TypeRefPtr TypeRef::New(const AbstractType& type) {
|
|||
const TypeRef& result = TypeRef::Handle(Z, TypeRef::New());
|
||||
result.set_type(type);
|
||||
|
||||
result.SetTypeTestingStub(
|
||||
result.InitializeTypeTestingStubNonAtomic(
|
||||
Code::Handle(Z, TypeTestingStubGenerator::DefaultCodeForType(result)));
|
||||
return result.ptr();
|
||||
}
|
||||
|
@ -21525,7 +21548,7 @@ TypeParameterPtr TypeParameter::ToNullability(Nullability value,
|
|||
type_parameter ^= Object::Clone(*this, space);
|
||||
type_parameter.set_nullability(value);
|
||||
type_parameter.SetHash(0);
|
||||
type_parameter.SetTypeTestingStub(Code::Handle(
|
||||
type_parameter.InitializeTypeTestingStubNonAtomic(Code::Handle(
|
||||
TypeTestingStubGenerator::DefaultCodeForType(type_parameter)));
|
||||
if (IsCanonical()) {
|
||||
// Object::Clone does not clone canonical bit.
|
||||
|
@ -21903,7 +21926,7 @@ TypeParameterPtr TypeParameter::New(const Class& parameterized_class,
|
|||
result.set_nullability(nullability);
|
||||
result.SetHash(0);
|
||||
|
||||
result.SetTypeTestingStub(
|
||||
result.InitializeTypeTestingStubNonAtomic(
|
||||
Code::Handle(Z, TypeTestingStubGenerator::DefaultCodeForType(result)));
|
||||
return result.ptr();
|
||||
}
|
||||
|
|
|
@ -8156,7 +8156,22 @@ class AbstractType : public Instance {
|
|||
}
|
||||
CodePtr type_test_stub() const { return untag()->type_test_stub(); }
|
||||
|
||||
// Sets the TTS to [stub].
|
||||
//
|
||||
// The update will ensure both fields (code as well as the cached entrypoint)
|
||||
// are updated together.
|
||||
//
|
||||
// Can be used concurrently by multiple threads - the updates will be applied
|
||||
// in undetermined order - but always consistently.
|
||||
void SetTypeTestingStub(const Code& stub) const;
|
||||
|
||||
// Sets the TTS to the [stub].
|
||||
//
|
||||
// The caller has to ensure no other thread can concurrently try to update the
|
||||
// TTS. This should mainly be used when initializing newly allocated Type
|
||||
// objects.
|
||||
void InitializeTypeTestingStubNonAtomic(const Code& stub) const;
|
||||
|
||||
void UpdateTypeTestingStubEntryPoint() const {
|
||||
StoreNonPointer(&untag()->type_test_stub_entry_point_,
|
||||
Code::EntryPointOf(untag()->type_test_stub()));
|
||||
|
|
|
@ -2644,9 +2644,10 @@ class UntaggedAbstractType : public UntaggedInstance {
|
|||
|
||||
protected:
|
||||
static constexpr intptr_t kTypeStateBitSize = 2;
|
||||
COMPILE_ASSERT(sizeof(std::atomic<word>) == sizeof(word));
|
||||
|
||||
// Accessed from generated code.
|
||||
uword type_test_stub_entry_point_;
|
||||
std::atomic<uword> type_test_stub_entry_point_;
|
||||
COMPRESSED_POINTER_FIELD(CodePtr, type_test_stub)
|
||||
VISIT_FROM(type_test_stub)
|
||||
|
||||
|
|
|
@ -113,7 +113,7 @@ TypePtr Type::ReadFrom(SnapshotReader* reader,
|
|||
// Fill in the type testing stub.
|
||||
Code& code = *reader->CodeHandle();
|
||||
code = TypeTestingStubGenerator::DefaultCodeForType(type);
|
||||
type.SetTypeTestingStub(code);
|
||||
type.InitializeTypeTestingStubNonAtomic(code);
|
||||
|
||||
if (is_canonical) {
|
||||
type ^= type.Canonicalize(Thread::Current(), nullptr);
|
||||
|
@ -194,7 +194,7 @@ TypeRefPtr TypeRef::ReadFrom(SnapshotReader* reader,
|
|||
// Fill in the type testing stub.
|
||||
Code& code = *reader->CodeHandle();
|
||||
code = TypeTestingStubGenerator::DefaultCodeForType(type_ref);
|
||||
type_ref.SetTypeTestingStub(code);
|
||||
type_ref.InitializeTypeTestingStubNonAtomic(code);
|
||||
|
||||
return type_ref.ptr();
|
||||
}
|
||||
|
@ -256,7 +256,7 @@ TypeParameterPtr TypeParameter::ReadFrom(SnapshotReader* reader,
|
|||
// Fill in the type testing stub.
|
||||
Code& code = *reader->CodeHandle();
|
||||
code = TypeTestingStubGenerator::DefaultCodeForType(type_parameter);
|
||||
type_parameter.SetTypeTestingStub(code);
|
||||
type_parameter.InitializeTypeTestingStubNonAtomic(code);
|
||||
|
||||
if (is_canonical) {
|
||||
type_parameter ^= type_parameter.Canonicalize(Thread::Current(), nullptr);
|
||||
|
|
|
@ -362,7 +362,7 @@ void SnapshotReader::RunDelayedTypePostprocessing() {
|
|||
for (intptr_t i = 0; i < types_to_postprocess_.Length(); ++i) {
|
||||
type ^= types_to_postprocess_.At(i);
|
||||
code = TypeTestingStubGenerator::DefaultCodeForType(type);
|
||||
type.SetTypeTestingStub(code);
|
||||
type.InitializeTypeTestingStubNonAtomic(code);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue