From ead7f7030afbb7e1fdc8e6fe1679f89bcc4ff6fe Mon Sep 17 00:00:00 2001 From: Johnni Winther Date: Fri, 30 Sep 2016 10:26:03 +0200 Subject: [PATCH] Remove Enqueuer argument from Backend.registerStaticUse - and from CustomElementsAnalysis, TypeVariableHandler, and LookupMapAnalysis methods R=sigmund@google.com Review URL: https://codereview.chromium.org/2378063002 . --- pkg/compiler/lib/src/common/backend_api.dart | 2 +- pkg/compiler/lib/src/compiler.dart | 4 +-- pkg/compiler/lib/src/enqueue.dart | 13 +++++---- pkg/compiler/lib/src/js_backend/backend.dart | 19 ++++++++----- .../js_backend/custom_elements_analysis.dart | 27 ++++++++++--------- pkg/compiler/lib/src/js_backend/enqueuer.dart | 7 +++-- .../src/js_backend/lookup_map_analysis.dart | 7 ++--- .../src/js_backend/type_variable_handler.dart | 9 ++++--- pkg/compiler/lib/src/ssa/builder.dart | 2 +- 9 files changed, 51 insertions(+), 39 deletions(-) diff --git a/pkg/compiler/lib/src/common/backend_api.dart b/pkg/compiler/lib/src/common/backend_api.dart index b93ddee2e33..b8944a1f23c 100644 --- a/pkg/compiler/lib/src/common/backend_api.dart +++ b/pkg/compiler/lib/src/common/backend_api.dart @@ -213,7 +213,7 @@ abstract class Backend extends Target { return false; } - void registerStaticUse(Element element, Enqueuer enqueuer) {} + void registerStaticUse(Element element, {bool forResolution}) {} /// This method is called immediately after the [LibraryElement] [library] has /// been created. diff --git a/pkg/compiler/lib/src/compiler.dart b/pkg/compiler/lib/src/compiler.dart index 789cbe44f2a..000542f8336 100644 --- a/pkg/compiler/lib/src/compiler.dart +++ b/pkg/compiler/lib/src/compiler.dart @@ -835,9 +835,9 @@ abstract class Compiler implements LibraryLoaderListener { work.element, () => selfTask.measureSubtask("world.applyImpact", () { world.applyImpact( - work.element, selfTask.measureSubtask( - "work.run", () => work.run(this, world))); + "work.run", () => work.run(this, world)), + impactSource: work.element); })); }); }); diff --git a/pkg/compiler/lib/src/enqueue.dart b/pkg/compiler/lib/src/enqueue.dart index c29912a3c3b..cacf4e1987e 100644 --- a/pkg/compiler/lib/src/enqueue.dart +++ b/pkg/compiler/lib/src/enqueue.dart @@ -110,7 +110,11 @@ abstract class Enqueuer { void registerInstantiatedType(InterfaceType type, {bool mirrorUsage: false}); void forEach(void f(WorkItem work)); - void applyImpact(Element element, WorldImpact worldImpact); + + /// Apply the [worldImpact] to this enqueuer. If the [impactSource] is provided + /// the impact strategy will remove it from the element impact cache, if it is + /// no longer needed. + void applyImpact(WorldImpact worldImpact, {Element impactSource}); bool checkNoEnqueuedInvokedInstanceMethods(); void logSummary(log(message)); @@ -183,10 +187,9 @@ class ResolutionEnqueuer extends Enqueuer { } } - /// Apply the [worldImpact] of processing [element] to this enqueuer. - void applyImpact(Element element, WorldImpact worldImpact) { + void applyImpact(WorldImpact worldImpact, {Element impactSource}) { compiler.impactStrategy - .visitImpact(element, worldImpact, impactVisitor, impactUse); + .visitImpact(impactSource, worldImpact, impactVisitor, impactUse); } void registerInstantiatedType(InterfaceType type, {bool mirrorUsage: false}) { @@ -574,7 +577,7 @@ class ResolutionEnqueuer extends Enqueuer { assert(invariant(element, element.isDeclaration, message: "Element ${element} is not the declaration.")); _universe.registerStaticUse(staticUse); - compiler.backend.registerStaticUse(element, this); + compiler.backend.registerStaticUse(element, forResolution: true); bool addElement = true; switch (staticUse.kind) { case StaticUseKind.STATIC_TEAR_OFF: diff --git a/pkg/compiler/lib/src/js_backend/backend.dart b/pkg/compiler/lib/src/js_backend/backend.dart index 0899d1b54fb..09af361a5be 100644 --- a/pkg/compiler/lib/src/js_backend/backend.dart +++ b/pkg/compiler/lib/src/js_backend/backend.dart @@ -1310,7 +1310,8 @@ class JavaScriptBackend extends Backend { enqueue(enqueuer, helpers.isJsIndexable, registry); } - customElementsAnalysis.registerInstantiatedClass(cls, enqueuer); + customElementsAnalysis.registerInstantiatedClass(cls, + forResolution: enqueuer.isResolutionQueue); if (!enqueuer.isResolutionQueue) { lookupMapAnalysis.registerInstantiatedClass(cls); } @@ -1904,7 +1905,7 @@ class JavaScriptBackend extends Backend { return compiler.closedWorld.hasOnlySubclasses(classElement); } - void registerStaticUse(Element element, Enqueuer enqueuer) { + void registerStaticUse(Element element, {bool forResolution}) { if (element == helpers.disableTreeShakingMarker) { isTreeShakingDisabled = true; } else if (element == helpers.preserveNamesMarker) { @@ -1927,7 +1928,8 @@ class JavaScriptBackend extends Backend { } else if (element == helpers.requiresPreambleMarker) { requiresPreamble = true; } - customElementsAnalysis.registerStaticUse(element, enqueuer); + customElementsAnalysis.registerStaticUse(element, + forResolution: forResolution); } /// Called when [:const Symbol(name):] is seen. @@ -2351,9 +2353,12 @@ class JavaScriptBackend extends Backend { // // Return early if any elements are added to avoid counting the elements as // due to mirrors. - customElementsAnalysis.onQueueEmpty(enqueuer); - lookupMapAnalysis.onQueueEmpty(enqueuer); - typeVariableHandler.onQueueEmpty(enqueuer); + enqueuer.applyImpact(customElementsAnalysis.flush( + forResolution: enqueuer.isResolutionQueue)); + enqueuer.applyImpact( + lookupMapAnalysis.flush(forResolution: enqueuer.isResolutionQueue)); + enqueuer.applyImpact( + typeVariableHandler.flush(forResolution: enqueuer.isResolutionQueue)); if (!enqueuer.queueIsEmpty) return false; @@ -2424,7 +2429,7 @@ class JavaScriptBackend extends Backend { } metadataConstants.clear(); } - enqueuer.applyImpact(null, impactBuilder.flush()); + enqueuer.applyImpact(impactBuilder.flush()); } return true; } diff --git a/pkg/compiler/lib/src/js_backend/custom_elements_analysis.dart b/pkg/compiler/lib/src/js_backend/custom_elements_analysis.dart index 807bcb95ced..d87ffa31d79 100644 --- a/pkg/compiler/lib/src/js_backend/custom_elements_analysis.dart +++ b/pkg/compiler/lib/src/js_backend/custom_elements_analysis.dart @@ -71,10 +71,11 @@ class CustomElementsAnalysis { codegenJoin.allClassesSelected = true; } - CustomElementsAnalysisJoin joinFor(Enqueuer enqueuer) => - enqueuer.isResolutionQueue ? resolutionJoin : codegenJoin; + CustomElementsAnalysisJoin joinFor({bool forResolution}) => + forResolution ? resolutionJoin : codegenJoin; - void registerInstantiatedClass(ClassElement classElement, Enqueuer enqueuer) { + void registerInstantiatedClass(ClassElement classElement, + {bool forResolution}) { classElement.ensureResolved(compiler.resolution); if (!backend.isNativeOrExtendsNative(classElement)) return; if (classElement.isMixinApplication) return; @@ -82,7 +83,7 @@ class CustomElementsAnalysis { // JsInterop classes are opaque interfaces without a concrete // implementation. if (backend.isJsInterop(classElement)) return; - joinFor(enqueuer).instantiatedClasses.add(classElement); + joinFor(forResolution: forResolution).instantiatedClasses.add(classElement); } void registerTypeLiteral(DartType type) { @@ -104,19 +105,20 @@ class CustomElementsAnalysis { codegenJoin.selectedClasses.add(element); } - void registerStaticUse(Element element, Enqueuer enqueuer) { + void registerStaticUse(Element element, {bool forResolution}) { assert(element != null); if (!fetchedTableAccessorMethod) { fetchedTableAccessorMethod = true; tableAccessorMethod = backend.helpers.findIndexForNativeSubclassType; } if (element == tableAccessorMethod) { - joinFor(enqueuer).demanded = true; + joinFor(forResolution: forResolution).demanded = true; } } - void onQueueEmpty(Enqueuer enqueuer) { - joinFor(enqueuer).flush(enqueuer); + /// Computes the [WorldImpact] of the classes registered since last flush. + WorldImpact flush({bool forResolution}) { + return joinFor(forResolution: forResolution).flush(); } bool get needsTable => codegenJoin.demanded; @@ -152,8 +154,8 @@ class CustomElementsAnalysisJoin { CustomElementsAnalysisJoin(this.backend); - void flush(Enqueuer enqueuer) { - if (!demanded) return; + WorldImpact flush() { + if (!demanded) return const WorldImpact(); var newActiveClasses = new Set(); for (ClassElement classElement in instantiatedClasses) { bool isNative = backend.isNative(classElement); @@ -168,7 +170,8 @@ class CustomElementsAnalysisJoin { Iterable escapingConstructors = computeEscapingConstructors(classElement); for (ConstructorElement constructor in escapingConstructors) { - enqueuer.registerStaticUse(new StaticUse.foreignUse(constructor)); + impactBuilder + .registerStaticUse(new StaticUse.foreignUse(constructor)); } escapingConstructors .forEach(compiler.globalDependencies.registerDependency); @@ -182,7 +185,7 @@ class CustomElementsAnalysisJoin { } activeClasses.addAll(newActiveClasses); instantiatedClasses.removeAll(newActiveClasses); - enqueuer.applyImpact(null, impactBuilder.flush()); + return impactBuilder.flush(); } TypeConstantValue makeTypeConstant(ClassElement element) { diff --git a/pkg/compiler/lib/src/js_backend/enqueuer.dart b/pkg/compiler/lib/src/js_backend/enqueuer.dart index e4b3027fc0f..6b7f19451f4 100644 --- a/pkg/compiler/lib/src/js_backend/enqueuer.dart +++ b/pkg/compiler/lib/src/js_backend/enqueuer.dart @@ -134,10 +134,9 @@ class CodegenEnqueuer implements Enqueuer { } } - /// Apply the [worldImpact] of processing [element] to this enqueuer. - void applyImpact(Element element, WorldImpact worldImpact) { + void applyImpact(WorldImpact worldImpact, {Element impactSource}) { _compiler.impactStrategy - .visitImpact(element, worldImpact, impactVisitor, impactUse); + .visitImpact(impactSource, worldImpact, impactVisitor, impactUse); } void registerInstantiatedType(InterfaceType type, {bool mirrorUsage: false}) { @@ -513,7 +512,7 @@ class CodegenEnqueuer implements Enqueuer { assert(invariant(element, element.isDeclaration, message: "Element ${element} is not the declaration.")); _universe.registerStaticUse(staticUse); - backend.registerStaticUse(element, this); + backend.registerStaticUse(element, forResolution: false); bool addElement = true; switch (staticUse.kind) { case StaticUseKind.STATIC_TEAR_OFF: diff --git a/pkg/compiler/lib/src/js_backend/lookup_map_analysis.dart b/pkg/compiler/lib/src/js_backend/lookup_map_analysis.dart index d729e561fa7..c6e204d7766 100644 --- a/pkg/compiler/lib/src/js_backend/lookup_map_analysis.dart +++ b/pkg/compiler/lib/src/js_backend/lookup_map_analysis.dart @@ -128,9 +128,10 @@ class LookupMapAnalysis { LookupMapAnalysis(this.backend, this.reporter); - void onQueueEmpty(Enqueuer enqueuer) { - if (enqueuer.isResolutionQueue) return; - enqueuer.applyImpact(null, impactBuilder.flush()); + /// Compute the [WorldImpact] for the constants registered since last flush. + WorldImpact flush({bool forResolution}) { + if (forResolution) return const WorldImpact(); + return impactBuilder.flush(); } /// Whether this analysis and optimization is enabled. diff --git a/pkg/compiler/lib/src/js_backend/type_variable_handler.dart b/pkg/compiler/lib/src/js_backend/type_variable_handler.dart index d7c3281f056..595da954243 100644 --- a/pkg/compiler/lib/src/js_backend/type_variable_handler.dart +++ b/pkg/compiler/lib/src/js_backend/type_variable_handler.dart @@ -57,10 +57,11 @@ class TypeVariableHandler { JavaScriptBackend get _backend => _compiler.backend; DiagnosticReporter get reporter => _compiler.reporter; - void onQueueEmpty(Enqueuer enqueuer) { - if (enqueuer.isResolutionQueue) return; - - enqueuer.applyImpact(null, impactBuilder.flush()); + /// Compute the [WorldImpact] for the type variables registered since last + /// flush. + WorldImpact flush({bool forResolution}) { + if (forResolution) return const WorldImpact(); + return impactBuilder.flush(); } void registerClassWithTypeVariables( diff --git a/pkg/compiler/lib/src/ssa/builder.dart b/pkg/compiler/lib/src/ssa/builder.dart index 021ada67844..1b3e19403be 100644 --- a/pkg/compiler/lib/src/ssa/builder.dart +++ b/pkg/compiler/lib/src/ssa/builder.dart @@ -377,7 +377,7 @@ class SsaBuilder extends ast.Visitor // TODO(johnniwinther): Register this on the [registry]. Currently the // [CodegenRegistry] calls the enqueuer, but [element] should _not_ be // enqueued. - backend.registerStaticUse(element, compiler.enqueuer.codegen); + backend.registerStaticUse(element, forResolution: false); if (backend.isJsInterop(element) && !element.isFactoryConstructor) { // We only inline factory JavaScript interop constructors.