Skip to content

Commit

Permalink
[vm] Remove dead --load_deferred_eagerly and dependent code tracking …
Browse files Browse the repository at this point in the history
…for library prefixes.

Since Dart 2, library prefixes are always loaded eagerly on the VM.

Also remove reload check for deferred prefixes. This check was added to avoid behavior of library prefixes giving spurious "not loaded" errors after a reload, but now this error can never occur. Resolves a difference in reload between AST and bytecode modes, since bytecode mode isn't surfacing the deferred property.

Change-Id: Ide5fb6cac2efc90ca1b108a35bc09d342cbd60de
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/116051
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
  • Loading branch information
rmacnak-google committed Sep 6, 2019
1 parent 1d6ab52 commit 84db163
Show file tree
Hide file tree
Showing 26 changed files with 7 additions and 555 deletions.
5 changes: 0 additions & 5 deletions runtime/bin/gen_snapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -864,11 +864,6 @@ int main(int argc, char** argv) {
TimerUtils::InitOnce();
EventHandler::Start();

#if !defined(PRODUCT)
// Constant true in PRODUCT mode.
vm_options.AddArgument("--load_deferred_eagerly");
#endif

if (IsSnapshottingForPrecompilation()) {
vm_options.AddArgument("--precompilation");
} else if ((snapshot_kind == kCoreJIT) || (snapshot_kind == kAppJIT)) {
Expand Down
7 changes: 0 additions & 7 deletions runtime/bin/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1139,13 +1139,6 @@ void main(int argc, char** argv) {
&app_isolate_snapshot_instructions);
}

#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
// Constant true if PRODUCT or DART_PRECOMPILED_RUNTIME.
if ((Options::gen_snapshot_kind() != kNone) || vm_run_app_snapshot) {
vm_options.AddArgument("--load_deferred_eagerly");
}
#endif

if (Options::gen_snapshot_kind() == kAppJIT) {
vm_options.AddArgument("--fields_may_be_reset");
}
Expand Down
1 change: 0 additions & 1 deletion runtime/include/dart_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -3456,7 +3456,6 @@ DART_EXPORT DART_WARN_UNUSED_RESULT Dart_Handle Dart_SortClasses();
* current VM. The instructions piece must be loaded with read and execute
* permissions; the data piece may be loaded as read-only.
*
* - Requires the VM to have been started with --load-deferred-eagerly.
* - Requires the VM to have not been started with --precompilation.
* - Not supported when targeting IA32 or DBC.
* - The VM writing the snapshot and the VM reading the snapshot must be the
Expand Down
71 changes: 2 additions & 69 deletions runtime/lib/lib_prefix.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,74 +11,7 @@ class _LibraryPrefix {
throw "Unreachable";
}

bool _load() native "LibraryPrefix_load";
Object _loadError() native "LibraryPrefix_loadError";
bool isLoaded() native "LibraryPrefix_isLoaded";
bool _invalidateDependentCode()
native "LibraryPrefix_invalidateDependentCode";
bool isLoaded() => true;

loadLibrary() {
for (int i = 0; i < _outstandingLoadRequests.length; i++) {
if (_outstandingLoadRequests[i][0] == this) {
return _outstandingLoadRequests[i][1].future;
}
}

var completer = new Completer<bool>();
var pair = new List();
pair.add(this);
pair.add(completer);
_outstandingLoadRequests.add(pair);
Timer.run(() {
var hasCompleted = this._load();
// Loading can complete immediately, for example when the same
// library has been loaded eagerly or through another deferred
// prefix. If that is the case, we must invalidate the dependent
// code and complete the future now since there will be no callback
// from the VM.
if (hasCompleted && !completer.isCompleted) {
_invalidateDependentCode();
completer.complete(true);
_outstandingLoadRequests.remove(pair);
}
});
return completer.future;
}
}

// A list of two element lists. The first element is the _LibraryPrefix. The
// second element is the Completer for the load request.
var _outstandingLoadRequests = new List<List>();

// Called from the VM when an outstanding load request has finished.
@pragma("vm:entry-point", "call")
_completeDeferredLoads() {
// Determine which outstanding load requests have completed and complete
// their completer (with an error or true). For outstanding load requests
// which have not completed, remember them for next time in
// stillOutstandingLoadRequests.
var stillOutstandingLoadRequests = new List<List>();

// Make a copy of the outstandingRequests because the call to _load below
// may recursively trigger another call to |_completeDeferredLoads|, which
// can cause |_outstandingLoadRequests| to be modified.
var outstandingRequests = _outstandingLoadRequests.toList();
for (int i = 0; i < outstandingRequests.length; i++) {
var prefix = outstandingRequests[i][0];
var completer = outstandingRequests[i][1];
var error = prefix._loadError();
if (completer.isCompleted) {
// Already completed. Skip.
continue;
}
if (error != null) {
completer.completeError(error);
} else if (prefix._load()) {
prefix._invalidateDependentCode();
completer.complete(true);
} else {
stillOutstandingLoadRequests.add(outstandingRequests[i]);
}
}
_outstandingLoadRequests = stillOutstandingLoadRequests;
loadLibrary() => new Future.value(true);
}
31 changes: 0 additions & 31 deletions runtime/lib/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -184,37 +184,6 @@ DEFINE_NATIVE_ENTRY(Type_getHashCode, 0, 1) {
return Smi::New(hash_val);
}

