Only reload libraries when they may have been modified.

runtime/vm/isolate_reload.cc

Index: runtime/vm/isolate_reload.cc
diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc
index 75102c9d1f089b71142cf02708a03df53f6d9a95..cf7c69c3c6aed3dcb48635b9e411cac372495e68 100644
--- a/runtime/vm/isolate_reload.cc
+++ b/runtime/vm/isolate_reload.cc
@@ -5,6 +5,7 @@
 #include "vm/isolate_reload.h"
 
 #include "vm/become.h"
+#include "vm/bit_vector.h"
 #include "vm/code_generator.h"
 #include "vm/compiler.h"
 #include "vm/dart_api_impl.h"
@@ -179,11 +180,14 @@ bool IsolateReloadContext::IsSameLibrary(
 
 IsolateReloadContext::IsolateReloadContext(Isolate* isolate)
     : start_time_micros_(OS::GetCurrentMonotonicMicros()),
+      reload_timestamp_(OS::GetCurrentTimeMillis()),

[Cutch, 2016/07/29 14:37:04]

Why does the reload context and the isolate have a reload timestamp? It looks
like you use the isolate's below.

[turnidge, 2016/07/29 17:37:37]

The isolate's reload timestamp is the timestamp of the last successful reload.

The IRC's reload timestamp is the timestamp that *will be* recorded if the
current reload succeeds.  This happens in isolate.cc.

I have renamed the isolate's timestamp to be "last_reload_timestamp" to make the
distinction more clear.  I have also moved setting the last_reload_timestamp
into isolate_reload.cc.

       isolate_(isolate),
+      reload_skipped_(false),
       has_error_(false),
       saved_num_cids_(-1),
       saved_class_table_(NULL),
       num_saved_libs_(-1),
+      modified_libs_(NULL),
       script_uri_(String::null()),
       error_(Error::null()),
       old_classes_set_storage_(Array::null()),
@@ -227,7 +231,7 @@ void IsolateReloadContext::ReportSuccess() {
 }
 
 
-void IsolateReloadContext::StartReload() {
+void IsolateReloadContext::StartReload(bool force_reload) {
   TIMELINE_SCOPE(Reload);
   Thread* thread = Thread::Current();
   ASSERT(isolate() == thread->isolate());
@@ -237,6 +241,15 @@ void IsolateReloadContext::StartReload() {
   ASSERT(!root_lib.IsNull());
   const String& root_lib_url = String::Handle(root_lib.url());
 
+  // Check to see which libraries have been modified.
+  modified_libs_ = FindModifiedLibraries(force_reload);
+  if (!modified_libs_->Contains(root_lib.index())) {
+    ASSERT(modified_libs_->IsEmpty());
+    reload_skipped_ = true;
+    TIR_Print("Skipping reload.  No libraries were modified\n");
+    return;
+  }
+
   // Preallocate storage for maps.
   old_classes_set_storage_ =
       HashTables::New<UnorderedHashSet<ClassMapTraits> >(4);
@@ -313,6 +326,9 @@ void IsolateReloadContext::RegisterClass(const Class& new_cls) {
 
 
 void IsolateReloadContext::FinishReload() {
+  if (reload_skipped_) {
+    return;
+  }
   BuildLibraryMapping();
   TIR_Print("---- DONE FINALIZING\n");
   if (ValidateReload()) {
@@ -327,7 +343,7 @@ void IsolateReloadContext::FinishReload() {
   RebuildDirectSubclasses();
 
   if (FLAG_write_protect_code) {
-    // Disable code page write protection while we are reloading.
+    // Re-enable code page write protection.
     I->heap()->WriteProtectCode(true);
   }
 
@@ -434,8 +450,125 @@ void IsolateReloadContext::CheckpointClasses() {
 }
 
 
-bool IsolateReloadContext::IsCleanLibrary(const Library& lib) {
-  return lib.is_dart_scheme();
+Dart_FileModifiedCallback IsolateReloadContext::file_modified_callback_ = NULL;
+
+
+bool IsolateReloadContext::ScriptModifiedSince(const Script& script,
+                                               int64_t since) {
+  if (file_modified_callback_ == NULL) {
+    return true;
+  }
+  // We use the resolved url to determine if the script has been modified.
+  const String& url = String::Handle(script.resolved_url());
+  const char* url_chars = url.ToCString();
+  return (*file_modified_callback_)(url_chars, since);
+}
+
+
+static void PropagateLibraryModified(
+    const ZoneGrowableArray<ZoneGrowableArray<intptr_t>* >* imported_by,
+    intptr_t lib_index,
+    BitVector* modified_libs) {
+  ZoneGrowableArray<intptr_t>* dep_libs = (*imported_by)[lib_index];
+  for (intptr_t i = 0; i < dep_libs->length(); i++) {
+    intptr_t dep_lib_index = (*dep_libs)[i];
+    if (!modified_libs->Contains(dep_lib_index)) {
+      modified_libs->Add(dep_lib_index);
+      PropagateLibraryModified(imported_by, dep_lib_index, modified_libs);
+    }
+  }
+}
+
+
+BitVector* IsolateReloadContext::FindModifiedLibraries(bool force_reload) {
+  Thread* thread = Thread::Current();
+  int64_t last_reload = I->reload_timestamp();
+
+  const GrowableObjectArray& libs =
+      GrowableObjectArray::Handle(object_store()->libraries());
+  Library& lib = Library::Handle();
+  Array& scripts = Array::Handle();
+  Script& script = Script::Handle();
+  intptr_t num_libs = libs.Length();
+
+  // Construct the imported-by graph.
+  ZoneGrowableArray<ZoneGrowableArray<intptr_t>* >* imported_by =
+      new ZoneGrowableArray<ZoneGrowableArray<intptr_t>* >(num_libs);
+  imported_by->SetLength(num_libs);
+  for (intptr_t i = 0; i < num_libs; i++) {
+    (*imported_by)[i] = new ZoneGrowableArray<intptr_t>();
+  }
+  Array& imports = Array::Handle();
+  Namespace& ns = Namespace::Handle();
+  Library& target = Library::Handle();
+
+  for (intptr_t lib_idx = 0; lib_idx < num_libs; lib_idx++) {
+    lib ^= libs.At(lib_idx);
+    ASSERT(lib_idx == lib.index());
+    if (lib.is_dart_scheme()) {
+      // We don't care about imports among dart scheme libraries.
+      continue;
+    }
+
+    // Add imports to the import-by graph.
+    imports = lib.imports();
+    for (intptr_t import_idx = 0; import_idx < imports.Length(); import_idx++) {
+      ns ^= imports.At(import_idx);
+      if (!ns.IsNull()) {
+        target = ns.library();
+        (*imported_by)[target.index()]->Add(lib.index());
+      }
+    }
+
+    // Add prefixed imports to the import-by graph.
+    DictionaryIterator entries(lib);
+    Object& entry = Object::Handle();
+    LibraryPrefix& prefix = LibraryPrefix::Handle();
+    while (entries.HasNext()) {
+      entry = entries.GetNext();
+      if (entry.IsLibraryPrefix()) {
+        prefix ^= entry.raw();
+        imports = prefix.imports();
+        for (intptr_t import_idx = 0; import_idx < imports.Length();
+             import_idx++) {
+          ns ^= imports.At(import_idx);
+          if (!ns.IsNull()) {
+            target = ns.library();
+            (*imported_by)[target.index()]->Add(lib.index());
+          }
+        }
+      }
+    }
+  }
+
+  BitVector* modified_libs = new(Z) BitVector(Z, num_libs);
+
+  for (intptr_t lib_idx = 0; lib_idx < num_libs; lib_idx++) {
+    lib ^= libs.At(lib_idx);
+    if (lib.is_dart_scheme() || modified_libs->Contains(lib_idx)) {
+      // We don't consider dart scheme libraries during reload.  If
+      // the modified libs set already contains this library, then we
+      // have already visited it.
+      continue;
+    }
+    scripts = lib.LoadedScripts();
+    for (intptr_t script_idx = 0; script_idx < scripts.Length(); script_idx++) {
+      script ^= scripts.At(script_idx);
+      if (force_reload) {
+        modified_libs->Add(lib_idx);
+        break;
+      }
+      if (ScriptModifiedSince(script, last_reload)) {

[Cutch, 2016/07/29 14:37:04]

maybe merge these two ifs:

if (force_reload || ScriptMOdifiedSince(script, last_reload))

[turnidge, 2016/07/29 17:37:37]

Done.

+        modified_libs->Add(lib_idx);
+        break;
+      }
+    }
+    if (modified_libs->Contains(lib_idx)) {
+      PropagateLibraryModified(imported_by, lib_idx, modified_libs);

[Cutch, 2016/07/29 14:37:04]

Could this be rolled into the if statement above?

if ... {
  modified_libs->Add(lib_idx);
  PropgateLibraryModified(imported_by, lib_idx, modified_libs);
}

[turnidge, 2016/07/29 17:37:37]

Done.

+    }
+  }
+
+  return modified_libs;
 }
 
 
@@ -462,19 +595,20 @@ void IsolateReloadContext::CheckpointLibraries() {
   num_saved_libs_ = 0;
   for (intptr_t i = 0; i < libs.Length(); i++) {
     lib ^= libs.At(i);
-    if (IsCleanLibrary(lib)) {
+    if (modified_libs_->Contains(i)) {
+      // We are going to reload this library. Clear the index.
+      lib.set_index(-1);
+    } else {
       // We are preserving this library across the reload, assign its new index
       lib.set_index(new_libs.Length());
       new_libs.Add(lib, Heap::kOld);
       num_saved_libs_++;
-    } else {
-      // We are going to reload this library. Clear the index.
-      lib.set_index(-1);
     }
     // Add old library to old libraries set.
     bool already_present = old_libraries_set.Insert(lib);
     ASSERT(!already_present);
   }
+  modified_libs_ = NULL;  // Renumbering the libraries has invalidated this.
   old_libraries_set_storage_ = old_libraries_set.Release().raw();
 
   // Reset the registered libraries to the filtered array.
@@ -1026,11 +1160,8 @@ void IsolateReloadContext::BuildLibraryMapping() {
 
   Library& replacement_or_new = Library::Handle();
   Library& old = Library::Handle();
-  for (intptr_t i = 0; i < libs.Length(); i++) {
+  for (intptr_t i = num_saved_libs_; i < libs.Length(); i++) {
     replacement_or_new = Library::RawCast(libs.At(i));
-    if (IsCleanLibrary(replacement_or_new)) {
-      continue;
-    }
     old ^= OldLibraryOrNull(replacement_or_new);
     if (old.IsNull()) {
       if (FLAG_identity_reload) {