Precompilation: when removing top-level fields and functions, also remove them from the library dic…

runtime/vm/precompiler.cc

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/precompiler.h"
 
 #include "vm/cha.h"
 #include "vm/code_patcher.h"
 #include "vm/compiler.h"
 #include "vm/hash_table.h"
@@ skipping 51 matching lines @@
     function_count_(0),
     class_count_(0),
     selector_count_(0),
     dropped_function_count_(0),
     dropped_field_count_(0),
     libraries_(GrowableObjectArray::Handle(I->object_store()->libraries())),
     pending_functions_(
         GrowableObjectArray::Handle(GrowableObjectArray::New())),
     sent_selectors_(),
     enqueued_functions_(),
+    fields_to_retain_(),
     error_(Error::Handle()) {
 }
 
 
 void Precompiler::DoCompileAll(
     Dart_QualifiedFunctionName embedder_entry_points[]) {
   ASSERT(I->compilation_allowed());
 
   {
     StackZone stack_zone(T);
@@ skipping 29 matching lines @@
       // Start with the allocations and invocations that happen from C++.
       AddRoots(embedder_entry_points);
 
       // Compile newly found targets and add their callees until we reach a
       // fixed point.
       Iterate();
     }
 
     I->set_compilation_allowed(false);
 
-    DropUncompiledFunctions();
+    DropFunctions();
     DropFields();
 
     // TODO(rmacnak): DropEmptyClasses();
 
     BindStaticCalls();
 
     DedupStackmaps();
     DedupStackmapLists();
 
     I->object_store()->set_compile_time_constants(Array::null_array());
@@ skipping 758 matching lines @@
                                          Object* function) {
   UniqueFunctionsSet functions_set(
       isolate->object_store()->unique_dynamic_targets());
   ASSERT(fname.IsSymbol());
   *function = functions_set.GetOrNull(fname);
   ASSERT(functions_set.Release().raw() ==
       isolate->object_store()->unique_dynamic_targets());
 }
 
 
