Skip to content

Commit

Permalink
Fixes ignored module property value changes in SDM.
Browse files Browse the repository at this point in the history 
Previously SDM was failing to invalidate its MinimalRebuildCache when 
property values were edited in .gwt.xml module files.

It did notice that the module files had been edited and treated it as a
recompile condition and it did retrieve/reuse a MinimalRebuildCache 
as keyed by the current set of binding properties. But that key 
construction was based on just the current set of binding properties as
being selected in the browser and ignored manual edits to them as 
performed in .gwt.xml file changes.

Change-Id: I952d42817abdc2000849f6b1f7b69de5b9420443
  • Loading branch information
@stalcup
stalcup committed Feb 13, 2015
1 parent 0e9a41f commit d174544
Show file tree
Hide file tree
Showing 6 changed files with 217 additions and 39 deletions.
41 changes: 30 additions & 11 deletions dev/codeserver/java/com/google/gwt/dev/codeserver/Recompiler.java
View file
Expand Up @@ -33,6 +33,8 @@
import com.google.gwt.dev.cfg.ConfigurationProperty;
import com.google.gwt.dev.cfg.ModuleDef;
import com.google.gwt.dev.cfg.ModuleDefLoader;
import com.google.gwt.dev.cfg.PropertyPermutations;
import com.google.gwt.dev.cfg.PropertyPermutations.PermutationDescription;
import com.google.gwt.dev.cfg.ResourceLoader;
import com.google.gwt.dev.cfg.ResourceLoaders;
import com.google.gwt.dev.codeserver.Job.Result;
Expand All @@ -52,6 +54,7 @@

import java.io.File;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;

