Makes SDM MinimalRebuildCache updates transactional.
When SDM invokes incremental compiles it takes care to pass the proper
MinimalRebuildCache instance (for the current binding properties) to
the Compiler, and the Compiler updates the cache in place.
But if the compile failed it was possible for the cache to be left in
an inconsistent state (for example having already had some entries
removed but not yet had them rebuilt).
This change teaches SDM to always pass the Compiler a copy of the
current known-good MinimalRebuildCache and only replace its copy with
a modified one after a successful compile. This way whenever a compile
starts it will always be starting with a known good MinimalRebuildCache
instance.
Change-Id: I1d536c01746f6ede846cf0ddc519267f3809b6df
Review-Link: https://gwt-review.googlesource.com/#/c/9360/
diff --git a/dev/codeserver/BUILD b/dev/codeserver/BUILD
index 548fd96..c634917 100644
--- a/dev/codeserver/BUILD
+++ b/dev/codeserver/BUILD
@@ -47,19 +47,30 @@
srcs = glob(["javatests/**/*.java"]),
deps = [
":codeserver-bare",
+ "//java/com/google/common/collect",
"//third_party/java/junit",
- "//third_party/java_src/gwt/svn/trunk/dev:gwt-dev-bare",
+ "//third_party/java_src/gwt/svn/tools:dev_deps",
+ "//third_party/java_src/gwt/svn/trunk/dev:gwt-dev-only",
+ "//third_party/java_src/gwt/svn/trunk/user:user-test-code",
],
)
java_test(
- name = "tests",
+ name = "source-handler-test",
test_class = "com.google.gwt.dev.codeserver.SourceHandlerTest",
runtime_deps = [
":testlib",
],
)
+java_test(
+ name = "recompiler-test",
+ test_class = "com.google.gwt.dev.codeserver.RecompilerTest",
+ runtime_deps = [
+ ":testlib",
+ ],
+)
+
# Repackaged codeserver for google3.
AugmentedJar(
name = "codeserver",
diff --git a/dev/codeserver/java/com/google/gwt/dev/codeserver/Recompiler.java b/dev/codeserver/java/com/google/gwt/dev/codeserver/Recompiler.java
index 565abed..d8735f6 100644
--- a/dev/codeserver/java/com/google/gwt/dev/codeserver/Recompiler.java
+++ b/dev/codeserver/java/com/google/gwt/dev/codeserver/Recompiler.java
@@ -295,15 +295,20 @@
compilerContext = compilerContextBuilder.options(runOptions).build();
// Looks up the matching rebuild cache using the final set of overridden binding properties.
- MinimalRebuildCache minimalRebuildCache = getOrCreateMinimalRebuildCache(bindingProperties);
+ MinimalRebuildCache knownGoodMinimalRebuildCache =
+ getKnownGoodMinimalRebuildCache(bindingProperties);
+ job.setCompileStrategy(knownGoodMinimalRebuildCache.isPopulated() ? CompileStrategy.INCREMENTAL
+ : CompileStrategy.FULL);
- job.setCompileStrategy(minimalRebuildCache.isPopulated() ? CompileStrategy.INCREMENTAL :
- CompileStrategy.FULL);
-
- boolean success = new Compiler(runOptions, minimalRebuildCache).run(compileLogger, module);
+ // Takes care to transactionally replace the saved cache only after a successful compile.
+ MinimalRebuildCache mutableMinimalRebuildCache = new MinimalRebuildCache();
+ mutableMinimalRebuildCache.copyFrom(knownGoodMinimalRebuildCache);
+ boolean success =
+ new Compiler(runOptions, mutableMinimalRebuildCache).run(compileLogger, module);
if (success) {
publishedCompileDir = compileDir;
lastBuildInput = input;
+ saveKnownGoodMinimalRebuildCache(bindingProperties, mutableMinimalRebuildCache);
} else {
// always recompile after an error
lastBuildInput = null;
@@ -351,7 +356,7 @@
}
}
- private MinimalRebuildCache getOrCreateMinimalRebuildCache(
+ private MinimalRebuildCache getKnownGoodMinimalRebuildCache(
Map<String, String> bindingProperties) {
if (!options.isIncrementalCompileEnabled()) {
return new NullRebuildCache();
@@ -366,6 +371,15 @@
return minimalRebuildCache;
}
+ private void saveKnownGoodMinimalRebuildCache(Map<String, String> bindingProperties,
+ MinimalRebuildCache knownGoodMinimalRebuildCache) {
+ if (!options.isIncrementalCompileEnabled()) {
+ return;
+ }
+
+ minimalRebuildCacheForProperties.put(bindingProperties, knownGoodMinimalRebuildCache);
+ }
+
/**
* Loads the module and configures it for SuperDevMode. (Does not restrict permutations.)
*/
diff --git a/dev/codeserver/javatests/com/google/gwt/dev/codeserver/RecompilerTest.java b/dev/codeserver/javatests/com/google/gwt/dev/codeserver/RecompilerTest.java
new file mode 100644
index 0000000..0bac061
--- /dev/null
+++ b/dev/codeserver/javatests/com/google/gwt/dev/codeserver/RecompilerTest.java
@@ -0,0 +1,163 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.dev.codeserver;
+
+import com.google.gwt.core.ext.TreeLogger;
+import com.google.gwt.core.ext.UnableToCompleteException;
+import com.google.gwt.dev.codeserver.Job.Result;
+import com.google.gwt.dev.javac.UnitCacheSingleton;
+import com.google.gwt.dev.javac.testing.impl.JavaResourceBase;
+import com.google.gwt.dev.javac.testing.impl.MockJavaResource;
+import com.google.gwt.dev.javac.testing.impl.MockResource;
+import com.google.gwt.dev.util.log.PrintWriterTreeLogger;
+import com.google.gwt.thirdparty.guava.common.base.Charsets;
+import com.google.gwt.thirdparty.guava.common.collect.Lists;
+import com.google.gwt.thirdparty.guava.common.io.Files;
+
+import junit.framework.TestCase;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Tests for {@link Recompiler}
+ */
+public class RecompilerTest extends TestCase {
+
+ private static void writeResourcesTo(List<MockResource> resources, File dir) throws IOException {
+ for (MockResource applicationResource : resources) {
+ File resourceFile =
+ new File(dir.getAbsolutePath() + File.separator + applicationResource.getPath());
+ resourceFile.getParentFile().mkdirs();
+ Files.write(applicationResource.getContent(), resourceFile, Charsets.UTF_8);
+ }
+ }
+
+ private MockJavaResource barReferencesBazResource =
+ JavaResourceBase.createMockJavaResource("com.foo.Bar",
+ "package com.foo;",
+ "public class Bar {",
+ " Baz baz = new Baz();",
+ "}");
+
+ private MockJavaResource bazReferencesFooResource =
+ JavaResourceBase.createMockJavaResource("com.foo.Baz",
+ "package com.foo;",
+ "public class Baz {",
+ " Foo foo = new Foo();",
+ "}");
+
+ private MockJavaResource fooResource =
+ JavaResourceBase.createMockJavaResource("com.foo.Foo",
+ "package com.foo;",
+ "public class Foo {}");
+
+ private MockJavaResource nonCompilableFooResource =
+ JavaResourceBase.createMockJavaResource("com.foo.Foo",
+ "package com.foo;",
+ "import com.google.gwt.core.client.impl.SpecializeMethod;",
+ "public class Foo {",
+ " // This will throw an error in UnifyAst.",
+ " @SpecializeMethod()",
+ " public void run() {}",
+ "}");
+
+ private MockJavaResource referencesBarEntryPointResource =
+ JavaResourceBase.createMockJavaResource("com.foo.TestEntryPoint",
+ "package com.foo;",
+ "import com.google.gwt.core.client.EntryPoint;",
+ "public class TestEntryPoint implements EntryPoint {",
+ " @Override",
+ " public void onModuleLoad() {",
+ " Bar bar = new Bar();",
+ " }",
+ "}");
+
+ private MockResource simpleModuleResource =
+ JavaResourceBase.createMockResource("com/foo/SimpleModule.gwt.xml",
+ "<module>",
+ "<source path=''/>",
+ "<entry-point class='com.foo.TestEntryPoint'/>",
+ "</module>");
+
+ public void testIncrementalRecompile_compileErrorDoesntCorruptMinimalRebuildCache()
+ throws UnableToCompleteException, IOException, InterruptedException {
+ PrintWriterTreeLogger logger = new PrintWriterTreeLogger();
+ logger.setMaxDetail(TreeLogger.ERROR);
+
+ File sourcePath = Files.createTempDir();
+ // Setup options to perform a per-file compile and compile the given module.
+ Options options = new Options();
+ options.parseArgs(new String[] {
+ "-incremental", "-src", sourcePath.getAbsolutePath(), "com.foo.SimpleModule"});
+
+ // Prepare the basic resources in the test application.
+ List<MockResource> originalResources = Lists.newArrayList(simpleModuleResource,
+ referencesBarEntryPointResource, barReferencesBazResource, bazReferencesFooResource,
+ fooResource);
+ writeResourcesTo(originalResources, sourcePath);
+
+ Recompiler recompiler =
+ new Recompiler(AppSpace.create(Files.createTempDir()), "com.foo.SimpleModule", options);
+ Outbox outbox = new Outbox("Transactional Cache", recompiler, options, logger);
+ OutboxTable outboxes = new OutboxTable();
+ outboxes.addOutbox(outbox);
+ JobRunner runner = new JobRunner(new JobEventTable(), outboxes);
+
+ // Perform a first compile. This should pass since all resources are valid.
+ Result result =
+ compileWithChanges(logger, runner, outbox, sourcePath, Lists.<MockResource> newArrayList());
+ assertTrue(result.isOk());
+
+ // Recompile should fail since the provided Foo is not compilable.
+ result = compileWithChanges(logger, runner, outbox, sourcePath,
+ Lists.<MockResource> newArrayList(nonCompilableFooResource));
+ assertFalse(result.isOk());
+
+ // Recompile with a modified entry point. This should fail again since Foo is still
+ // bad, but if transactionality protection failed on the minimalRebuildCache this compile will
+ // succeed because it will think that it has "already processed" Foo.
+ result = compileWithChanges(logger, runner, outbox, sourcePath,
+ Lists.<MockResource> newArrayList(referencesBarEntryPointResource));
+ assertFalse(result.isOk());
+ }
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+ // Make sure we're using a MemoryUnitCache.
+ System.setProperty(UnitCacheSingleton.GWT_PERSISTENTUNITCACHE, "false");
+ }
+
+ private Result compileWithChanges(TreeLogger logger, JobRunner runner, Outbox outbox,
+ File sourcePath, List<MockResource> changedResources) throws InterruptedException,
+ IOException {
+ // Wait 1 second so that any new file modification times are actually different.
+ Thread.sleep(1001);
+
+ // Write the Java/XML/etc resources that make up the test application.
+ writeResourcesTo(changedResources, sourcePath);
+
+ // Compile and return success status.
+ Map<String, String> bindingProperties = new HashMap<String, String>();
+ Job job = outbox.makeJob(bindingProperties, logger);
+ runner.submit(job);
+ return job.waitForResult();
+ }
+}
diff --git a/dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java b/dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java
index 5bc440a..4815126 100644
--- a/dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java
+++ b/dev/core/src/com/google/gwt/dev/MinimalRebuildCache.java
@@ -72,6 +72,24 @@
}
}
+ @SuppressWarnings({"rawtypes", "unchecked"})
+ private static void copyCollection(Collection fromCollection, Collection toCollection) {
+ toCollection.clear();
+ toCollection.addAll(fromCollection);
+ }
+
+ @SuppressWarnings({"rawtypes", "unchecked"})
+ private static void copyMap(Map fromMap, Map toMap) {
+ toMap.clear();
+ toMap.putAll(fromMap);
+ }
+
+ @SuppressWarnings({"rawtypes", "unchecked"})
+ private static void copyMultimap(Multimap fromMap, Multimap toMap) {
+ toMap.clear();
+ toMap.putAll(fromMap);
+ }
+
/**
* Diffs lastModifiedByResourcePath from the previous compile against currentResources from the
* current compile. modifiedResourcePaths is wiped and recreated to be a list of just the modified
@@ -356,6 +374,58 @@
return reachableTypeNames;
}
+ /**
+ * Replaces the contents of this cache with the contents of the given cache.
+ * <p>
+ * This operation should be kept fast as it will be called once per compile. At the moment it
+ * takes about 2.5% of the time in an incremental recompile. If wanting to recover this time in
+ * the future consider parallelizing the copy or grouping the values from hash tables with similar
+ * keys into a single data object and hashtable so that fewer references need to be replicated.
+ */
+ public void copyFrom(MinimalRebuildCache that) {
+ this.lastLinkedJsBytes = that.lastLinkedJsBytes;
+
+ this.intTypeIdGenerator.copyFrom(that.intTypeIdGenerator);
+ this.persistentPrettyNamerState.copyFrom(that.persistentPrettyNamerState);
+ this.immediateTypeRelations.copyFrom(that.immediateTypeRelations);
+
+ copyMap(that.compilationUnitTypeNameByNestedTypeName,
+ this.compilationUnitTypeNameByNestedTypeName);
+ copyMap(that.contentHashByGeneratedTypeName, this.contentHashByGeneratedTypeName);
+ copyMap(that.jsByTypeName, this.jsByTypeName);
+ copyMap(that.lastModifiedByDiskSourcePath, this.lastModifiedByDiskSourcePath);
+ copyMap(that.lastModifiedByResourcePath, this.lastModifiedByResourcePath);
+ copyMap(that.sourceMapsByTypeName, this.sourceMapsByTypeName);
+ copyMap(that.statementRangesByTypeName, this.statementRangesByTypeName);
+
+ copyMultimap(that.generatedCompilationUnitNamesByReboundTypeNames,
+ this.generatedCompilationUnitNamesByReboundTypeNames);
+ copyMultimap(that.nestedTypeNamesByUnitTypeName, this.nestedTypeNamesByUnitTypeName);
+ copyMultimap(that.rebinderTypeNamesByReboundTypeName,
+ this.rebinderTypeNamesByReboundTypeName);
+ copyMultimap(that.reboundTypeNamesByGeneratedCompilationUnitNames,
+ this.reboundTypeNamesByGeneratedCompilationUnitNames);
+ copyMultimap(that.reboundTypeNamesByInputResource, this.reboundTypeNamesByInputResource);
+ copyMultimap(that.referencedTypeNamesByTypeName, this.referencedTypeNamesByTypeName);
+ copyMultimap(that.typeNamesByReferencingTypeName, this.typeNamesByReferencingTypeName);
+
+ copyCollection(that.deletedCompilationUnitNames, this.deletedCompilationUnitNames);
+ copyCollection(that.deletedDiskSourcePaths, this.deletedDiskSourcePaths);
+ copyCollection(that.deletedResourcePaths, this.deletedResourcePaths);
+ copyCollection(that.dualJsoImplInterfaceNames, this.dualJsoImplInterfaceNames);
+ copyCollection(that.generatedArtifacts, this.generatedArtifacts);
+ copyCollection(that.jsoStatusChangedTypeNames, this.jsoStatusChangedTypeNames);
+ copyCollection(that.jsoTypeNames, this.jsoTypeNames);
+ copyCollection(that.modifiedCompilationUnitNames, this.modifiedCompilationUnitNames);
+ copyCollection(that.modifiedDiskSourcePaths, this.modifiedDiskSourcePaths);
+ copyCollection(that.modifiedResourcePaths, this.modifiedResourcePaths);
+ copyCollection(that.preambleTypeNames, this.preambleTypeNames);
+ copyCollection(that.rootTypeNames, this.rootTypeNames);
+ copyCollection(that.singleJsoImplInterfaceNames, this.singleJsoImplInterfaceNames);
+ copyCollection(that.sourceCompilationUnitNames, this.sourceCompilationUnitNames);
+ copyCollection(that.staleTypeNames, this.staleTypeNames);
+ }
+
public ArtifactSet getGeneratedArtifacts() {
return generatedArtifacts;
}
diff --git a/dev/core/src/com/google/gwt/dev/javac/UnitCacheSingleton.java b/dev/core/src/com/google/gwt/dev/javac/UnitCacheSingleton.java
index 6b7b330..2259a9e 100644
--- a/dev/core/src/com/google/gwt/dev/javac/UnitCacheSingleton.java
+++ b/dev/core/src/com/google/gwt/dev/javac/UnitCacheSingleton.java
@@ -25,10 +25,12 @@
*/
public class UnitCacheSingleton {
+ public static final String GWT_PERSISTENTUNITCACHE = "gwt.persistentunitcache";
+
/**
* The API must be enabled explicitly for persistent caching to be live.
*/
- private static final String configPropertyValue = System.getProperty("gwt.persistentunitcache",
+ private static final String configPropertyValue = System.getProperty(GWT_PERSISTENTUNITCACHE,
"true");
private static final boolean usePersistent = configPropertyValue.length() == 0
|| Boolean.parseBoolean(configPropertyValue);
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
index c8ba256..fa84cf9 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
@@ -78,6 +78,16 @@
private Multimap<String, String> immediateImplementedInterfacesByClass =
HashMultimap.create();
+ public void copyFrom(ImmediateTypeRelations that) {
+ this.immediateImplementedInterfacesByClass.clear();
+ this.immediateSuperclassesByClass.clear();
+ this.immediateSuperInterfacesByInterface.clear();
+
+ this.immediateImplementedInterfacesByClass.putAll(that.immediateImplementedInterfacesByClass);
+ this.immediateSuperclassesByClass.putAll(that.immediateSuperclassesByClass);
+ this.immediateSuperInterfacesByInterface.putAll(that.immediateSuperInterfacesByInterface);
+ }
+
@VisibleForTesting
public Map<String, String> getImmediateSuperclassesByClass() {
return immediateSuperclassesByClass;
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ResolveRuntimeTypeReferences.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ResolveRuntimeTypeReferences.java
index 5401414..1d67b1d 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ResolveRuntimeTypeReferences.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ResolveRuntimeTypeReferences.java
@@ -59,6 +59,12 @@
private final Map<String, Integer> typeIdByTypeName = Maps.newHashMap();
private int nextAvailableId = 0;
+ public void copyFrom(IntTypeIdGenerator that) {
+ this.nextAvailableId = that.nextAvailableId;
+ this.typeIdByTypeName.clear();
+ this.typeIdByTypeName.putAll(that.typeIdByTypeName);
+ }
+
public int getOrCreateTypeId(String typeName) {
if (typeIdByTypeName.containsKey(typeName)) {
return typeIdByTypeName.get(typeName);
diff --git a/dev/core/src/com/google/gwt/dev/js/JsPersistentPrettyNamer.java b/dev/core/src/com/google/gwt/dev/js/JsPersistentPrettyNamer.java
index a6a0efc..f045f11 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsPersistentPrettyNamer.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsPersistentPrettyNamer.java
@@ -42,6 +42,16 @@
private Map<String, String> prettyIdentByOriginalIdent = Maps.newHashMap();
private Set<String> usedPrettyIdents = Sets.newHashSet();
+
+ public void copyFrom(PersistentPrettyNamerState that) {
+ this.shortIdentCollisionCounts.clear();
+ this.prettyIdentByOriginalIdent.clear();
+ this.usedPrettyIdents.clear();
+
+ this.shortIdentCollisionCounts.addAll(that.shortIdentCollisionCounts);
+ this.prettyIdentByOriginalIdent.putAll(that.prettyIdentByOriginalIdent);
+ this.usedPrettyIdents.addAll(that.usedPrettyIdents);
+ }
}
@VisibleForTesting
diff --git a/dev/core/test/com/google/gwt/dev/CompilerTest.java b/dev/core/test/com/google/gwt/dev/CompilerTest.java
index 4328139..f1dcb7f 100644
--- a/dev/core/test/com/google/gwt/dev/CompilerTest.java
+++ b/dev/core/test/com/google/gwt/dev/CompilerTest.java
@@ -21,6 +21,7 @@
import com.google.gwt.dev.cfg.ModuleDefLoader;
import com.google.gwt.dev.cfg.ResourceLoader;
import com.google.gwt.dev.cfg.ResourceLoaders;
+import com.google.gwt.dev.javac.UnitCacheSingleton;
import com.google.gwt.dev.javac.testing.impl.JavaResourceBase;
import com.google.gwt.dev.javac.testing.impl.MockJavaResource;
import com.google.gwt.dev.javac.testing.impl.MockResource;
@@ -45,7 +46,6 @@
*/
public class CompilerTest extends ArgProcessorTestBase {
- public static final String GWT_PERSISTENTUNITCACHE = "gwt.persistentunitcache";
public static final String HELLO_MODULE = "com.google.gwt.sample.hello.Hello";
public static final String HELLO_MODULE_STACKMODE_STRIP =
"com.google.gwt.sample.hello.Hello_stackMode_strip";
@@ -1131,7 +1131,8 @@
File firstCompileWorkDir = Utility.makeTemporaryDirectory(null, "hellowork");
File secondCompileWorkDir = Utility.makeTemporaryDirectory(null, "hellowork");
- String oldPersistentUnitCacheValue = System.setProperty(GWT_PERSISTENTUNITCACHE, "false");
+ String oldPersistentUnitCacheValue =
+ System.setProperty(UnitCacheSingleton.GWT_PERSISTENTUNITCACHE, "false");
try {
options.addModuleName(topLevelModule);
options.setWarDir(new File(firstCompileWorkDir, "war"));
@@ -1156,9 +1157,9 @@
secondTimeOutput);
} finally {
if (oldPersistentUnitCacheValue == null) {
- System.clearProperty(GWT_PERSISTENTUNITCACHE);
+ System.clearProperty(UnitCacheSingleton.GWT_PERSISTENTUNITCACHE);
} else {
- System.setProperty(GWT_PERSISTENTUNITCACHE, oldPersistentUnitCacheValue);
+ System.setProperty(UnitCacheSingleton.GWT_PERSISTENTUNITCACHE, oldPersistentUnitCacheValue);
}
Util.recursiveDelete(firstCompileWorkDir, false);
Util.recursiveDelete(secondCompileWorkDir, false);
@@ -1223,7 +1224,7 @@
MinimalRebuildCache minimalRebuildCache, Set<String> expectedStaleTypeNames,
JsOutputOption output) throws IOException, UnableToCompleteException, InterruptedException {
// Make sure we're using a MemoryUnitCache.
- System.setProperty(GWT_PERSISTENTUNITCACHE, "false");
+ System.setProperty(UnitCacheSingleton.GWT_PERSISTENTUNITCACHE, "false");
// Wait 1 second so that any new file modification times are actually different.
Thread.sleep(1001);
PrintWriterTreeLogger logger = new PrintWriterTreeLogger();
