Skip to content

Commit fc54f90

Browse files
mkustermanncommit-bot@chromium.org
authored and
commit-bot@chromium.org
committed
[vm] Cleanup some outdated code from hot-reload (and deferred loading)
Hot-reload used to be implemented by calling a tag handler (i.e. embedder), which would load new Dart source code and call recursively into hot reloading code. => This no longer happens, since the hot reload implementation obtains and loads the kernel diffs itself in isolate_reload.cc. With the switch to the Dart 2.0 CFE the VM no longer supports any kind of deferred library loading. The CFE provides the VM with all sources together. This CL cleans up some unnecessary code and outdated comments. Issue #36097 Change-Id: I63aaa6e2a1910165185172a9dd975c3974ac5dd5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/110720 Reviewed-by: Alexander Aprelev <aam@google.com> Commit-Queue: Martin Kustermann <kustermann@google.com>
1 parent fc82ed2 commit fc54f90

7 files changed

+15
-68
lines changed

runtime/vm/dart_api_impl.cc

-4
Original file line numberDiff line numberDiff line change
@@ -5473,8 +5473,6 @@ DART_EXPORT Dart_Handle Dart_FinalizeLoading(bool complete_futures) {
54735473
// instead of freelists.
54745474
BumpAllocateScope bump_allocate_scope(T);
54755475

5476-
I->DoneLoading();
5477-
54785476
// TODO(hausner): move the remaining code below (finalization and
54795477
// invoking of _completeDeferredLoads) into Isolate::DoneLoading().
54805478

@@ -5484,8 +5482,6 @@ DART_EXPORT Dart_Handle Dart_FinalizeLoading(bool complete_futures) {
54845482
return state;
54855483
}
54865484

5487-
I->DoneFinalizing();
5488-
54895485
#if !defined(PRODUCT)
54905486
// Now that the newly loaded classes are finalized, notify the debugger
54915487
// that new code has been loaded. If there are latent breakpoints in

runtime/vm/isolate.cc

-23
Original file line numberDiff line numberDiff line change
@@ -1404,21 +1404,6 @@ void Isolate::BuildName(const char* name_prefix) {
14041404
}
14051405
}
14061406