Expand All @@ -73,7 +76,7 @@ public class Recompiler {

private final AtomicReference<CompileDir> lastBuild = new AtomicReference<CompileDir>();

private InputSummary lastBuildInput;
private InputSummary previousInputSummary;

private CompileDir publishedCompileDir;
private final AtomicReference<ResourceLoader> resourceLoader =
Expand Down Expand Up @@ -305,43 +308,60 @@ private boolean doCompile(TreeLogger compileLogger, CompileDir compileDir, Job j
// We need to generate the stub before restricting permutations
String recompileJs = generateModuleRecompileJs(module, compileLogger);

Map<String, String> bindingProperties = restrictPermutations(compileLogger, module,
job.getBindingProperties());
Map<String, String> bindingProperties =
restrictPermutations(compileLogger, module, job.getBindingProperties());

// Propagates module rename.
String newModuleName = module.getName();
outputModuleName.set(newModuleName);

// Check if the module definition + job specific binding property restrictions expanded to more
// than permutation.
PropertyPermutations propertyPermutations =
new PropertyPermutations(module.getProperties(), module.getActiveLinkerNames());
List<PropertyPermutations> permutationPropertySets = propertyPermutations.collapseProperties();
if (options.isIncrementalCompileEnabled() && permutationPropertySets.size() > 1) {
compileLogger.log(Type.INFO,
"Current binding properties are expanding to more than one permutation "
+ "but incremental compilation requires that each compile operate on only "
+ "one permutation.");
job.setCompileStrategy(CompileStrategy.SKIPPED);
return true;
}
PropertyPermutations permutationPropertySet = permutationPropertySets.get(0);
PermutationDescription permutationDescription =
permutationPropertySet.getPermutationDescription(0);

// Check if we can skip the compile altogether.
InputSummary input = new InputSummary(bindingProperties, module);
if (input.equals(lastBuildInput)) {
InputSummary inputSummary = new InputSummary(bindingProperties, module);
if (inputSummary.equals(previousInputSummary)) {
compileLogger.log(Type.INFO, "skipped compile because no input files have changed");
job.setCompileStrategy(CompileStrategy.SKIPPED);
return true;
}
// Force a recompile if we don't succeed.
lastBuildInput = null;
previousInputSummary = null;

job.onProgress("Compiling");
// TODO: use speed tracer to get more compiler events?

CompilerOptions runOptions = new CompilerOptionsImpl(compileDir, newModuleName, options);
compilerContext = compilerContextBuilder.options(runOptions).build();

MinimalRebuildCache minimalRebuildCache = new NullRebuildCache();
if (options.isIncrementalCompileEnabled()) {
// Returns a copy of the intended cache, which is safe to modify in this compile.
minimalRebuildCache = minimalRebuildCacheManager.getCache(inputModuleName, bindingProperties);
minimalRebuildCache =
minimalRebuildCacheManager.getCache(inputModuleName, permutationDescription);
}
job.setCompileStrategy(minimalRebuildCache.isPopulated() ? CompileStrategy.INCREMENTAL
: CompileStrategy.FULL);

boolean success = new Compiler(runOptions, minimalRebuildCache).run(compileLogger, module);
if (success) {
publishedCompileDir = compileDir;
lastBuildInput = input;
previousInputSummary = inputSummary;
if (options.isIncrementalCompileEnabled()) {
minimalRebuildCacheManager.putCache(inputModuleName, bindingProperties,
minimalRebuildCacheManager.putCache(inputModuleName, permutationDescription,
minimalRebuildCache);
}
String moduleName = outputModuleName.get();
Expand Down Expand Up @@ -505,7 +525,6 @@ private ModuleDef loadModule(TreeLogger logger) throws UnableToCompleteException
/**
* Restricts the compiled permutations by applying the given binding properties, if possible.
* In some cases, a different binding may be chosen instead.
* @return a map of the actual properties used.
*/
private Map<String, String> restrictPermutations(TreeLogger logger, ModuleDef moduleDef,
Map<String, String> bindingProperties) {
Expand Down
105 changes: 105 additions & 0 deletions dev/codeserver/javatests/com/google/gwt/dev/codeserver/RecompilerTest.java
View file
Expand Up @@ -32,7 +32,10 @@
import junit.framework.TestCase;

import java.io.File;
import java.io.FilenameFilter;
import java.io.IOException;
import java.util.Arrays;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -42,6 +45,23 @@
*/
public class RecompilerTest extends TestCase {

private static File findCompiledJsFile(Result result) {
File outputDir = new File(result.outputDir.getWarDir(), result.outputModuleName);
File[] files = outputDir.listFiles(new FilenameFilter() {
@Override
public boolean accept(File dir, String name) {
return name.endsWith(".js") && !name.endsWith("nocache.js");
}
});
Arrays.sort(files, new Comparator<File>() {
@Override
public int compare(File thisFile, File thatFile) {
return -Long.compare(thisFile.length(), thatFile.length());
}
});
return files[0];
}

private static void writeResourcesTo(List<MockResource> resources, File dir) throws IOException {
for (MockResource applicationResource : resources) {
File resourceFile =
Expand Down Expand Up @@ -98,6 +118,44 @@ private static void writeResourcesTo(List<MockResource> resources, File dir) thr
"<entry-point class='com.foo.TestEntryPoint'/>",
"</module>");

private MockResource propertyIsFooModuleResource =
JavaResourceBase.createMockResource("com/foo/PropertyModule.gwt.xml",
"<module>",
"<source path=''/>",
"<entry-point class='com.foo.PropertyEntryPoint'/>",
"<define-property name=\"target\" values=\"foo,bar\" />",
"<set-property name=\"target\" value=\"foo\" />",
"<replace-with class=\"com.foo.Foo\">",
" <when-type-is class=\"com.foo.Bar\"/>",
" <when-property-is name=\"target\" value=\"foo\"/>",
"</replace-with>",
"</module>");

private MockResource propertyIsBarModuleResource =
JavaResourceBase.createMockResource("com/foo/PropertyModule.gwt.xml",
"<module>",
"<source path=''/>",
"<entry-point class='com.foo.PropertyEntryPoint'/>",
"<define-property name=\"target\" values=\"foo,bar\" />",
"<set-property name=\"target\" value=\"bar\" />",
"<replace-with class=\"com.foo.Foo\">",
" <when-type-is class=\"com.foo.Bar\"/>",
" <when-property-is name=\"target\" value=\"foo\"/>",
"</replace-with>",
"</module>");

private MockJavaResource performsRebindEntryPointResource =
JavaResourceBase.createMockJavaResource("com.foo.PropertyEntryPoint",
"package com.foo;",
"import com.google.gwt.core.client.EntryPoint;",
"import com.google.gwt.core.client.GWT;",
"public class PropertyEntryPoint implements EntryPoint {",
" @Override",
" public void onModuleLoad() {",
" GWT.create(Bar.class);",
" }",
"}");

public void testIncrementalRecompile_compileErrorDoesntCorruptMinimalRebuildCache()
throws UnableToCompleteException, IOException, InterruptedException {
PrintWriterTreeLogger logger = new PrintWriterTreeLogger();
Expand Down Expand Up @@ -144,6 +202,53 @@ public void testIncrementalRecompile_compileErrorDoesntCorruptMinimalRebuildCach
assertFalse(result.isOk());
}

public void testIncrementalRecompile_modulePropertyEditsWork() 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.PropertyModule"});

// Prepare the basic resources in the test application.
List<MockResource> originalResources = Lists.newArrayList(propertyIsFooModuleResource,
performsRebindEntryPointResource, barReferencesBazResource, bazReferencesFooResource,
fooResource);
writeResourcesTo(originalResources, sourcePath);

File baseCacheDir = Files.createTempDir();
UnitCache unitCache = UnitCacheSingleton.get(logger, null, baseCacheDir);
MinimalRebuildCacheManager minimalRebuildCacheManager =
new MinimalRebuildCacheManager(logger, baseCacheDir);
Recompiler recompiler = new Recompiler(OutboxDir.create(Files.createTempDir(), logger), null,
"com.foo.PropertyModule", options, unitCache, minimalRebuildCacheManager);
Outbox outbox = new Outbox("Transactional Cache", recompiler, options, logger);
OutboxTable outboxes = new OutboxTable();
outboxes.addOutbox(outbox);
JobRunner runner = new JobRunner(new JobEventTable(), minimalRebuildCacheManager);

// Perform a first compile with configuration to rebind Bar to Foo.
Result result =
compileWithChanges(logger, runner, outbox, sourcePath, Lists.<MockResource> newArrayList());
assertTrue(result.isOk());
File compiledJsFile1 = findCompiledJsFile(result);

// Perform a second compile with a changed property value that will result in NOT rebinding Bar
// to Foo.
result = compileWithChanges(logger, runner, outbox, sourcePath,
Lists.<MockResource> newArrayList(propertyIsBarModuleResource));
assertTrue(result.isOk());
File compiledJsFile2 = findCompiledJsFile(result);

// The compiled Js files are different files on disk and their contents are not the same as
// evidenced by their names being different (the names are a hash of content).
assertFalse(compiledJsFile1.equals(compiledJsFile2));
assertFalse(compiledJsFile1.getName().equals(compiledJsFile2.getName()));
}

@Override
protected void setUp() throws Exception {
super.setUp();
Expand Down
42 changes: 22 additions & 20 deletions dev/core/src/com/google/gwt/dev/MinimalRebuildCacheManager.java
View file
Expand Up @@ -14,6 +14,7 @@
package com.google.gwt.dev;

import com.google.gwt.core.ext.TreeLogger;
import com.google.gwt.dev.cfg.PropertyPermutations.PermutationDescription;
import com.google.gwt.dev.util.CompilerVersion;
import com.google.gwt.thirdparty.guava.common.annotations.VisibleForTesting;
import com.google.gwt.thirdparty.guava.common.cache.Cache;
Expand All @@ -32,7 +33,6 @@
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
Expand Down Expand Up @@ -87,15 +87,15 @@ public synchronized void deleteCaches() {
* If it is still not found a new empty cache will be returned.
*/
public synchronized MinimalRebuildCache getCache(String moduleName,
Map<String, String> bindingProperties) {
String cacheName = computeMinimalRebuildCacheName(moduleName, bindingProperties);
PermutationDescription permutationDescription) {
String cacheName = computeMinimalRebuildCacheName(moduleName, permutationDescription);

MinimalRebuildCache minimalRebuildCache = minimalRebuildCachesByName.getIfPresent(cacheName);

// If there's no cache already in memory, try to load a cache from disk.
if (minimalRebuildCache == null && haveCacheDir()) {
// Might return null.
minimalRebuildCache = syncReadDiskCache(moduleName, bindingProperties);
minimalRebuildCache = syncReadDiskCache(moduleName, permutationDescription);
if (minimalRebuildCache != null) {
minimalRebuildCachesByName.put(cacheName, minimalRebuildCache);
}
Expand All @@ -119,11 +119,12 @@ public synchronized MinimalRebuildCache getCache(String moduleName,
* <p>
* A copy of the cache will be lazily persisted to disk as well.
*/
public synchronized void putCache(String moduleName, Map<String, String> bindingProperties,
public synchronized void putCache(String moduleName,
PermutationDescription permutationDescription,
MinimalRebuildCache knownGoodMinimalRebuildCache) {
syncPutMemoryCache(moduleName, bindingProperties, knownGoodMinimalRebuildCache);
syncPutMemoryCache(moduleName, permutationDescription, knownGoodMinimalRebuildCache);
if (haveCacheDir()) {
enqueueAsyncWriteDiskCache(moduleName, bindingProperties, knownGoodMinimalRebuildCache);
enqueueAsyncWriteDiskCache(moduleName, permutationDescription, knownGoodMinimalRebuildCache);
}
}

Expand Down Expand Up @@ -151,13 +152,13 @@ public Void call() {
*/
@VisibleForTesting
synchronized Future<MinimalRebuildCache> enqueueAsyncReadDiskCache(final String moduleName,
final Map<String, String> bindingProperties) {
final PermutationDescription permutationDescription) {
return executorService.submit(new Callable<MinimalRebuildCache>() {
@Override
public MinimalRebuildCache call() {
// Find the cache file unique to this module, binding properties and working directory.
File minimalRebuildCacheFile =
computeMinimalRebuildCacheFile(moduleName, bindingProperties);
computeMinimalRebuildCacheFile(moduleName, permutationDescription);

// If the file exists.
if (minimalRebuildCacheFile.exists()) {
Expand Down Expand Up @@ -200,12 +201,13 @@ public MinimalRebuildCache call() {
*/
@VisibleForTesting
synchronized Future<Void> enqueueAsyncWriteDiskCache(final String moduleName,
final Map<String, String> bindingProperties, final MinimalRebuildCache minimalRebuildCache) {
final PermutationDescription permutationDescription,
final MinimalRebuildCache minimalRebuildCache) {
return executorService.submit(new Callable<Void>() {
@Override
public Void call() {
File oldMinimalRebuildCacheFile =
computeMinimalRebuildCacheFile(moduleName, bindingProperties);
computeMinimalRebuildCacheFile(moduleName, permutationDescription);
File newMinimalRebuildCacheFile =
new File(oldMinimalRebuildCacheFile.getAbsoluteFile() + ".new");

Expand Down Expand Up @@ -252,24 +254,24 @@ boolean shutdown() throws InterruptedException {
*/
@VisibleForTesting
synchronized MinimalRebuildCache syncReadDiskCache(String moduleName,
Map<String, String> bindingProperties) {
return Futures.getUnchecked(enqueueAsyncReadDiskCache(moduleName, bindingProperties));
PermutationDescription permutationDescription) {
return Futures.getUnchecked(enqueueAsyncReadDiskCache(moduleName, permutationDescription));
}

private File computeMinimalRebuildCacheFile(String moduleName,
Map<String, String> bindingProperties) {
PermutationDescription permutationDescription) {
return new File(minimalRebuildCacheDir,
computeMinimalRebuildCacheName(moduleName, bindingProperties));
computeMinimalRebuildCacheName(moduleName, permutationDescription));
}

private String computeMinimalRebuildCacheName(String moduleName,
Map<String, String> bindingProperties) {
PermutationDescription permutationDescription) {
String currentWorkingDirectory = System.getProperty("user.dir");
String compilerVersionHash = CompilerVersion.getHash();
String bindingPropertiesString = bindingProperties.toString();
String permutationDescriptionString = permutationDescription.toString();

String consistentHash = StringUtils.toHexString(Md5Utils.getMd5Digest((
compilerVersionHash + moduleName + currentWorkingDirectory + bindingPropertiesString)
compilerVersionHash + moduleName + currentWorkingDirectory + permutationDescriptionString)
.getBytes()));
return REBUILD_CACHE_PREFIX + "-" + consistentHash;
}
Expand All @@ -282,9 +284,9 @@ private void syncDeleteMemoryCaches() {
minimalRebuildCachesByName.invalidateAll();
}

private void syncPutMemoryCache(String moduleName, Map<String, String> bindingProperties,
private void syncPutMemoryCache(String moduleName, PermutationDescription permutationDescription,
MinimalRebuildCache knownGoodMinimalRebuildCache) {
String cacheName = computeMinimalRebuildCacheName(moduleName, bindingProperties);
String cacheName = computeMinimalRebuildCacheName(moduleName, permutationDescription);
minimalRebuildCachesByName.put(cacheName, knownGoodMinimalRebuildCache);
}
}
4 changes: 2 additions & 2 deletions dev/core/src/com/google/gwt/dev/Precompile.java
View file
Expand Up @@ -204,8 +204,8 @@ static List<PropertyPermutations> getCollapsedPermutations(ModuleDef module) {

static AbstractCompiler getCompiler(ModuleDef module) {
ConfigurationProperty compilerClassProp =
module.getProperties().createConfiguration("x.compiler.class", false);
String compilerClassName = compilerClassProp.getValue();
module.getProperties().findConfigProp("x.compiler.class");
String compilerClassName = compilerClassProp != null ? compilerClassProp.getValue() : null;
if (compilerClassName == null || compilerClassName.length() == 0) {
return new JavaScriptCompiler();
}
Expand Down

0 comments on commit d174544

Please sign in to comment.