Skip to content

Commit

Permalink
[cfe] Track dependencies in the incremental compiler
Browse files Browse the repository at this point in the history 
The objective of this CL is to allow the incremental compiler to track
which dill input libraries it actually used (in order to - in a modular
context - be able to ignore changes to dependencies we didn't really
use).

This is done by:
* Making the dill library builder lazy (and mark if it has been used).
* Making kernels class hierarchy record which classes it has been
  asked questions about.
* Add special handling to redirecting factory constructors as they
  bypass the fasta-builders and directly use the kernel ast.

Please note that:
* This has to be enabled, but kernels class hierarchy always records
  which classes it was asked about (even if disabled,
  or not running though the incremental compiler).
  There might potentially be some overhead to this (though setting
  a bool to true is probably comparably cheap - we did just do a
  map lookup).
* This was designed to be used together with modules
  (e.g. setModulesToLoadOnNextComputeDelta) - as used via for instance
  api_unstable/bazel_worker.dart. It might not function the way you'd
  expect in other circumstances.
* The incremental compiler (potentially) gives you the full kernel tree
  despite not having marked all of those libraries as 'used'. If a
  client use such libraries it is the clients responsibility to take
  that into account when answering any questions about this
  libraries/dill files was used.
* This feature works on the library level. In practice it is most likely
  needed at the dill-filename level. A translation from library to
  dill-filename is up to the client.
* This is a new feature, and we cannot promise that 100% of actually
  used libraries are marked. If you find used but un-marked libraries
  please report a bug.

Change-Id: I01d7ff95b9baac9550b77d8e09ea772d43173641
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/107280
Commit-Queue: Jens Johansen <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
  • Loading branch information
@jensjoha
jensjoha authored and [email protected] committed Jul 12, 2019
1 parent afac6b1 commit 28f95fc
Show file tree
Hide file tree
Showing 17 changed files with 830 additions and 76 deletions.
65 changes: 64 additions & 1 deletion pkg/front_end/lib/src/fasta/dill/dill_library_builder.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,31 @@ import 'dill_loader.dart' show DillLoader;

import 'dill_type_alias_builder.dart' show DillTypeAliasBuilder;

class LazyLibraryScope extends Scope {
DillLibraryBuilder libraryBuilder;

LazyLibraryScope(Map<String, Declaration> local,
Map<String, Declaration> setters, Scope parent, String debugName,
{bool isModifiable: true})
: super(local, setters, parent, debugName, isModifiable: isModifiable);

LazyLibraryScope.top({bool isModifiable: false})
: this(<String, Declaration>{}, <String, Declaration>{}, null, "top",
isModifiable: isModifiable);

Map<String, Declaration> get local {
if (libraryBuilder == null) throw new StateError("No library builder.");
libraryBuilder.ensureLoaded();
return super.local;
}

Map<String, Declaration> get setters {
if (libraryBuilder == null) throw new StateError("No library builder.");
libraryBuilder.ensureLoaded();
return super.setters;
}
}