-void Precompiler::DropUncompiledFunctions() {
+void Precompiler::DropFunctions() {
   Library& lib = Library::Handle(Z);
   Class& cls = Class::Handle(Z);
   Array& functions = Array::Handle(Z);
   Function& function = Function::Handle(Z);
   Function& function2 = Function::Handle(Z);
   GrowableObjectArray& retained_functions = GrowableObjectArray::Handle(Z);
   GrowableObjectArray& closures = GrowableObjectArray::Handle(Z);
+  String& name = String::Handle(Z);
 
   for (intptr_t i = 0; i < libraries_.Length(); i++) {
     lib ^= libraries_.At(i);
     ClassDictionaryIterator it(lib, ClassDictionaryIterator::kIteratePrivate);
     while (it.HasNext()) {
       cls = it.GetNextClass();
       if (cls.IsDynamicClass()) {
         continue;  // class 'dynamic' is in the read-only VM isolate.
       }
 
+      bool top_level = cls.IsTopLevel();

[Florian Schneider, 2016/02/02 21:30:20]

Move this right before its use?

[rmacnak, 2016/02/03 01:43:39]

Done.

+
       functions = cls.functions();
       retained_functions = GrowableObjectArray::New();
       for (intptr_t j = 0; j < functions.Length(); j++) {
         function ^= functions.At(j);
         bool retain = function.HasCode();
         if (!retain && function.HasImplicitClosureFunction()) {
           // It can happen that all uses of an implicit closure inline their
           // target function, leaving the target function uncompiled. Keep
           // the target function anyway so we can enumerate it to bind its
           // static calls, etc.
           function2 = function.ImplicitClosureFunction();
           retain = function2.HasCode();
         }
         if (retain) {
           retained_functions.Add(function);
           function.DropUncompiledImplicitClosureFunction();
         } else {
+          if (top_level &&
+              (function.kind() != RawFunction::kImplicitStaticFinalGetter)) {

[Florian Schneider, 2016/02/02 21:30:20]

Why not for implicit static final getters?

[rmacnak, 2016/02/03 01:43:40]

They aren't in the library dictionary in the first place, so the assert that we
removed something fails. Added comment to this effect.

+            name = function.DictionaryName();
+            bool removed = lib.RemoveObject(function, name);
+            ASSERT(removed);
+          }
           dropped_function_count_++;
           if (FLAG_trace_precompiler) {
             THR_Print("Precompilation dropping %s\n",
                       function.ToLibNamePrefixedQualifiedCString());
           }
         }
       }
 
       if (retained_functions.Length() > 0) {
         functions = Array::MakeArray(retained_functions);
@@ skipping 21 matching lines @@
   isolate()->object_store()->set_closure_functions(retained_functions);
 }
 
 
 void Precompiler::DropFields() {
   Library& lib = Library::Handle(Z);
   Class& cls = Class::Handle(Z);
   Array& fields = Array::Handle(Z);
   Field& field = Field::Handle(Z);
   GrowableObjectArray& retained_fields = GrowableObjectArray::Handle(Z);
+  String& name = String::Handle(Z);
 
   for (intptr_t i = 0; i < libraries_.Length(); i++) {
     lib ^= libraries_.At(i);
     ClassDictionaryIterator it(lib, ClassDictionaryIterator::kIteratePrivate);
     while (it.HasNext()) {
       cls = it.GetNextClass();
       if (cls.IsDynamicClass()) {
         continue;  // class 'dynamic' is in the read-only VM isolate.
       }
 
+      bool top_level = cls.IsTopLevel();

[Florian Schneider, 2016/02/02 21:30:19]

Move this right before its use?

[rmacnak, 2016/02/03 01:43:39]

Done.

+
       fields = cls.fields();
       retained_fields = GrowableObjectArray::New();
       for (intptr_t j = 0; j < fields.Length(); j++) {
         field ^= fields.At(j);
-        bool drop = fields_to_retain_.Lookup(&field) == NULL;
-        if (drop) {
+        bool retain = fields_to_retain_.Lookup(&field) != NULL;
+        if (retain) {
+          retained_fields.Add(field);
+        } else {
+          if (top_level) {
+            name = field.DictionaryName();
+            bool removed = lib.RemoveObject(field, name);
+            ASSERT(removed);
+          }
           dropped_field_count_++;
           if (FLAG_trace_precompiler) {
             THR_Print("Precompilation dropping %s\n",
                       field.ToCString());
           }
-        } else {
-          retained_fields.Add(field);
         }
       }
 
       if (retained_fields.Length() > 0) {
         fields = Array::MakeArray(retained_fields);
         cls.SetFields(fields);
       } else {
         cls.SetFields(Object::empty_array());
       }
     }
@@ skipping 236 matching lines @@
   I->set_all_classes_finalized(true);
 }
 
 
 void Precompiler::ResetPrecompilerState() {
   changed_ = false;
   function_count_ = 0;
   class_count_ = 0;
   selector_count_ = 0;
   dropped_function_count_ = 0;
+  dropped_field_count_ = 0;
   ASSERT(pending_functions_.Length() == 0);
   sent_selectors_.Clear();
   enqueued_functions_.Clear();
 
   Library& lib = Library::Handle(Z);
   Class& cls = Class::Handle(Z);
 
   for (intptr_t i = 0; i < libraries_.Length(); i++) {
     lib ^= libraries_.At(i);
     ClassDictionaryIterator it(lib, ClassDictionaryIterator::kIteratePrivate);
     while (it.HasNext()) {
       cls = it.GetNextClass();
       if (cls.IsDynamicClass()) {
         continue;  // class 'dynamic' is in the read-only VM isolate.
       }
       cls.set_is_allocated(false);
     }
   }
 }
 
 }  // namespace dart