Revert "[vm/concurrency] Remove --(no-)enable-isolate-groups flag in Dart VM"
This reverts commit f8df8ca78eec7ef0b6456aaad190fc64e1151c2f.
Reason for revert: breaks google3 (b/205519688)
Original change's description:
> [vm/concurrency] Remove --(no-)enable-isolate-groups flag in Dart VM
>
> The --enable-isolate-groups flag has been turned on by-default for
> months now. In this CL we're going to remove the opt-out of this (which
> was possible by explicitly passing --no-enable-isolate-groups to the VM)
>
> TEST=Existing CI. Removes flag and simplifies runtime.
>
> Change-Id: I8706b9e30df437548a81846e75e67a658d6d49d4
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/219480
> Reviewed-by: Alexander Aprelev <aam@google.com>
> Commit-Queue: Martin Kustermann <kustermann@google.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: Iabe6ccda6c184bb183b54a056d5011e657265d1d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/219700
Reviewed-by: Emmanuel Pellereau <emmanuelp@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Commit-Queue: Emmanuel Pellereau <emmanuelp@google.com>
diff --git a/runtime/lib/isolate.cc b/runtime/lib/isolate.cc
index f3bb2bd..3569eb9 100644
--- a/runtime/lib/isolate.cc
+++ b/runtime/lib/isolate.cc
@@ -111,8 +111,9 @@
// We have to check whether the reciever has the same isolate group (e.g.
// native message handlers such as an IOService handler does not but does
// share the same origin port).
- const bool same_group = PortMap::IsReceiverInThisIsolateGroup(
- destination_port_id, isolate->group());
+ const bool same_group =
+ FLAG_enable_isolate_groups && PortMap::IsReceiverInThisIsolateGroup(
+ destination_port_id, isolate->group());
// TODO(turnidge): Throw an exception when the return value is false?
PortMap::PostMessage(WriteMessage(can_send_any_object, same_group, obj,
destination_port_id,
@@ -638,7 +639,7 @@
ASSERT(name != nullptr);
auto group = state_->isolate_group();
- if (group == nullptr) {
+ if (!FLAG_enable_isolate_groups || group == nullptr) {
RunHeavyweight(name);
} else {
RunLightweight(name);
@@ -675,7 +676,8 @@
}
void RunLightweight(const char* name) {
- // The create isolate initialize callback is mandatory.
+ // The create isolate initialize callback is mandatory if
+ // --enable-isolate-groups was passed.
auto initialize_callback = Isolate::InitializeCallback();
if (initialize_callback == nullptr) {
FailedSpawn(
@@ -932,16 +934,22 @@
const auto& func = Function::Handle(zone, GetTopLevelFunction(zone, closure));
PersistentHandle* closure_tuple_handle = nullptr;
if (func.IsNull()) {
- // We have a non-toplevel closure that we might need to copy.
- // Result will be [<closure-copy>, <objects-in-msg-to-rehash>]
- const auto& closure_copy_tuple = Object::Handle(
- zone, CopyMutableObjectGraph(closure)); // Throws if it fails.
- ASSERT(closure_copy_tuple.IsArray());
- ASSERT(Object::Handle(zone, Array::Cast(closure_copy_tuple).At(0))
- .IsClosure());
- closure_tuple_handle =
- isolate->group()->api_state()->AllocatePersistentHandle();
- closure_tuple_handle->set_ptr(closure_copy_tuple.ptr());
+ if (!FLAG_enable_isolate_groups) {
+ const String& msg = String::Handle(String::New(
+ "Isolate.spawn expects to be passed a static or top-level function"));
+ Exceptions::ThrowArgumentError(msg);
+ } else {
+ // We have a non-toplevel closure that we might need to copy.
+ // Result will be [<closure-copy>, <objects-in-msg-to-rehash>]
+ const auto& closure_copy_tuple = Object::Handle(
+ zone, CopyMutableObjectGraph(closure)); // Throws if it fails.
+ ASSERT(closure_copy_tuple.IsArray());
+ ASSERT(Object::Handle(zone, Array::Cast(closure_copy_tuple).At(0))
+ .IsClosure());
+ closure_tuple_handle =
+ isolate->group()->api_state()->AllocatePersistentHandle();
+ closure_tuple_handle->set_ptr(closure_copy_tuple.ptr());
+ }
}
bool fatal_errors = fatalErrors.IsNull() ? true : fatalErrors.value();
@@ -952,8 +960,9 @@
// serializable this will throw an exception.
SerializedObjectBuffer message_buffer;
message_buffer.set_message(WriteMessage(
- /*can_send_any_object=*/true,
- /*same_group=*/true, message, ILLEGAL_PORT, Message::kNormalPriority));
+ /* can_send_any_object */ true,
+ /* same_group */ FLAG_enable_isolate_groups, message, ILLEGAL_PORT,
+ Message::kNormalPriority));
const char* utf8_package_config =
packageConfig.IsNull() ? NULL : String2UTF8(packageConfig);
diff --git a/runtime/vm/compiler/jit/compiler.cc b/runtime/vm/compiler/jit/compiler.cc
index 5b08899..1ec2adc 100644
--- a/runtime/vm/compiler/jit/compiler.cc
+++ b/runtime/vm/compiler/jit/compiler.cc
@@ -211,7 +211,7 @@
ASSERT(thread->IsMutatorThread());
const Function& function = Function::CheckedHandle(zone, arguments.ArgAt(0));
- {
+ if (FLAG_enable_isolate_groups) {
// Another isolate's mutator thread may have created [function] and
// published it via an ICData, MegamorphicCache etc. Entering the lock below
// is an acquire operation that pairs with the release operation when the
@@ -220,6 +220,12 @@
SafepointReadRwLocker ml(thread, thread->isolate_group()->program_lock());
}
+ // In single-isolate scenarios the lazy compile stub is only invoked if
+ // there's no existing code. In multi-isolate scenarios with shared JITed code
+ // we can end up in the lazy compile runtime entry here with code being
+ // installed.
+ ASSERT(!function.HasCode() || FLAG_enable_isolate_groups);
+
// Will throw if compilation failed (e.g. with compile-time error).
function.EnsureHasCode();
}
diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc
index 88dc441..1c7026a 100644
--- a/runtime/vm/dart_api_impl.cc
+++ b/runtime/vm/dart_api_impl.cc
@@ -1463,6 +1463,13 @@
*error = nullptr;
+ if (!FLAG_enable_isolate_groups) {
+ *error = Utils::StrDup(
+ "Lightweight isolates need to be explicitly enabled by passing "
+ "--enable-isolate-groups.");
+ return nullptr;
+ }
+
Isolate* isolate;
isolate = CreateWithinExistingIsolateGroup(member->group(), name, error);
if (isolate != nullptr) {
diff --git a/runtime/vm/flag_list.h b/runtime/vm/flag_list.h
index eccc2ac..224a6b1 100644
--- a/runtime/vm/flag_list.h
+++ b/runtime/vm/flag_list.h
@@ -194,6 +194,7 @@
P(retain_code_objects, bool, true, \
"Serialize all code objects even if not otherwise " \
"needed in the precompiled runtime.") \
+ P(enable_isolate_groups, bool, true, "Enable isolate group support.") \
P(show_invisible_frames, bool, false, \
"Show invisible frames in stack traces.") \
D(trace_cha, bool, false, "Trace CHA operations") \
diff --git a/runtime/vm/heap/heap_test.cc b/runtime/vm/heap/heap_test.cc
index c45765c..b30bf66 100644
--- a/runtime/vm/heap/heap_test.cc
+++ b/runtime/vm/heap/heap_test.cc
@@ -574,6 +574,9 @@
};
VM_UNIT_TEST_CASE(CleanupBequestNeverReceived) {
+ // This test uses features from isolate groups
+ FLAG_enable_isolate_groups = true;
+
const char* TEST_MESSAGE = "hello, world";
Dart_Isolate parent = TestCase::CreateTestIsolate("parent");
EXPECT_EQ(parent, Dart_CurrentIsolate());
@@ -605,6 +608,9 @@
}
VM_UNIT_TEST_CASE(ReceivesSendAndExitMessage) {
+ // This test uses features from isolate groups
+ FLAG_enable_isolate_groups = true;
+
const char* TEST_MESSAGE = "hello, world";
Dart_Isolate parent = TestCase::CreateTestIsolate("parent");
EXPECT_EQ(parent, Dart_CurrentIsolate());
diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
index 3f425e4..c1e6cbe 100644
--- a/runtime/vm/isolate.cc
+++ b/runtime/vm/isolate.cc
@@ -2638,9 +2638,11 @@
Dart::thread_pool()->Run<ShutdownGroupTask>(isolate_group);
}
} else {
- // TODO(dartbug.com/36097): An isolate just died. A significant amount of
- // memory might have become unreachable. We should evaluate how to best
- // inform the GC about this situation.
+ if (FLAG_enable_isolate_groups) {
+ // TODO(dartbug.com/36097): An isolate just died. A significant amount of
+ // memory might have become unreachable. We should evaluate how to best
+ // inform the GC about this situation.
+ }
}
} // namespace dart
@@ -2805,6 +2807,11 @@
auto thread = Thread::Current();
StoppedMutatorsScope stopped_mutators_scope(thread);
+ if (thread->IsMutatorThread() && !FLAG_enable_isolate_groups) {
+ single_current_mutator->Call();
+ return;
+ }
+
if (thread->IsAtSafepoint()) {
RELEASE_ASSERT(safepoint_handler()->IsOwnedByTheThread(thread));
single_current_mutator->Call();
diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc
index c693c02..549dd9b 100644
--- a/runtime/vm/runtime_entry.cc
+++ b/runtime/vm/runtime_entry.cc
@@ -869,6 +869,9 @@
new_cache.WriteEntryToBuffer(zone, &buffer, colliding_index, " ");
THR_Print("%s\n", buffer.buffer());
}
+ if (!FLAG_enable_isolate_groups) {
+ FATAL("Duplicate subtype test cache entry");
+ }
if (old_result.ptr() != result.ptr()) {
FATAL("Existing subtype test cache entry has result %s, not %s",
old_result.ToCString(), result.ToCString());
@@ -1243,6 +1246,9 @@
const Code& target_code = Code::Handle(zone, target_function.EnsureHasCode());
// Before patching verify that we are not repeatedly patching to the same
// target.
+ ASSERT(FLAG_enable_isolate_groups ||
+ target_code.ptr() != CodePatcher::GetStaticCallTargetAt(
+ caller_frame->pc(), caller_code));
if (target_code.ptr() !=
CodePatcher::GetStaticCallTargetAt(caller_frame->pc(), caller_code)) {
GcSafepointOperationScope safepoint(thread);
@@ -3012,6 +3018,10 @@
current_target_code.EntryPoint(),
current_target_code.is_optimized() ? "optimized" : "unoptimized");
}
+ // With isolate groups enabled, it is possible that the target code
+ // has been deactivated just now(as a result of re-optimizatin for example),
+ // which will result in another run through FixCallersTarget.
+ ASSERT(!current_target_code.IsDisabled() || FLAG_enable_isolate_groups);
arguments.SetReturn(current_target_code);
#else
UNREACHABLE();
diff --git a/runtime/vm/symbols.cc b/runtime/vm/symbols.cc
index a60edfa..673f26e 100644
--- a/runtime/vm/symbols.cc
+++ b/runtime/vm/symbols.cc
@@ -401,6 +401,11 @@
// cases.
if (thread->IsAtSafepoint()) {
RELEASE_ASSERT(group->safepoint_handler()->IsOwnedByTheThread(thread));
+ // In DEBUG mode the snapshot writer also calls this method inside a
+ // safepoint.
+#if !defined(DEBUG)
+ RELEASE_ASSERT(FLAG_enable_isolate_groups || !USING_PRODUCT);
+#endif
data = object_store->symbol_table();
CanonicalStringSet table(&key, &value, &data);
symbol ^= table.GetOrNull(str);
