[vm/send_and_exit] Allow for safepoints when validating exit message.

``` before
$ dart isolate_exit_pause_time_test.dart
.Min(RunTimeRaw): 0.001 ms.
.Avg(RunTimeRaw): 212.399592 ms.
.Percentile50(RunTimeRaw): 49.055 ms.
.Percentile90(RunTimeRaw): 809.049 ms.
.Percentile95(RunTimeRaw): 1009.049 ms.
.Percentile99(RunTimeRaw): 1169.049 ms.
.Max(RunTimeRaw): 1208.049 ms.
```

``` after
$ dart isolate_exit_pause_time_test.dart
.Min(RunTimeRaw): 0.001 ms.
.Avg(RunTimeRaw): 53.87510125 ms.
.Percentile50(RunTimeRaw): 8.05 ms.
.Percentile90(RunTimeRaw): 174.005 ms.
.Percentile95(RunTimeRaw): 217.975 ms.
.Percentile99(RunTimeRaw): 275.033 ms.
.Max(RunTimeRaw): 314.033 ms.
```

Remaining latency after the change is due to large root set formed by pointers_to_verify_at_exit_ that have to be visited during GC.

Fixes https://github.com/dart-lang/sdk/issues/49050
TEST=ci

Change-Id: I1ed7af017f80890900b137a1514e5ffdeb63f53f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246482
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
This commit is contained in:
Alexander Aprelev 2022-06-08 16:27:22 +00:00 committed by Commit Bot
parent 88c2f888ea
commit f681734aef
3 changed files with 110 additions and 77 deletions

View file