1407-
void Isolate::DoneLoading() {
1408-
GrowableObjectArray& libs =
1409-
GrowableObjectArray::Handle(current_zone(), object_store()->libraries());
1410-
Library& lib = Library::Handle(current_zone());
1411-
intptr_t num_libs = libs.Length();
1412-
for (intptr_t i = 0; i < num_libs; i++) {
1413-
lib ^= libs.At(i);
1414-
// If this library was loaded with Dart_LoadLibrary, it was marked
1415-
// as 'load in progres'. Set the status to 'loaded'.
1416-
if (lib.LoadInProgress()) {
1417-
lib.SetLoaded();
1418-
}
1419-
}
1420-
}
1421-
14221407
#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
14231408
bool Isolate::CanReload() const {
14241409
return !Isolate::IsVMInternalIsolate(this) && is_runnable() &&
@@ -1475,14 +1460,6 @@ void Isolate::DeleteReloadContext() {
14751460
}
14761461
#endif // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
14771462

1478-
void Isolate::DoneFinalizing() {
1479-
#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
1480-
if (IsReloading()) {
1481-
reload_context_->FinalizeLoading();
1482-
}
1483-
#endif // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
1484-
}
1485-
14861463
const char* Isolate::MakeRunnable() {
14871464
ASSERT(Isolate::Current() == nullptr);
14881465

runtime/vm/isolate.h

-4
Original file line numberDiff line numberDiff line change
@@ -422,10 +422,6 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
422422

423423
void ScheduleInterrupts(uword interrupt_bits);
424424

425-
// Marks all libraries as loaded.
426-
void DoneLoading();
427-
void DoneFinalizing();
428-
429425
#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
430426
// By default the reload context is deleted. This parameter allows
431427
// the caller to delete is separately if it is still needed.

runtime/vm/isolate_reload.cc

+6-25
Original file line numberDiff line numberDiff line change
@@ -736,26 +736,11 @@ void IsolateReloadContext::Reload(bool force_reload,
736736
DeoptimizeDependentCode();
737737
Checkpoint();
738738

739-
// WEIRD CONTROL FLOW BEGINS.
739+
// We synchronously load the hot-reload kernel diff (which includes changed
740+
// libraries and any libraries transitively depending on them).
740741
//
741-
// The flow of execution until we return from the tag handler can be complex.
742-
//
743-
// On a successful load, the following will occur:
744-
// 1) Tag Handler is invoked and the embedder is in control.
745-
// 2) All sources and libraries are loaded.
746-
// 3) Dart_FinalizeLoading is called by the embedder.
747-
// 4) Dart_FinalizeLoading invokes IsolateReloadContext::FinalizeLoading
748-
// and we are temporarily back in control.
749-
// This is where we validate the reload and commit or reject.
750-
// 5) Dart_FinalizeLoading invokes Dart code related to deferred libraries.
751-
// 6) The tag handler returns and we move on.
752-
//
753-
// Even after a successful reload the Dart code invoked in (5) can result
754-
// in an Unwind error or an UnhandledException error. This error will be
755-
// returned by the tag handler. The tag handler can return other errors,
756-
// for example, top level parse errors. We want to capture these errors while
757-
// propagating the UnwindError or an UnhandledException error.
758-
742+
// If loading the hot-reload diff succeeded we'll finalize the loading, which
743+
// will either commit or reject the reload request.
759744
{
760745
const Object& tmp =
761746
kernel::KernelLoader::LoadEntireProgram(kernel_program.get());
@@ -782,8 +767,6 @@ void IsolateReloadContext::Reload(bool force_reload,
782767
result = tmp.raw();
783768
}
784769
}
785-
//
786-
// WEIRD CONTROL FLOW ENDS.
787770

788771
// Re-enable the background compiler. Do this before propagating any errors.
789772
BackgroundCompiler::Enable(I);
@@ -840,8 +823,6 @@ void IsolateReloadContext::RegisterClass(const Class& new_cls) {
840823
AddClassMapping(new_cls, old_cls);
841824
}
842825

843-
// FinalizeLoading will be called *before* Reload() returns but will not be
844-
// called if the embedder fails to load sources.
845826
void IsolateReloadContext::FinalizeLoading() {
846827
if (reload_skipped_ || reload_finalized_) {
847828
return;
@@ -1475,9 +1456,9 @@ void IsolateReloadContext::Commit() {
14751456
if (new_cls.raw() != old_cls.raw()) {
14761457
ASSERT(new_cls.is_enum_class() == old_cls.is_enum_class());
14771458
if (new_cls.is_enum_class() && new_cls.is_finalized()) {
1478-
new_cls.ReplaceEnum(old_cls);
1459+
new_cls.ReplaceEnum(this, old_cls);
14791460
} else {
1480-
new_cls.CopyStaticFieldValues(old_cls);
1461+
new_cls.CopyStaticFieldValues(this, old_cls);
14811462
}
14821463
old_cls.PatchFieldsAndFunctions();
14831464
old_cls.MigrateImplicitStaticClosures(this, new_cls);

runtime/vm/object.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -1364,8 +1364,10 @@ class Class : public Object {
13641364
bool TraceAllocation(Isolate* isolate) const;
13651365
void SetTraceAllocation(bool trace_allocation) const;
13661366

1367-
void ReplaceEnum(const Class& old_enum) const;
1368-
void CopyStaticFieldValues(const Class& old_cls) const;
1367+
void ReplaceEnum(IsolateReloadContext* reload_context,
1368+
const Class& old_enum) const;
1369+
void CopyStaticFieldValues(IsolateReloadContext* reload_context,
1370+
const Class& old_cls) const;
13691371
void PatchFieldsAndFunctions() const;
13701372
void MigrateImplicitStaticClosures(IsolateReloadContext* context,
13711373
const Class& new_cls) const;

runtime/vm/object_reload.cc

+5-9
Original file line numberDiff line numberDiff line change
@@ -183,13 +183,11 @@ void ObjectPool::ResetICDatas(Zone* zone) const {
183183
#endif
184184
}
185185

186-
void Class::CopyStaticFieldValues(const Class& old_cls) const {
186+
void Class::CopyStaticFieldValues(IsolateReloadContext* reload_context,
187+
const Class& old_cls) const {
187188
// We only update values for non-enum classes.
188189
const bool update_values = !is_enum_class();
189190

190-
IsolateReloadContext* reload_context = Isolate::Current()->reload_context();
191-
ASSERT(reload_context != NULL);
192-
193191
const Array& old_field_list = Array::Handle(old_cls.fields());
194192
Field& old_field = Field::Handle();
195193
String& old_name = String::Handle();
@@ -285,17 +283,15 @@ class EnumMapTraits {
285283
// When an enum value is deleted, we 'become' all references to the 'deleted'
286284
// sentinel value. The index value is -1.
287285
//
288-
void Class::ReplaceEnum(const Class& old_enum) const {
286+
void Class::ReplaceEnum(IsolateReloadContext* reload_context,
287+
const Class& old_enum) const {
289288
// We only do this for finalized enum classes.
290289
ASSERT(is_enum_class());
291290
ASSERT(old_enum.is_enum_class());
292291
ASSERT(is_finalized());
293292
ASSERT(old_enum.is_finalized());
294293

295-
Thread* thread = Thread::Current();
296-
Zone* zone = thread->zone();
297-
IsolateReloadContext* reload_context = Isolate::Current()->reload_context();
298-
ASSERT(reload_context != NULL);
294+
Zone* zone = Thread::Current()->zone();
299295

300296
Array& enum_fields = Array::Handle(zone);
301297
Field& field = Field::Handle(zone);

runtime/vm/symbols.h

-1
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@ class ObjectPointerVisitor;
193193
V(List, "List") \
194194
V(ListFactory, "List.") \
195195
V(ListLiteralFactory, "List._fromLiteral") \
196-
V(LoadLibrary, "loadLibrary") \
197196
V(LocalVarDescriptors, "LocalVarDescriptors") \
198197
V(Map, "Map") \
199198
V(MapLiteralFactory, "Map._fromLiteral") \

0 commit comments

Comments
 (0)