DEFINE_NATIVE_ENTRY(LibraryPrefix_invalidateDependentCode, 0, 1) {
const LibraryPrefix& prefix =
LibraryPrefix::CheckedHandle(zone, arguments->NativeArgAt(0));
prefix.InvalidateDependentCode();
return Bool::Get(true).raw();
}

DEFINE_NATIVE_ENTRY(LibraryPrefix_load, 0, 1) {
const LibraryPrefix& prefix =
LibraryPrefix::CheckedHandle(zone, arguments->NativeArgAt(0));
bool hasCompleted = prefix.LoadLibrary();
return Bool::Get(hasCompleted).raw();
}

DEFINE_NATIVE_ENTRY(LibraryPrefix_loadError, 0, 1) {
const LibraryPrefix& prefix =
LibraryPrefix::CheckedHandle(zone, arguments->NativeArgAt(0));
// Currently all errors are Dart instances, e.g. I/O errors
// created by deferred loading code. LanguageErrors from
// failed loading or finalization attempts are propagated and result
// in the isolate's death.
const Instance& error = Instance::Handle(zone, prefix.LoadError());
return error.raw();
}

DEFINE_NATIVE_ENTRY(LibraryPrefix_isLoaded, 0, 1) {
const LibraryPrefix& prefix =
LibraryPrefix::CheckedHandle(zone, arguments->NativeArgAt(0));
return Bool::Get(prefix.is_loaded()).raw();
}

DEFINE_NATIVE_ENTRY(Internal_inquireIs64Bit, 0, 0) {
#if defined(ARCH_IS_64_BIT)
return Bool::True().raw();
Expand Down
4 changes: 0 additions & 4 deletions runtime/vm/bootstrap_natives.h
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,6 @@ namespace dart {
V(WeakProperty_getValue, 1) \
V(WeakProperty_setValue, 2) \
V(Uri_isWindowsPlatform, 0) \
V(LibraryPrefix_load, 1) \
V(LibraryPrefix_invalidateDependentCode, 1) \
V(LibraryPrefix_loadError, 1) \
V(LibraryPrefix_isLoaded, 1) \
V(UserTag_new, 2) \
V(UserTag_label, 1) \
V(UserTag_defaultTag, 0) \
Expand Down
1 change: 0 additions & 1 deletion runtime/vm/clustered_snapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2846,7 +2846,6 @@ class LibraryPrefixDeserializationCluster : public DeserializationCluster {
ReadFromTo(prefix);
prefix->ptr()->num_imports_ = d->Read<uint16_t>();
prefix->ptr()->is_deferred_load_ = d->Read<bool>();
prefix->ptr()->is_loaded_ = !prefix->ptr()->is_deferred_load_;
}
}
};
Expand Down
2 changes: 0 additions & 2 deletions runtime/vm/compiler/aot/precompiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2227,8 +2227,6 @@ void PrecompileParsedFunctionHelper::FinalizeCompilation(
function.set_unoptimized_code(code);
function.AttachCode(code);
}
ASSERT(!parsed_function()->HasDeferredPrefixes());
ASSERT(FLAG_load_deferred_eagerly);
}