@ -144,6 +144,17 @@ static ObjectPtr ValidateMessageObject(Zone* zone,
visited_(visited),
working_set_(working_set) {}
void VisitObject(ObjectPtr obj) {
if (!obj->IsHeapObject() || obj->untag()->IsCanonical()) {
return;
}
if (visited_->GetValueExclusive(obj) == 1) {
return;
}
visited_->SetValueExclusive(obj, 1);
working_set_->Add(obj);
}
private:
void VisitPointers(ObjectPtr* from, ObjectPtr* to) {
for (ObjectPtr* ptr = from; ptr <= to; ptr++) {
@ -159,17 +170,6 @@ static ObjectPtr ValidateMessageObject(Zone* zone,
}
}
void VisitObject(ObjectPtr obj) {
if (!obj->IsHeapObject() || obj->untag()->IsCanonical()) {
return;
}
if (visited_->GetValueExclusive(obj) == 1) {
return;
}
visited_->SetValueExclusive(obj, 1);
working_set_->Add(obj);
}
WeakTable* visited_;
MallocGrowableArray<ObjectPtr>* const working_set_;
};
@ -185,55 +185,76 @@ static ObjectPtr ValidateMessageObject(Zone* zone,
Function& erroneous_closure_function = Function::Handle(zone);
Class& erroneous_nativewrapper_class = Class::Handle(zone);
Class& erroneous_finalizable_class = Class::Handle(zone);
Array& array = Array::Handle(zone);
const char* error_message = nullptr;
Thread* thread = Thread::Current();
{
NoSafepointScope no_safepoint;
// working_set contains only elements that have not been visited yet that
// need to be processed.
// So before adding elements to working_set ensure to check visited flag,
// set visited flag at the same time as the element is added.
MallocGrowableArray<ObjectPtr> working_set;
std::unique_ptr<WeakTable> visited(new WeakTable());
// working_set contains only elements that have not been visited yet that
// need to be processed.
// So before adding elements to working_set ensure to check visited flag,
// set visited flag at the same time as the element is added.
SendMessageValidator visitor(isolate->group(), visited.get(), &working_set);
// This working set of raw pointers is visited by GC, only one for a given
// isolate should be in use.
MallocGrowableArray<ObjectPtr>* const working_set =
isolate->pointers_to_verify_at_exit();
ASSERT(working_set->length() == 0);
std::unique_ptr<WeakTable> visited(new WeakTable());
visited->SetValueExclusive(obj.ptr(), 1);
working_set.Add(obj.ptr());
SendMessageValidator visitor(isolate->group(), visited.get(), working_set);
while (!working_set.is_empty() && !error_found) {
ObjectPtr raw = working_set.RemoveLast();
visited->SetValueExclusive(obj.ptr(), 1);
working_set->Add(obj.ptr());
const intptr_t cid = raw->GetClassId();
// Keep the list in sync with the one in runtime/vm/object_graph_copy.cc
switch (cid) {
// Can be shared.
case kOneByteStringCid:
case kTwoByteStringCid:
case kExternalOneByteStringCid:
case kExternalTwoByteStringCid:
case kMintCid:
case kImmutableArrayCid:
case kNeverCid:
case kSentinelCid:
case kInt32x4Cid:
case kSendPortCid:
case kCapabilityCid:
case kRegExpCid:
case kStackTraceCid:
continue;
// Cannot be shared due to possibly being mutable boxes for unboxed
// fields in JIT, but can be transferred via Isolate.exit()
case kDoubleCid:
case kFloat32x4Cid:
case kFloat64x2Cid:
continue;
while (!working_set->is_empty() && !error_found) {
thread->CheckForSafepoint();
case kClosureCid:
closure ^= raw;
// Only context has to be checked.
working_set.Add(closure.context());
continue;
ObjectPtr raw = working_set->RemoveLast();
const intptr_t cid = raw->GetClassId();
// Keep the list in sync with the one in runtime/vm/object_graph_copy.cc
switch (cid) {
// Can be shared.
case kOneByteStringCid:
case kTwoByteStringCid:
case kExternalOneByteStringCid:
case kExternalTwoByteStringCid:
case kMintCid:
case kImmutableArrayCid:
case kNeverCid:
case kSentinelCid:
case kInt32x4Cid:
case kSendPortCid:
case kCapabilityCid:
case kRegExpCid:
case kStackTraceCid:
continue;
// Cannot be shared due to possibly being mutable boxes for unboxed
// fields in JIT, but can be transferred via Isolate.exit()
case kDoubleCid:
case kFloat32x4Cid:
case kFloat64x2Cid:
continue;
case kArrayCid:
{
array ^= Array::RawCast(raw);
visitor.VisitObject(array.GetTypeArguments());
const intptr_t batch_size = (2 << 14) - 1;
for (intptr_t i = 0; i < array.Length(); ++i) {
ObjectPtr ptr = array.At(i);
visitor.VisitObject(ptr);
if ((i & batch_size) == batch_size) {
thread->CheckForSafepoint();
}
}
continue;
}
case kClosureCid:
closure ^= raw;
// Only context has to be checked.
working_set->Add(closure.context());
continue;
#define MESSAGE_SNAPSHOT_ILLEGAL(type) \
case k##type##Cid: \
@ -242,33 +263,32 @@ static ObjectPtr ValidateMessageObject(Zone* zone,
error_found = true; \
break;
MESSAGE_SNAPSHOT_ILLEGAL(DynamicLibrary);
// TODO(http://dartbug.com/47777): Send and exit support: remove this.
MESSAGE_SNAPSHOT_ILLEGAL(Finalizer);
MESSAGE_SNAPSHOT_ILLEGAL(NativeFinalizer);
MESSAGE_SNAPSHOT_ILLEGAL(MirrorReference);
MESSAGE_SNAPSHOT_ILLEGAL(Pointer);
MESSAGE_SNAPSHOT_ILLEGAL(ReceivePort);
MESSAGE_SNAPSHOT_ILLEGAL(UserTag);
MESSAGE_SNAPSHOT_ILLEGAL(SuspendState);
MESSAGE_SNAPSHOT_ILLEGAL(DynamicLibrary);
// TODO(http://dartbug.com/47777): Send and exit support: remove this.
MESSAGE_SNAPSHOT_ILLEGAL(Finalizer);
MESSAGE_SNAPSHOT_ILLEGAL(NativeFinalizer);
MESSAGE_SNAPSHOT_ILLEGAL(MirrorReference);
MESSAGE_SNAPSHOT_ILLEGAL(Pointer);
MESSAGE_SNAPSHOT_ILLEGAL(ReceivePort);
MESSAGE_SNAPSHOT_ILLEGAL(UserTag);
MESSAGE_SNAPSHOT_ILLEGAL(SuspendState);
default:
if (cid >= kNumPredefinedCids) {
klass = class_table->At(cid);
if (klass.num_native_fields() != 0) {
erroneous_nativewrapper_class = klass.ptr();
error_found = true;
break;
}
if (klass.implements_finalizable()) {
erroneous_finalizable_class = klass.ptr();
error_found = true;
break;
}
default:
if (cid >= kNumPredefinedCids) {
klass = class_table->At(cid);
if (klass.num_native_fields() != 0) {
erroneous_nativewrapper_class = klass.ptr();
error_found = true;
break;
}
}
raw->untag()->VisitPointers(&visitor);
if (klass.implements_finalizable()) {
erroneous_finalizable_class = klass.ptr();
error_found = true;
break;
}
}
}
raw->untag()->VisitPointers(&visitor);
}
if (error_found) {
const char* exception_message;
@ -292,9 +312,11 @@ static ObjectPtr ValidateMessageObject(Zone* zone,
" : (object implements Finalizable - %s)",
erroneous_finalizable_class.ToCString());
}
working_set->Clear();
return Exceptions::CreateUnhandledException(
zone, Exceptions::kArgumentValue, exception_message);
}
ASSERT(working_set->length() == 0);
isolate->set_forward_table_new(nullptr);
return obj.ptr();
}

View file

@ -2823,6 +2823,11 @@ void Isolate::VisitObjectPointers(ObjectPointerVisitor* visitor,
visitor->VisitPointer(
reinterpret_cast<ObjectPtr*>(&loaded_prefixes_set_storage_));
if (pointers_to_verify_at_exit_.length() != 0) {
visitor->VisitPointers(&pointers_to_verify_at_exit_[0],
pointers_to_verify_at_exit_.length());
}
}
void IsolateGroup::ReleaseStoreBuffers() {

View file

@ -1477,6 +1477,10 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
bool IsPrefixLoaded(const LibraryPrefix& prefix) const;
void SetPrefixIsLoaded(const LibraryPrefix& prefix);
MallocGrowableArray<ObjectPtr>* pointers_to_verify_at_exit() {
return &pointers_to_verify_at_exit_;
}
private:
friend class Dart; // Init, InitOnce, Shutdown.
friend class IsolateKillerVisitor; // Kill().
@ -1735,6 +1739,8 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
ArrayPtr loaded_prefixes_set_storage_;
MallocGrowableArray<ObjectPtr> pointers_to_verify_at_exit_;
#define REUSABLE_FRIEND_DECLARATION(name) \
friend class Reusable##name##HandleScope;
REUSABLE_HANDLE_LIST(REUSABLE_FRIEND_DECLARATION)