Revert "[vm, compiler] Prune dictionaries to only contain elements needed at runtime."
This reverts commit 6de162e014ebbfeb098620456de96bb729ac269d.
Reason for revert: Local testing of a large Flutter app with this change causes the app to crash on startup
Original change's description:
> [vm, compiler] Prune dictionaries to only contain elements needed at runtime.
>
> flutter_gallery
> Isolate(CodeSize): 2116400 -> 1981238 (-6.28%)
> Total(CodeSize): 7217938 -> 7082600 (-1.87%)
>
> TEST=ci
> Bug: https://github.com/dart-lang/sdk/issues/48910
> Change-Id: I8cd285ddab3a611cd7a2a91d50414be402f8543a
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244303
> Reviewed-by: Siva Annamalai <asiva@google.com>
> Commit-Queue: Ryan Macnak <rmacnak@google.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: https://github.com/dart-lang/sdk/issues/48910
Change-Id: Ie51f004e84970907fa1233e8e7c3ed63e2da1c4c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246683
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Siva Annamalai <asiva@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
diff --git a/runtime/vm/app_snapshot.cc b/runtime/vm/app_snapshot.cc
index da71473..2b20b44 100644
--- a/runtime/vm/app_snapshot.cc
+++ b/runtime/vm/app_snapshot.cc
@@ -1873,12 +1873,12 @@
// in the current loading unit).
ObjectPoolPtr pool = code->untag()->object_pool_;
if (s->kind() == Snapshot::kFullAOT) {
- TracePool(s, pool, /*only_call_targets=*/is_deferred);
+ TracePool(s, pool, /*only_code=*/is_deferred);
} else {
if (s->InCurrentLoadingUnitOrRoot(pool)) {
s->Push(pool);
} else {
- TracePool(s, pool, /*only_call_targets=*/true);
+ TracePool(s, pool, /*only_code=*/true);
}
}
@@ -1946,7 +1946,7 @@
#endif
}
- void TracePool(Serializer* s, ObjectPoolPtr pool, bool only_call_targets) {
+ void TracePool(Serializer* s, ObjectPoolPtr pool, bool only_code) {
if (pool == ObjectPool::null()) {
return;
}
@@ -1957,18 +1957,8 @@
auto entry_type = ObjectPool::TypeBits::decode(entry_bits[i]);
if (entry_type == ObjectPool::EntryType::kTaggedObject) {
const ObjectPtr target = pool->untag()->data()[i].raw_obj_;
- // A field is a call target because its initializer may be called
- // indirectly by passing the field to the runtime. A const closure
- // is a call target because its function may be called indirectly
- // via a closure call.
- if (!only_call_targets || target->IsCode() || target->IsFunction() ||
- target->IsField() || target->IsClosure()) {
+ if (!only_code || target->IsCode()) {
s->Push(target);
- } else {
- intptr_t cid = target->GetClassIdMayBeSmi();
- if (cid >= kNumPredefinedCids) {
- s->Push(s->isolate_group()->class_table()->At(cid));
- }
}
}
}
diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc
index 6c6a023..ee450a6 100644
--- a/runtime/vm/compiler/aot/precompiler.cc
+++ b/runtime/vm/compiler/aot/precompiler.cc
@@ -424,7 +424,6 @@
typeparams_to_retain_(),
consts_to_retain_(),
seen_table_selectors_(),
- api_uses_(),
error_(Error::Handle()),
get_runtime_type_is_unique_(false) {
ASSERT(Precompiler::singleton_ == NULL);
@@ -663,8 +662,6 @@
ProgramVisitor::Dedup(T);
}
- PruneDictionaries();
-
if (retained_reasons_writer_ != nullptr) {
reasons_writer.Write();
retained_reasons_writer_ = nullptr;
@@ -755,7 +752,6 @@
}
}
if (!main.IsNull()) {
- AddApiUse(main);
if (lib.LookupLocalFunction(name) == Function::null()) {
retain_root_library_caches_ = true;
}
@@ -1468,14 +1464,6 @@
return seen_table_selectors_.HasKey(selector_id);
}
-void Precompiler::AddApiUse(const Object& obj) {
- api_uses_.Insert(&Object::ZoneHandle(Z, obj.ptr()));
-}
-
-bool Precompiler::HasApiUse(const Object& obj) {
- return api_uses_.HasKey(&obj);
-}
-
void Precompiler::AddInstantiatedClass(const Class& cls) {
if (is_tracing()) {
tracer_->WriteClassInstantiationRef(cls);
@@ -1535,7 +1523,6 @@
&reusable_object_handle) ==
EntryPointPragma::kAlways) {
AddInstantiatedClass(cls);
- AddApiUse(cls);
}
}
@@ -1554,7 +1541,6 @@
if (pragma == EntryPointPragma::kNever) continue;
AddField(field);
- AddApiUse(field);
if (!field.is_static()) {
if (pragma != EntryPointPragma::kSetterOnly) {
@@ -1583,7 +1569,6 @@
type == EntryPointPragma::kCallOnly) {
functions_with_entry_point_pragmas_.Insert(function);
AddFunction(function, RetainReasons::kEntryPointPragma);
- AddApiUse(function);
}
if ((type == EntryPointPragma::kAlways ||
@@ -1593,16 +1578,10 @@
function2 = function.ImplicitClosureFunction();
functions_with_entry_point_pragmas_.Insert(function2);
AddFunction(function2, RetainReasons::kEntryPointPragma);
-
- // Not `function2`: Dart_GetField will lookup the regular function
- // and get the implicit closure function from that.
- AddApiUse(function);
}
if (function.IsGenerativeConstructor()) {
AddInstantiatedClass(cls);
- AddApiUse(function);
- AddApiUse(cls);
}
}
if (function.kind() == UntaggedFunction::kImplicitGetter &&
@@ -1612,7 +1591,6 @@
if (function.accessor_field() == field.ptr()) {
functions_with_entry_point_pragmas_.Insert(function);
AddFunction(function, RetainReasons::kImplicitGetter);
- AddApiUse(function);
}
}
}
@@ -1623,7 +1601,6 @@
if (function.accessor_field() == field.ptr()) {
functions_with_entry_point_pragmas_.Insert(function);
AddFunction(function, RetainReasons::kImplicitSetter);
- AddApiUse(function);
}
}
}
@@ -1634,15 +1611,9 @@
if (function.accessor_field() == field.ptr()) {
functions_with_entry_point_pragmas_.Insert(function);
AddFunction(function, RetainReasons::kImplicitStaticGetter);
- AddApiUse(function);
}
}
}
- if (function.is_native()) {
- // The embedder will need to lookup this library to provide the native
- // resolver, even if there are no embedder calls into the library.
- AddApiUse(lib);
- }
}
implicit_getters = GrowableObjectArray::null();
@@ -3098,166 +3069,6 @@
}
}
-void Precompiler::PruneDictionaries() {
- // PRODUCT-only: pruning interferes with various uses of the service protocol,
- // including heap analysis tools.
-#if defined(PRODUCT)
- class PruneDictionariesVisitor {
- public:
- GrowableObjectArrayPtr PruneLibraries(
- const GrowableObjectArray& libraries) {
- for (intptr_t i = 0; i < libraries.Length(); i++) {
- lib_ ^= libraries.At(i);
- bool retain = PruneLibrary(lib_);
- if (retain) {
- lib_.set_index(retained_libraries_.Length());
- retained_libraries_.Add(lib_);
- } else {
- lib_.set_index(-1);
- lib_.set_private_key(null_string_);
- }
- }
-
- Library::RegisterLibraries(Thread::Current(), retained_libraries_);
- return retained_libraries_.ptr();
- }
-
- bool PruneLibrary(const Library& lib) {
- dict_ = lib.dictionary();
- intptr_t dict_size = dict_.Length() - 1;
- intptr_t used = 0;
- for (intptr_t i = 0; i < dict_size; i++) {
- entry_ = dict_.At(i);
- if (entry_.IsNull()) continue;
-
- bool retain = false;
- if (entry_.IsClass()) {
- // dart:async: Fix async stack trace lookups in dart:async to annotate
- // entry points or fail gracefully.
- // dart:core, dart:collection, dart:typed_data: Isolate messaging
- // between groups allows any class in these libraries.
- retain = PruneClass(Class::Cast(entry_)) ||
- (lib.url() == Symbols::DartAsync().ptr()) ||
- (lib.url() == Symbols::DartCore().ptr()) ||
- (lib.url() == Symbols::DartCollection().ptr()) ||
- (lib.url() == Symbols::DartTypedData().ptr());
- } else if (entry_.IsFunction() || entry_.IsField()) {
- retain = precompiler_->HasApiUse(entry_);
- } else {
- FATAL("Unexpected library entry: %s", entry_.ToCString());
- }
- if (retain) {
- used++;
- } else {
- dict_.SetAt(i, Object::null_object());
- }
- }
- lib.RehashDictionary(dict_, used * 4 / 3 + 1);
-
- bool retain = used > 0;
- cls_ = lib.toplevel_class();
- if (PruneClass(cls_)) {
- retain = true;
- }
- if (lib.is_dart_scheme()) {
- retain = true;
- }
- if (lib.ptr() == root_lib_.ptr()) {
- retain = true;
- }
- if (precompiler_->HasApiUse(lib)) {
- retain = true;
- }
- return retain;
- }
-
- bool PruneClass(const Class& cls) {
- bool retain = precompiler_->HasApiUse(cls);
-
- functions_ = cls.functions();
- retained_functions_ = GrowableObjectArray::New();
- for (intptr_t i = 0; i < functions_.Length(); i++) {
- function_ ^= functions_.At(i);
- if (precompiler_->HasApiUse(function_)) {
- retained_functions_.Add(function_);
- retain = true;
- } else if (precompiler_->functions_called_dynamically_.ContainsKey(
- function_)) {
- retained_functions_.Add(function_);
- // No `retain = true`: the function must appear in the method
- // dictionary for lookup, but the class may still be removed from the
- // library.
- }
- }
- if (retained_functions_.Length() > 0) {
- functions_ = Array::MakeFixedLength(retained_functions_);
- cls.SetFunctions(functions_);
- } else {
- cls.SetFunctions(Object::empty_array());
- }
-
- fields_ = cls.fields();
- retained_fields_ = GrowableObjectArray::New();
- for (intptr_t i = 0; i < fields_.Length(); i++) {
- field_ ^= fields_.At(i);
- if (precompiler_->HasApiUse(field_)) {
- retained_fields_.Add(field_);
- retain = true;
- }
- }
- if (retained_fields_.Length() > 0) {
- fields_ = Array::MakeFixedLength(retained_fields_);
- cls.SetFields(fields_);
- } else {
- cls.SetFields(Object::empty_array());
- }
-
- return retain;
- }
-
- explicit PruneDictionariesVisitor(Precompiler* precompiler, Zone* zone)
- : precompiler_(precompiler),
- lib_(Library::Handle(zone)),
- dict_(Array::Handle(zone)),
- entry_(Object::Handle(zone)),
- cls_(Class::Handle(zone)),
- functions_(Array::Handle(zone)),
- fields_(Array::Handle(zone)),
- function_(Function::Handle(zone)),
- field_(Field::Handle(zone)),
- retained_functions_(GrowableObjectArray::Handle(zone)),
- retained_fields_(GrowableObjectArray::Handle(zone)),
- retained_libraries_(
- GrowableObjectArray::Handle(zone, GrowableObjectArray::New())),
- root_lib_(Library::Handle(
- zone,
- precompiler->isolate_group()->object_store()->root_library())),
- null_string_(String::Handle(zone)) {}
-
- private:
- Precompiler* const precompiler_;
- Library& lib_;
- Array& dict_;
- Object& entry_;
- Class& cls_;
- Array& functions_;
- Array& fields_;
- Function& function_;
- Field& field_;
- GrowableObjectArray& retained_functions_;
- GrowableObjectArray& retained_fields_;
- const GrowableObjectArray& retained_libraries_;
- const Library& root_lib_;
- const String& null_string_;
- };
-
- HANDLESCOPE(T);
- SafepointWriteRwLocker ml(T, T->isolate_group()->program_lock());
- PruneDictionariesVisitor visitor(this, Z);
- libraries_ = visitor.PruneLibraries(libraries_);
-#endif // defined(PRODUCT)
-}
-
// Traits for the HashTable template.
struct CodeKeyTraits {
static uint32_t Hash(const Object& key) { return Code::Cast(key).Size(); }
diff --git a/runtime/vm/compiler/aot/precompiler.h b/runtime/vm/compiler/aot/precompiler.h
index 50df9e7..4e0d211 100644
--- a/runtime/vm/compiler/aot/precompiler.h
+++ b/runtime/vm/compiler/aot/precompiler.h
@@ -210,37 +210,6 @@
typedef DirectChainedHashMap<TypeArgumentsKeyValueTrait> TypeArgumentsSet;
-class ProgramElementKeyValueTrait {
- public:
- // Typedefs needed for the DirectChainedHashMap template.
- typedef const Object* Key;
- typedef const Object* Value;
- typedef const Object* Pair;
-
- static Key KeyOf(Pair kv) { return kv; }
-
- static Value ValueOf(Pair kv) { return kv; }
-
- static inline uword Hash(Key key) {
- if (key->IsFunction()) {
- return Function::Cast(*key).Hash();
- } else if (key->IsField()) {
- return Field::Cast(*key).kernel_offset();
- } else if (key->IsClass()) {
- return Class::Cast(*key).kernel_offset();
- } else if (key->IsLibrary()) {
- return Library::Cast(*key).index();
- }
- FATAL("Unexpected type: %s\n", key->ToCString());
- }
-
- static inline bool IsKeyEqual(Pair pair, Key key) {
- return pair->ptr() == key->ptr();
- }
-};
-
-typedef DirectChainedHashMap<ProgramElementKeyValueTrait> ProgramElementSet;
-
class InstanceKeyValueTrait {
public:
// Typedefs needed for the DirectChainedHashMap template.
@@ -352,8 +321,6 @@
bool IsHitByTableSelector(const Function& function);
// Returns the reason if the function must be retained, otherwise nullptr.
const char* MustRetainFunction(const Function& function);
- void AddApiUse(const Object& obj);
- bool HasApiUse(const Object& obj);
void ProcessFunction(const Function& function);
void CheckForNewDynamicFunctions();
@@ -376,7 +343,6 @@
void DropClasses();
void DropLibraries();
void DiscardCodeObjects();
- void PruneDictionaries();
DEBUG_ONLY(FunctionPtr FindUnvisitedRetainedFunction());
@@ -427,7 +393,6 @@
TypeParameterSet typeparams_to_retain_;
InstanceSet consts_to_retain_;
TableSelectorSet seen_table_selectors_;
- ProgramElementSet api_uses_;
Error& error_;
compiler::DispatchTableGenerator* dispatch_table_generator_;
diff --git a/runtime/vm/compiler/frontend/kernel_translation_helper.cc b/runtime/vm/compiler/frontend/kernel_translation_helper.cc
index 7dba48a..d52cc58 100644
--- a/runtime/vm/compiler/frontend/kernel_translation_helper.cc
+++ b/runtime/vm/compiler/frontend/kernel_translation_helper.cc
@@ -1891,11 +1891,9 @@
Zone* zone = thread->zone();
intptr_t unit_count = helper_->ReadUInt();
Array& loading_units = Array::Handle(zone, Array::New(unit_count + 1));
- Array& loading_unit_uris = Array::Handle(zone, Array::New(unit_count + 1));
LoadingUnit& unit = LoadingUnit::Handle(zone);
LoadingUnit& parent = LoadingUnit::Handle(zone);
Library& lib = Library::Handle(zone);
- Array& uris = Array::Handle(zone);
for (int i = 0; i < unit_count; i++) {
intptr_t id = helper_->ReadUInt();
@@ -1909,7 +1907,6 @@
unit.set_parent(parent);
intptr_t library_count = helper_->ReadUInt();
- uris = Array::New(library_count);
for (intptr_t j = 0; j < library_count; j++) {
const String& uri =
translation_helper_.DartSymbolPlain(helper_->ReadStringReference());
@@ -1918,18 +1915,14 @@
FATAL1("Missing library: %s\n", uri.ToCString());
}
lib.set_loading_unit(unit);
- uris.SetAt(j, uri);
}
loading_units.SetAt(id, unit);
- loading_unit_uris.SetAt(id, uris);
}
ObjectStore* object_store = IG->object_store();
ASSERT(object_store->loading_units() == Array::null());
object_store->set_loading_units(loading_units);
- ASSERT(object_store->loading_unit_uris() == Array::null());
- object_store->set_loading_unit_uris(loading_unit_uris);
}
CallSiteAttributesMetadataHelper::CallSiteAttributesMetadataHelper(
diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc
index f8b05d2..4f03fb7a 100644
--- a/runtime/vm/dart_api_impl.cc
+++ b/runtime/vm/dart_api_impl.cc
@@ -6728,12 +6728,24 @@
DARTSCOPE(Thread::Current());
API_TIMELINE_DURATION(T);
- const Array& loading_units =
- Array::Handle(Z, T->isolate_group()->object_store()->loading_unit_uris());
- if (loading_unit_id >= 0 && loading_unit_id < loading_units.Length()) {
- return Api::NewHandle(T, loading_units.At(loading_unit_id));
+ const GrowableObjectArray& result =
+ GrowableObjectArray::Handle(Z, GrowableObjectArray::New());
+ const GrowableObjectArray& libs = GrowableObjectArray::Handle(
+ Z, T->isolate_group()->object_store()->libraries());
+ Library& lib = Library::Handle(Z);
+ LoadingUnit& unit = LoadingUnit::Handle(Z);
+ String& uri = String::Handle(Z);
+ for (intptr_t i = 0; i < libs.Length(); i++) {
+ lib ^= libs.At(i);
+ unit = lib.loading_unit();
+ if (unit.IsNull() || (unit.id() != loading_unit_id)) {
+ continue;
+ }
+ uri = lib.url();
+ result.Add(uri);
}
- return Api::NewError("Invalid loading_unit_id");
+
+ return Api::NewHandle(T, Array::MakeFixedLength(result));
#endif
}
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index 9e867c4..46d3e1b 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -12448,12 +12448,8 @@
untag()->set_name(name.ptr());
}
-void Library::set_url(const String& url) const {
- untag()->set_url(url.ptr());
-}
-
-void Library::set_private_key(const String& key) const {
- untag()->set_private_key(key.ptr());
+void Library::set_url(const String& name) const {
+ untag()->set_url(name.ptr());
}
void Library::set_kernel_data(const ExternalTypedData& data) const {
diff --git a/runtime/vm/object.h b/runtime/vm/object.h
index dd1699a..6f226c3 100644
--- a/runtime/vm/object.h
+++ b/runtime/vm/object.h
@@ -5109,7 +5109,6 @@
// the name and url.
void set_name(const String& name) const;
void set_url(const String& url) const;
- void set_private_key(const String& key) const;
void set_num_imports(intptr_t value) const;
void set_flags(uint8_t flags) const;
diff --git a/runtime/vm/object_store.h b/runtime/vm/object_store.h
index 718e61d..674d1b4 100644
--- a/runtime/vm/object_store.h
+++ b/runtime/vm/object_store.h
@@ -261,7 +261,6 @@
RW(Array, dispatch_table_code_entries) \
RW(GrowableObjectArray, instructions_tables) \
RW(Array, obfuscation_map) \
- RW(Array, loading_unit_uris) \
RW(GrowableObjectArray, ffi_callback_functions) \
RW(Class, ffi_pointer_class) \
RW(Class, ffi_native_type_class) \
diff --git a/sdk/lib/_internal/vm/bin/socket_patch.dart b/sdk/lib/_internal/vm/bin/socket_patch.dart
index a70896e..d0bddf8 100644
--- a/sdk/lib/_internal/vm/bin/socket_patch.dart
+++ b/sdk/lib/_internal/vm/bin/socket_patch.dart
@@ -2543,7 +2543,6 @@
}
@patch
-@pragma("vm:entry-point")
class ResourceHandle {
factory ResourceHandle.fromFile(RandomAccessFile file) {
int fd = (file as _RandomAccessFile).fd;
