Makes Generator output invalidation more accurate. Previously all output for any Generator that was being rerun as a result of modified resources or types was assumed stale. For Generators with large amounts of output (Gin, GWTRPC) this was very costly. This change stops making that assumption and instead watches the types created by Generators, flags the ones that have changed as modified and recalculates the stale types list. Previously UnifyAst assumed that the stale types list did not change, so extra care had to be taken there. Change-Id: I5f296eacb30bb6abc22d9daeccb87423c1766736 Review-Link: https://gwt-review.googlesource.com/#/c/9201/
diff --git a/dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java b/dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java index 94aab2a..5bc440a 100644 --- a/dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java +++ b/dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java
@@ -160,11 +160,11 @@ private final Multimap<String, String> referencedTypeNamesByTypeName = HashMultimap.create(); private final Set<String> rootTypeNames = Sets.newHashSet(); private final Set<String> singleJsoImplInterfaceNames = Sets.newHashSet(); + private final Set<String> sourceCompilationUnitNames = Sets.newHashSet(); private final Map<String, JsSourceMap> sourceMapsByTypeName = Maps.newHashMap(); private final Set<String> staleTypeNames = Sets.newHashSet(); private final Map<String, StatementRanges> statementRangesByTypeName = Maps.newHashMap(); private final Multimap<String, String> typeNamesByReferencingTypeName = HashMultimap.create(); - private final Set<String> sourceCompilationUnitNames = Sets.newHashSet(); /** * Accumulates generated artifacts so that they can be output on recompiles even if no generators @@ -254,8 +254,8 @@ * changes in interface while the invalidation here must also look at JSO promotion/demotion and * cast references because of the peculiarities of output JS format. */ - public Set<String> clearStaleTypeJsAndStatements(TreeLogger logger, JTypeOracle typeOracle) { - if (immediateTypeRelations.isEmpty()) { + public Set<String> computeAndClearStaleTypesCache(TreeLogger logger, JTypeOracle typeOracle) { + if (!isPopulated()) { return Sets.newHashSet(); } @@ -323,23 +323,11 @@ * Computes and returns the set of names of modified types. * <p> * Modified types are all those types that are nested within compilation units that are known to - * have been modified or were generated by a Generator that is known to have had its output - * invalidated. + * have been modified. */ public Set<String> computeModifiedTypeNames() { // Accumulate the names of types that are nested within known modified compilation units. - Set<String> modifiedTypeNames = Sets.newHashSet(); - modifiedTypeNames.addAll(computeNestedTypeNames(modifiedCompilationUnitNames)); - - // Accumulate the names of types that are nested within compilation units that are expected to - // be modified once invalidated Generators have been run (the Generators were invalidated - // because they read resources that are known to be modified). - Set<String> affectedReboundTypeNames = computeReboundTypesAffectedByModifiedResources(); - Set<String> assumedModifiedGeneratedCompilationUnitNames = - computeCompilationUnitsGeneratedForReboundTypes(affectedReboundTypeNames); - modifiedTypeNames.addAll(computeNestedTypeNames(assumedModifiedGeneratedCompilationUnitNames)); - - return modifiedTypeNames; + return Sets.newHashSet(computeNestedTypeNames(modifiedCompilationUnitNames)); } /** @@ -376,17 +364,6 @@ return immediateTypeRelations; } - /** - * Returns true if there is cached data to reuse in the next recompile. - */ - public boolean isPopulated() { - return !immediateTypeRelations.isEmpty(); - } - - public boolean isSourceCompilationUnit(String compilationUnitName) { - return sourceCompilationUnitNames.contains(compilationUnitName); - } - public IntTypeIdGenerator getIntTypeIdGenerator() { return intTypeIdGenerator; } @@ -433,6 +410,17 @@ return !preambleTypeNames.isEmpty(); } + /** + * Returns true if there is cached data to reuse in the next recompile. + */ + public boolean isPopulated() { + return !immediateTypeRelations.isEmpty(); + } + + public boolean isSourceCompilationUnit(String compilationUnitName) { + return sourceCompilationUnitNames.contains(compilationUnitName); + } + public boolean knowsLastLinkedJsBytes() { return lastLinkedJsBytes != null; } @@ -612,14 +600,6 @@ // Mark these generator triggering types stale and keep track of whether any of them were not // previously known to be stale. discoveredMoreStaleTypes = staleTypeNames.addAll(generatorTriggeringTypes); - // Output of Generators run for these triggering types, might not be stable, so all the - // resulting generated types must be assumed stale. - // TODO(stalcup): this overly broad assumption that generated types are stale might be very - // costly for individual Generators that have lots of output (like GWTRPC). It would be faster - // (and more complicated to instead watch Generator output for actually modified types and - // then recalculate staleness. - staleTypeNames.addAll(computeNestedTypeNames(computeCompilationUnitsGeneratedForReboundTypes( - reboundTypesThatGenerateTheStaleCompilationUnits))); // It's possible that a generator triggering type was itself also created by a Generator. // Repeat the backwards trace process till none of the newly stale types are generated types.
diff --git a/dev/core/src/com/google/gwt/dev/NullRebuildCache.java b/dev/core/src/com/google/gwt/dev/NullRebuildCache.java index 289371d..0f0079a 100644 --- a/dev/core/src/com/google/gwt/dev/NullRebuildCache.java +++ b/dev/core/src/com/google/gwt/dev/NullRebuildCache.java
@@ -37,10 +37,6 @@ "The RebuildCache should not be interacted with outside of per-file compiles."; @Override - public void addSourceCompilationUnitName(String sourceCompilationUnitName) { - } - - @Override public void addGeneratedArtifacts(ArtifactSet generatedArtifacts) { } @@ -50,6 +46,10 @@ } @Override + public void addSourceCompilationUnitName(String sourceCompilationUnitName) { + } + + @Override public void addTypeReference(String fromTypeName, String toTypeName) { } @@ -76,7 +76,7 @@ } @Override - public Set<String> clearStaleTypeJsAndStatements(TreeLogger logger, JTypeOracle typeOracle) { + public Set<String> computeAndClearStaleTypesCache(TreeLogger logger, JTypeOracle typeOracle) { throw new UnsupportedOperationException(failMessage); }
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java index fb1c9e2..1c81594 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
@@ -1049,6 +1049,10 @@ } } + public void removeReferenceOnlyType(JDeclaredType type) { + referenceOnlyTypeNames.remove(type.getName()); + } + public void setFragmentPartitioningResult(FragmentPartitioningResult result) { fragmentPartitioningResult = result; }
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java b/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java index bcd7043..2c5adb1 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
@@ -486,7 +486,14 @@ minimalRebuildCache.addGeneratedArtifacts(rpo.getGeneratorContext().getArtifacts()); } rpo.getGeneratorContext().finish(logger); - fullFlowIntoStaleTypes(); + if (compilePerFile) { + // There may be more types known to be modified after Generator execution, which would + // mean the previous stale types calculation was too small. Redo it. + staleTypeNames = + minimalRebuildCache.computeAndClearStaleTypesCache(logger, program.typeOracle); + checkPreambleTypesStillFresh(logger); + fullFlowIntoRemainingStaleTypes(); + } } catch (UnableToCompleteException e) { error(gwtCreateCall, "Failed to resolve '" + reqType + "' via deferred binding"); return null; @@ -667,14 +674,17 @@ * The names of types whose per-file compilation cached Js and StatementRanges are known to no * longer be valid. * <p> - * Is initialized to the full initial list at the beginning of exec() and shrinks as each stale - * type is processed. Reducing the stale types set size helps keep the repeated attempts (once - * after each Generator) to eagerly process stale types (some of which don't exist yet) from - * becoming a performance problem. + * Is initialized to the full initial list at the beginning of exec() and may be recalculated + * (larger) after Generator executions reveal more modified types. */ private Set<String> staleTypeNames = Sets.newHashSet(); /** + * The names of stale types that have been processed (fully traversed) so far. + */ + private Set<String> processedStaleTypeNames = Sets.newHashSet(); + + /** * A work queue of methods whose bodies we need to traverse. Prevents * excessive stack use. */ @@ -709,7 +719,7 @@ this.minimalRebuildCache = compilerContext.getMinimalRebuildCache(); if (compilePerFile) { this.staleTypeNames = - minimalRebuildCache.clearStaleTypeJsAndStatements(logger, program.typeOracle); + minimalRebuildCache.computeAndClearStaleTypesCache(logger, program.typeOracle); checkPreambleTypesStillFresh(logger); } } @@ -794,7 +804,7 @@ } if (compilePerFile) { - fullFlowIntoStaleTypes(); + fullFlowIntoRemainingStaleTypes(); } else if (isLibraryCompile) { // Trace execution from all types supplied as source and resolve references. Set<String> internalNames = ImmutableSet.copyOf(compiledClassesByInternalName.keySet()); @@ -862,10 +872,12 @@ + program.getModuleDeclaredTypes().size() + " are considered part of the current module and " + fullFlowTypes.size() + " had all of their fields and methods traversed."); - if (!staleTypeNames.isEmpty()) { - logger.log(TreeLogger.WARN, "Some stale types (" + staleTypeNames - + ") should have been but were not reprocessed. This is likely " - + "a bug. Please report it to the GWT team."); + + Set<String> remainingStaleTypeNames = computeRemainingStaleTypeNames(); + if (!remainingStaleTypeNames.isEmpty()) { + logger.log(TreeLogger.WARN, "Some stale types (" + remainingStaleTypeNames + + ") were not reprocessed as was expected. This is either a compiler bug or a " + + "Generator has legitimately stopped creating these types."); } } @@ -889,10 +901,8 @@ * Some types may not exist till after some Generator execution so missing types will be * temporarily ignored. */ - private void fullFlowIntoStaleTypes() { - Set<String> remainingStaleTypeNames = Sets.newHashSet(staleTypeNames); - - for (String staleTypeName : remainingStaleTypeNames) { + private void fullFlowIntoRemainingStaleTypes() { + for (String staleTypeName : computeRemainingStaleTypeNames()) { JDeclaredType staleType = internalFindType(staleTypeName, binaryNameBasedTypeLocator, false); if (staleType == null) { @@ -902,6 +912,12 @@ // GWT.create() calls that process that create this type. continue; } + // It's possible that the type was previously loaded before it was discovered to be stale (it + // became stale as a result of a Generator execution). If this happens then the type will have + // already been marked "reference only" in JProgram. This needs to be undone. + program.removeReferenceOnlyType(staleType); + + // Make sure that the entire type is traversed. fullFlowIntoType(staleType); } } @@ -1112,6 +1128,10 @@ } } + private Set<String> computeRemainingStaleTypeNames() { + return Sets.newHashSet(Sets.difference(staleTypeNames, processedStaleTypeNames)); + } + private boolean containsAllTypes(CompilationUnit unit, List<JDeclaredType> types) { Set<String> binaryTypeNames = new HashSet<String>(); for (JDeclaredType type : types) { @@ -1157,7 +1177,7 @@ fullFlowTypes.add(type.getName()); // Remove the type from the remaining stale types set so that the fullFlowIntoStaleTypes() // attempt is shorter. - staleTypeNames.remove(type.getName()); + processedStaleTypeNames.add(type.getName()); instantiate(type); for (JField field : type.getFields()) { flowInto(field);
diff --git a/dev/core/test/com/google/gwt/dev/CompilerTest.java b/dev/core/test/com/google/gwt/dev/CompilerTest.java index 1efda99..bdbb86c 100644 --- a/dev/core/test/com/google/gwt/dev/CompilerTest.java +++ b/dev/core/test/com/google/gwt/dev/CompilerTest.java
@@ -801,9 +801,10 @@ // FooResourceGenerator and cascade the invalidate the Generators that triggered // FooResourceGenerator. compileToJs(compilerOptions, relinkApplicationDir, "com.foo.SimpleModule", - Lists.<MockResource> newArrayList(classNameToGenerateResource), relinkMinimalRebuildCache, - stringSet("com.foo.TestEntryPoint", "com.foo.Baz", "com.foo.Baz$InnerBaz", "com.foo.Bar", - "com.foo.HasCustomContent", "com.foo.FooReplacementOne"), output); + Lists.<MockResource> newArrayList(modifiedClassNameToGenerateResource), + relinkMinimalRebuildCache, stringSet("com.foo.TestEntryPoint", "com.foo.Baz$InnerBaz", + "com.foo.Bar", "com.foo.HasCustomContent", "com.foo.FooReplacementOne", + "com.foo.FooReplacementTwo"), output); // Generators were run again. assertEquals(2, CauseStringRebindGenerator.runCount); @@ -892,9 +893,8 @@ checkRecompiledModifiedApp(compilerOptions, "com.foo.UiBinderTestModule", Lists.newArrayList( uiBinderTestModuleResource, uiBinderTestEntryPointResource, myWidgetUiXml), myWidget, - myWidget, stringSet("com.foo.MyWidget", "com.foo.MyWidget_BinderImpl_GenBundle", - "com.foo.MyWidget$Binder", "com.foo.MyWidget_BinderImpl$Template", "com.foo.TestEntryPoint", - "com.foo.MyWidget_BinderImpl", "com.foo.MyWidget_BinderImpl$Widgets"), output); + myWidget, stringSet("com.foo.MyWidget", "com.foo.MyWidget$Binder", "com.foo.TestEntryPoint", + "com.foo.MyWidget_BinderImpl", "com.foo.MyWidget_BinderImpl$Widgets"), output); } private void checkPerFileRecompile_packagePrivateOverride(JsOutputOption output) @@ -995,7 +995,7 @@ checkRecompiledModifiedApp(compilerOptions, "com.foo.SimpleModule", Lists.newArrayList(multipleClassGeneratorModuleResource, generatorEntryPointResource), bazResource, bazResource, - stringSet("com.foo.Baz", "com.foo.TestEntryPoint", "com.foo.Foo", "com.foo.Bar"), output); + stringSet("com.foo.Baz", "com.foo.TestEntryPoint", "com.foo.Bar"), output); } private void checkPerFileRecompile_dualJsoIntfDispatchChange(JsOutputOption output) @@ -1018,7 +1018,7 @@ resourceReadingGeneratorModuleResource, generatorEntryPointResource, fooInterfaceResource, nonJsoFooResource), classNameToGenerateResource, modifiedClassNameToGenerateResource, Sets.< String> newHashSet("com.foo.TestEntryPoint", "com.foo.FooReplacementOne", - "com.foo.HasCustomContent"), outputOption); + "com.foo.HasCustomContent", "com.foo.FooReplacementTwo"), outputOption); } private void assertDeterministicBuild(String topLevelModule, int optimizationLevel)
diff --git a/dev/core/test/com/google/gwt/dev/MinimalRebuildCacheTest.java b/dev/core/test/com/google/gwt/dev/MinimalRebuildCacheTest.java index 65a50fb..77bf748 100644 --- a/dev/core/test/com/google/gwt/dev/MinimalRebuildCacheTest.java +++ b/dev/core/test/com/google/gwt/dev/MinimalRebuildCacheTest.java
@@ -62,7 +62,7 @@ minimalRebuildCache.recordDiskSourceResources(laterModifiedBySourcePath); // Request clearing of cache related to stale types. - minimalRebuildCache.clearStaleTypeJsAndStatements(TreeLogger.NULL, + minimalRebuildCache.computeAndClearStaleTypesCache(TreeLogger.NULL, new JTypeOracle(null, minimalRebuildCache, true)); // Has the expected JS been cleared?