// Return false if bailed out.
Expand Down
15 changes: 0 additions & 15 deletions runtime/vm/compiler/backend/flow_graph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ FlowGraph::FlowGraph(const ParsedFunction& parsed_function,
prologue_info_(prologue_info),
loop_hierarchy_(nullptr),
loop_invariant_loads_(nullptr),
deferred_prefixes_(parsed_function.deferred_prefixes()),
captured_parameters_(new (zone()) BitVector(zone(), variable_count())),
inlining_id_(-1),
should_print_(FlowGraphPrinter::ShouldPrint(parsed_function.function())) {
Expand Down Expand Up @@ -105,20 +104,6 @@ void FlowGraph::ReplaceCurrentInstruction(ForwardInstructionIterator* iterator,
iterator->RemoveCurrentFromGraph();
}

void FlowGraph::AddToDeferredPrefixes(
ZoneGrowableArray<const LibraryPrefix*>* from) {
ZoneGrowableArray<const LibraryPrefix*>* to = deferred_prefixes();
for (intptr_t i = 0; i < from->length(); i++) {
const LibraryPrefix* prefix = (*from)[i];
for (intptr_t j = 0; j < to->length(); j++) {
if ((*to)[j]->raw() == prefix->raw()) {
return;
}
}
to->Add(prefix);
}
}

bool FlowGraph::ShouldReorderBlocks(const Function& function,
bool is_optimized) {
return is_optimized && FLAG_reorder_basic_blocks &&
Expand Down
7 changes: 0 additions & 7 deletions runtime/vm/compiler/backend/flow_graph.h
Original file line number Diff line number Diff line change
Expand Up @@ -341,12 +341,6 @@ class FlowGraph : public ZoneAllocated {

bool IsCompiledForOsr() const { return graph_entry()->IsCompiledForOsr(); }

void AddToDeferredPrefixes(ZoneGrowableArray<const LibraryPrefix*>* from);

ZoneGrowableArray<const LibraryPrefix*>* deferred_prefixes() const {
return deferred_prefixes_;
}

BitVector* captured_parameters() const { return captured_parameters_; }

intptr_t inlining_id() const { return inlining_id_; }
Expand Down Expand Up @@ -532,7 +526,6 @@ class FlowGraph : public ZoneAllocated {
LoopHierarchy* loop_hierarchy_;
ZoneGrowableArray<BitVector*>* loop_invariant_loads_;

ZoneGrowableArray<const LibraryPrefix*>* deferred_prefixes_;
DirectChainedHashMap<ConstantPoolTrait> constant_instr_pool_;
BitVector* captured_parameters_;

Expand Down
4 changes: 0 additions & 4 deletions runtime/vm/compiler/backend/inliner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1240,10 +1240,6 @@ class CallSiteInliner : public ValueObject {
caller_graph()->parsed_function().AddToGuardedFields(
callee_guarded_fields[i]);
}
// When inlined, we add the deferred prefixes of the callee to the
// caller's list of deferred prefixes.
caller_graph()->AddToDeferredPrefixes(
callee_graph->deferred_prefixes());

FlowGraphInliner::SetInliningId(
callee_graph,
Expand Down
9 changes: 0 additions & 9 deletions runtime/vm/compiler/jit/compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ static void PrecompilationModeHandler(bool value) {
// These flags are constants with PRODUCT and DART_PRECOMPILED_RUNTIME.
FLAG_deoptimize_alot = false; // Used in some tests.
FLAG_deoptimize_every = 0; // Used in some tests.
FLAG_load_deferred_eagerly = true;
FLAG_use_osr = false;
#endif
}
Expand Down Expand Up @@ -491,14 +490,6 @@ RawCode* CompileParsedFunctionHelper::FinalizeCompilation(
function.SetUsageCounter(0);
}
}
if (parsed_function()->HasDeferredPrefixes()) {
ASSERT(!FLAG_load_deferred_eagerly);
ZoneGrowableArray<const LibraryPrefix*>* prefixes =
parsed_function()->deferred_prefixes();
for (intptr_t i = 0; i < prefixes->length(); i++) {
(*prefixes)[i]->RegisterDependentCode(code);
}
}
return code.raw();
}

Expand Down
43 changes: 0 additions & 43 deletions runtime/vm/dart_api_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1643,10 +1643,6 @@ Dart_CreateSnapshot(uint8_t** vm_snapshot_data_buffer,
DARTSCOPE(Thread::Current());
API_TIMELINE_DURATION(T);
Isolate* I = T->isolate();
if (!FLAG_load_deferred_eagerly) {
return Api::NewError(
"Creating full snapshots requires --load_deferred_eagerly");
}
if (vm_snapshot_data_buffer != NULL && vm_snapshot_data_size == NULL) {
RETURN_NULL_ERROR(vm_snapshot_data_size);
}
Expand Down Expand Up @@ -5403,7 +5399,6 @@ DART_EXPORT Dart_Handle Dart_LookupLibrary(Dart_Handle url) {
DART_EXPORT Dart_Handle Dart_LibraryHandleError(Dart_Handle library_in,
Dart_Handle error_in) {
DARTSCOPE(Thread::Current());
Isolate* I = T->isolate();

const Library& lib = Api::UnwrapLibraryHandle(Z, library_in);
if (lib.IsNull()) {
Expand All @@ -5415,15 +5410,6 @@ DART_EXPORT Dart_Handle Dart_LibraryHandleError(Dart_Handle library_in,
}
CHECK_CALLBACK_STATE(T);

const GrowableObjectArray& pending_deferred_loads =
GrowableObjectArray::Handle(Z,
I->object_store()->pending_deferred_loads());
for (intptr_t i = 0; i < pending_deferred_loads.Length(); i++) {
if (pending_deferred_loads.At(i) == lib.raw()) {
lib.SetLoadError(err);
return Api::Null();
}
}
return error_in;
}

Expand Down Expand Up @@ -5514,9 +5500,6 @@ DART_EXPORT Dart_Handle Dart_FinalizeLoading(bool complete_futures) {
// instead of freelists.
BumpAllocateScope bump_allocate_scope(T);

// TODO(hausner): move the remaining code below (finalization and
// invoking of _completeDeferredLoads) into Isolate::DoneLoading().

// Finalize all classes if needed.
Dart_Handle state = Api::CheckAndFinalizePendingClasses(T);
if (Api::IsError(state)) {
Expand Down Expand Up @@ -5545,22 +5528,6 @@ DART_EXPORT Dart_Handle Dart_FinalizeLoading(bool complete_futures) {
}
#endif

if (complete_futures) {
const Library& corelib = Library::Handle(Z, Library::CoreLibrary());
const String& function_name =
String::Handle(Z, String::New("_completeDeferredLoads"));
const Function& function =
Function::Handle(Z, corelib.LookupFunctionAllowPrivate(function_name));
ASSERT(!function.IsNull());
const Array& args = Array::empty_array();

const Object& res =
Object::Handle(Z, DartEntry::InvokeFunction(function, args));
I->object_store()->clear_pending_deferred_loads();
if (res.IsError() || res.IsUnhandledException()) {
return Api::NewHandle(T, res.raw());
}
}
return Api::Success();
}

Expand Down Expand Up @@ -6171,7 +6138,6 @@ Dart_CreateAppAOTSnapshotAsAssembly(Dart_StreamingWriteCallback callback,
"Isolate is not precompiled. "
"Did you forget to call Dart_Precompile?");
}
ASSERT(FLAG_load_deferred_eagerly);
CHECK_NULL(callback);

TIMELINE_DURATION(T, Isolate, "WriteAppAOTSnapshot");
Expand Down Expand Up @@ -6307,7 +6273,6 @@ Dart_CreateAppAOTSnapshotAsBlobs(uint8_t** vm_snapshot_data_buffer,
"Isolate is not precompiled. "
"Did you forget to call Dart_Precompile?");
}
ASSERT(FLAG_load_deferred_eagerly);
CHECK_NULL(vm_snapshot_data_buffer);
CHECK_NULL(vm_snapshot_data_size);
CHECK_NULL(vm_snapshot_instructions_buffer);
Expand Down Expand Up @@ -6411,10 +6376,6 @@ DART_EXPORT Dart_Handle Dart_CreateCoreJITSnapshotAsBlobs(
DARTSCOPE(Thread::Current());
API_TIMELINE_DURATION(T);
Isolate* I = T->isolate();
if (!FLAG_load_deferred_eagerly) {
return Api::NewError(
"Creating full snapshots requires --load_deferred_eagerly");
}
CHECK_NULL(vm_snapshot_data_buffer);
CHECK_NULL(vm_snapshot_data_size);
CHECK_NULL(vm_snapshot_instructions_buffer);
Expand Down Expand Up @@ -6474,10 +6435,6 @@ Dart_CreateAppJITSnapshotAsBlobs(uint8_t** isolate_snapshot_data_buffer,
DARTSCOPE(Thread::Current());
API_TIMELINE_DURATION(T);
Isolate* I = T->isolate();
if (!FLAG_load_deferred_eagerly) {
return Api::NewError(
"Creating full snapshots requires --load_deferred_eagerly");
}
CHECK_NULL(isolate_snapshot_data_buffer);
CHECK_NULL(isolate_snapshot_data_size);
CHECK_NULL(isolate_snapshot_instructions_buffer);
Expand Down
2 changes: 0 additions & 2 deletions runtime/vm/flag_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,6 @@ constexpr bool kDartUseBytecode = false;
P(interpret_irregexp, bool, USING_DBC, "Use irregexp bytecode interpreter") \
P(lazy_dispatchers, bool, true, "Generate dispatchers lazily") \
P(link_natives_lazily, bool, false, "Link native calls lazily") \
C(load_deferred_eagerly, true, true, bool, false, \
"Load deferred libraries eagerly.") \
R(log_marker_tasks, false, bool, false, \
"Log debugging information for old gen GC marking tasks.") \
P(marker_tasks, int, USING_MULTICORE ? 2 : 0, \
Expand Down
5 changes: 0 additions & 5 deletions runtime/vm/isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,6 @@ static void DeterministicModeHandler(bool value) {
FLAG_concurrent_mark = false; // Timing dependent.
FLAG_concurrent_sweep = false; // Timing dependent.
FLAG_random_seed = 0x44617274; // "Dart"
#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
FLAG_load_deferred_eagerly = true;
#else
COMPILE_ASSERT(FLAG_load_deferred_eagerly);
#endif
}
}

Expand Down
Loading

0 comments on commit 84db163

Please sign in to comment.