class DillLibraryBuilder extends LibraryBuilder<KernelTypeBuilder, Library> {
final Library library;

Expand All @@ -64,8 +89,38 @@ class DillLibraryBuilder extends LibraryBuilder<KernelTypeBuilder, Library> {

bool exportsAlreadyFinalized = false;

// TODO(jensj): These 4 booleans could potentially be merged into a single
// state field.
bool isReadyToBuild = false;
bool isReadyToFinalizeExports = false;
bool isBuilt = false;
bool isBuiltAndMarked = false;

DillLibraryBuilder(this.library, this.loader)
: super(library.fileUri, new Scope.top(), new Scope.top());
: super(library.fileUri, new LazyLibraryScope.top(),
new LazyLibraryScope.top()) {
LazyLibraryScope lazyScope = scope;
lazyScope.libraryBuilder = this;
LazyLibraryScope lazyExportScope = exportScope;
lazyExportScope.libraryBuilder = this;
}

void ensureLoaded() {
if (!isReadyToBuild) throw new StateError("Not ready to build.");
isBuiltAndMarked = true;
if (isBuilt) return;
isBuilt = true;
library.classes.forEach(addClass);
library.procedures.forEach(addMember);
library.typedefs.forEach(addTypedef);
library.fields.forEach(addMember);

if (isReadyToFinalizeExports) {
finalizeExports();
} else {
throw new StateError("Not ready to finalize exports.");
}
}

@override
bool get isSynthetic => library.isSynthetic;
Expand Down Expand Up @@ -174,6 +229,14 @@ class DillLibraryBuilder extends LibraryBuilder<KernelTypeBuilder, Library> {
return library.name ?? "<library '${library.fileUri}'>";
}

void markAsReadyToBuild() {
isReadyToBuild = true;
}

void markAsReadyToFinalizeExports() {
isReadyToFinalizeExports = true;
}

void finalizeExports() {
if (exportsAlreadyFinalized) return;
exportsAlreadyFinalized = true;
Expand Down
7 changes: 2 additions & 5 deletions pkg/front_end/lib/src/fasta/dill/dill_loader.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,7 @@ class DillLoader extends Loader<Library> {
if (builder.library == null) {
unhandled("null", "builder.library", 0, builder.fileUri);
}
builder.library.classes.forEach(builder.addClass);
builder.library.procedures.forEach(builder.addMember);
builder.library.typedefs.forEach(builder.addTypedef);
builder.library.fields.forEach(builder.addMember);
builder.markAsReadyToBuild();
}

Future<Null> buildBody(DillLibraryBuilder builder) {
Expand All @@ -74,7 +71,7 @@ class DillLoader extends Loader<Library> {
void finalizeExports() {
builders.forEach((Uri uri, LibraryBuilder builder) {
DillLibraryBuilder library = builder;
library.finalizeExports();
library.markAsReadyToFinalizeExports();
});
}

Expand Down
106 changes: 105 additions & 1 deletion pkg/front_end/lib/src/fasta/incremental_compiler.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import 'package:kernel/binary/ast_from_binary.dart'
CanonicalNameSdkError,
InvalidKernelVersionError;

import 'package:kernel/class_hierarchy.dart' show ClassHierarchy;
import 'package:kernel/class_hierarchy.dart'
show ClassHierarchy, ClosedWorldClassHierarchy;

import 'package:kernel/kernel.dart'
show
Expand All @@ -32,6 +33,7 @@ import 'package:kernel/kernel.dart'
ProcedureKind,
ReturnStatement,
Source,
Supertype,
TreeNode,
TypeParameter;

Expand Down Expand Up @@ -68,6 +70,8 @@ import 'fasta_codes.dart'

import 'hybrid_file_system.dart' show HybridFileSystem;

import 'kernel/kernel_builder.dart' show ClassHierarchyBuilder;

import 'kernel/kernel_library_builder.dart' show KernelLibraryBuilder;

import 'kernel/kernel_shadow_ast.dart' show VariableDeclarationJudgment;
Expand All @@ -90,6 +94,8 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
final Ticker ticker;

final bool outlineOnly;
bool trackNeededDillLibraries = false;
Set<Library> neededDillLibraries;

Set<Uri> invalidatedUris = new Set<Uri>();

Expand Down Expand Up @@ -301,6 +307,19 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
uriTranslator);
userCode.loader.hierarchy = hierarchy;

if (trackNeededDillLibraries) {
// Reset dill loaders and kernel class hierarchy.
for (LibraryBuilder builder in dillLoadedData.loader.builders.values) {
if (builder is DillLibraryBuilder) {
builder.isBuiltAndMarked = false;
}
}

if (hierarchy is ClosedWorldClassHierarchy) {
hierarchy.resetUsed();
}
}

for (LibraryBuilder library in reusedLibraries) {
userCode.loader.builders[library.uri] = library;
if (library.uri.scheme == "dart" && library.uri.path == "core") {
Expand All @@ -324,8 +343,23 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
componentWithDill =
await userCode.buildComponent(verify: c.options.verify);
}
hierarchy ??= userCode.loader.hierarchy;

recordNonFullComponentForTesting(componentWithDill);
if (trackNeededDillLibraries) {
// Which dill builders were built?
neededDillLibraries = new Set<Library>();
for (LibraryBuilder builder in dillLoadedData.loader.builders.values) {
if (builder is DillLibraryBuilder) {
if (builder.isBuiltAndMarked) {
neededDillLibraries.add(builder.library);
}
}
}

updateNeededDillLibraresWithHierarchy(
hierarchy, userCode.loader.builderHierarchy);
}

if (componentWithDill != null) {
this.invalidatedUris.clear();
Expand Down Expand Up @@ -388,6 +422,76 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
});
}

/// Allows for updating the list of needed libraries.
///
/// Useful if a class hierarchy has been used externally.
/// Currently there are two different class hierarchies which is unfortunate.
/// For now this method allows the 'ClassHierarchyBuilder' to be null.
///
/// TODO(jensj,CFE in general): Eventually we should get to a point where we
/// only have one class hierarchy.
/// TODO(jensj): This could probably be a utility method somewhere instead
/// (though handling of the case where all bets are off should probably still
/// live locally).
void updateNeededDillLibraresWithHierarchy(
ClassHierarchy hierarchy, ClassHierarchyBuilder builderHierarchy) {
if (hierarchy is ClosedWorldClassHierarchy && !hierarchy.allBetsOff) {
neededDillLibraries ??= new Set<Library>();
Set<Class> classes = new Set<Class>();
List<Class> worklist = new List<Class>();
// Get all classes touched by kernel class hierarchy.
List<Class> usedClasses = hierarchy.getUsedClasses();
worklist.addAll(usedClasses);
classes.addAll(usedClasses);

// Get all classes touched by fasta class hierarchy.
if (builderHierarchy != null) {
for (Class c in builderHierarchy.nodes.keys) {
if (classes.add(c)) worklist.add(c);
}
}

// Get all supers etc.
while (worklist.isNotEmpty) {
Class c = worklist.removeLast();
for (Supertype supertype in c.implementedTypes) {
if (classes.add(supertype.classNode)) {
worklist.add(supertype.classNode);
}
}
if (c.mixedInType != null) {
if (classes.add(c.mixedInType.classNode)) {
worklist.add(c.mixedInType.classNode);
}
}
if (c.supertype != null) {
if (classes.add(c.supertype.classNode)) {
worklist.add(c.supertype.classNode);
}
}
}

// Add any libraries that was used or was in the "parent-chain" of a
// used class.
for (Class c in classes) {
Library library = c.enclosingLibrary;
// Only add if loaded from a dill file.
if (dillLoadedData.loader.builders.containsKey(library.importUri)) {
neededDillLibraries.add(library);
}
}
} else {
// Cannot track in other kernel class hierarchies or
// if all bets are off: Add everything.
neededDillLibraries = new Set<Library>();
for (LibraryBuilder builder in dillLoadedData.loader.builders.values) {
if (builder is DillLibraryBuilder) {
neededDillLibraries.add(builder.library);
}
}
}
}

/// Internal method.
void invalidateNotKeptUserBuilders(Set<Uri> invalidatedUris) {
if (modulesToLoad != null && userBuilders != null) {
Expand Down
56 changes: 51 additions & 5 deletions pkg/front_end/lib/src/fasta/kernel/body_builder.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import 'dart:core' hide MapEntry;

import '../constant_context.dart' show ConstantContext;

import '../dill/dill_library_builder.dart' show DillLibraryBuilder;

import '../fasta_codes.dart' as fasta;

import '../fasta_codes.dart' show LocatedMessage, Message, noLength, Template;
Expand Down Expand Up @@ -133,7 +135,7 @@ const noLocation = null;
const invalidCollectionElement = const Object();

abstract class BodyBuilder extends ScopeListener<JumpTarget>
implements ExpressionGeneratorHelper {
implements ExpressionGeneratorHelper, EnsureLoaded {
// TODO(ahe): Rename [library] to 'part'.
@override
final KernelLibraryBuilder library;
Expand Down Expand Up @@ -903,6 +905,44 @@ abstract class BodyBuilder extends ScopeListener<JumpTarget>
finishVariableMetadata();
}

/// Ensure that the containing library of the [member] has been loaded.
///
/// This is for instance important for lazy dill library builders where this
/// method has to be called to ensure that
/// a) The library has been fully loaded (and for instance any internal
/// transformation needed has been performed); and
/// b) The library is correctly marked as being used to allow for proper
/// 'dependency pruning'.
void ensureLoaded(Member member) {
if (member == null) return;
Library ensureLibraryLoaded = member.enclosingLibrary;
LibraryBuilder<dynamic, dynamic> builder =
library.loader.builders[ensureLibraryLoaded.importUri] ??
library.loader.target.dillTarget.loader
.builders[ensureLibraryLoaded.importUri];
if (builder is DillLibraryBuilder) {
builder.ensureLoaded();
}
}

/// Check if the containing library of the [member] has been loaded.
///
/// This is designed for use with asserts.
/// See [ensureLoaded] for a description of what 'loaded' means and the ideas
/// behind that.
bool isLoaded(Member member) {
if (member == null) return true;
Library ensureLibraryLoaded = member.enclosingLibrary;
LibraryBuilder<dynamic, dynamic> builder =
library.loader.builders[ensureLibraryLoaded.importUri] ??
library.loader.target.dillTarget.loader
.builders[ensureLibraryLoaded.importUri];
if (builder is DillLibraryBuilder) {
return builder.isBuiltAndMarked;
}
return true;
}

void resolveRedirectingFactoryTargets() {
for (StaticInvocation invocation in redirectingFactoryInvocations) {
// If the invocation was invalid, it or its parent has already been
Expand All @@ -927,7 +967,7 @@ abstract class BodyBuilder extends ScopeListener<JumpTarget>
Expression replacementNode;

RedirectionTarget redirectionTarget =
getRedirectionTarget(initialTarget, legacyMode: legacyMode);
getRedirectionTarget(initialTarget, this, legacyMode: legacyMode);
Member resolvedTarget = redirectionTarget?.target;

if (resolvedTarget == null) {
Expand Down Expand Up @@ -3355,7 +3395,7 @@ abstract class BodyBuilder extends ScopeListener<JumpTarget>
int charLength: noLength}) {
// The argument checks for the initial target of redirecting factories
// invocations are skipped in Dart 1.
if (!legacyMode || !isRedirectingFactory(target)) {
if (!legacyMode || !isRedirectingFactory(target, helper: this)) {
List<TypeParameter> typeParameters = target.function.typeParameters;
if (target is Constructor) {
assert(!target.enclosingClass.isAbstract);
Expand Down Expand Up @@ -3664,7 +3704,7 @@ abstract class BodyBuilder extends ScopeListener<JumpTarget>
(target is Procedure && target.kind == ProcedureKind.Factory)) {
Expression invocation;

if (legacyMode && isRedirectingFactory(target)) {
if (legacyMode && isRedirectingFactory(target, helper: this)) {
// In legacy mode the checks that are done in [buildStaticInvocation]
// on the initial target of a redirecting factory invocation should
// be skipped. So we build the invocation nodes directly here without
Expand All @@ -3690,7 +3730,8 @@ abstract class BodyBuilder extends ScopeListener<JumpTarget>
charLength: nameToken.length);
}

if (invocation is StaticInvocation && isRedirectingFactory(target)) {
if (invocation is StaticInvocation &&
isRedirectingFactory(target, helper: this)) {
redirectingFactoryInvocations.add(invocation);
}

Expand Down Expand Up @@ -5408,6 +5449,11 @@ abstract class BodyBuilder extends ScopeListener<JumpTarget>
}
}

abstract class EnsureLoaded {
void ensureLoaded(Member member);
bool isLoaded(Member member);
}

class Operator {
final Token token;
String get name => token.stringValue;
Expand Down
Loading

0 comments on commit 28f95fc

Please sign in to comment.