Fixes issue #2379; unstable generators should not crash the compiler or hosted mode.
- StandardRebindOracle is retrofitted to always cache rebind results, so that the same input type is never run twice in the same compilation.
- Doc on PropertyOracle updated to insist on stable results (StaticPropertyOracle does this now, but used to violate this; it would not have been possible to make StandardRebindOracle cache before).
- CompilationRebindOracle is removed since the caching behavior is now handled by StandardRebindOracle
- Unit test added to make sure multiple requests of the same type return a stable result.
This may also speed up hosted mode startup slightly in cases where the same type is requested many times through GWT.create().
Review by: spoon
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@3560 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/core/ext/PropertyOracle.java b/dev/core/src/com/google/gwt/core/ext/PropertyOracle.java
index 8f07ad4..188cc5a 100644
--- a/dev/core/src/com/google/gwt/core/ext/PropertyOracle.java
+++ b/dev/core/src/com/google/gwt/core/ext/PropertyOracle.java
@@ -23,7 +23,8 @@
/**
* Attempts to get a named deferred binding property. Throws
* <code>BadPropertyValueException</code> if the property is either
- * undefined or has a value that is unsupported.
+ * undefined or has a value that is unsupported. The result of invoking this
+ * method with the same <code>propertyName</code> must be stable.
*
* @param logger the current logger
* @param propertyName the name of the property
@@ -35,7 +36,8 @@
/**
* Attempts to get a named deferred binding property and returns the list of
* possible values. Throws <code>BadPropertyValueException</code> if the
- * property is undefined.
+ * property is undefined. The result of invoking this method with the same
+ * <code>propertyName</code> must be stable.
*
* @param logger the current logger
* @param propertyName the name of the property
diff --git a/dev/core/src/com/google/gwt/dev/GWTCompiler.java b/dev/core/src/com/google/gwt/dev/GWTCompiler.java
index 0a38598..beb2f7f 100644
--- a/dev/core/src/com/google/gwt/dev/GWTCompiler.java
+++ b/dev/core/src/com/google/gwt/dev/GWTCompiler.java
@@ -28,6 +28,7 @@
import com.google.gwt.dev.cfg.StaticPropertyOracle;
import com.google.gwt.dev.javac.CompilationState;
import com.google.gwt.dev.javac.CompilationUnit;
+import com.google.gwt.dev.jdt.RebindOracle;
import com.google.gwt.dev.jdt.RebindPermutationOracle;
import com.google.gwt.dev.jdt.WebModeCompilerFrontEnd;
import com.google.gwt.dev.jjs.JJSOptions;
@@ -51,9 +52,7 @@
import com.google.gwt.util.tools.ToolBase;
import java.io.File;
-import java.util.HashMap;
import java.util.HashSet;
-import java.util.Map;
import java.util.Set;
/**
@@ -104,64 +103,24 @@
}
}
- /**
- * Used to smartly deal with rebind across the production of an entire
- * permutation, including cache checking and recording the inputs and outputs
- * into a {@link Compilation}.
- */
- private class CompilationRebindOracle extends StandardRebindOracle {
-
- private final Map<String, String> cache = new HashMap<String, String>();
- private final StaticPropertyOracle propOracle;
-
- public CompilationRebindOracle(ArtifactSet generatorArtifacts,
- StaticPropertyOracle propOracle) {
- super(compilationState, propOracle, module, rules, genDir,
- generatorResourcesDir, generatorArtifacts);
- this.propOracle = propOracle;
- }
-
- public StaticPropertyOracle getPropertyOracle() {
- return propOracle;
- }
-
- /**
- * Overridden so that we can cache answers.
- */
- @Override
- public String rebind(TreeLogger logger, String in)
- throws UnableToCompleteException {
- String out = cache.get(in);
- if (out == null) {
- // Actually do the work, then cache it.
- //
- out = super.rebind(logger, in);
- cache.put(in, out);
- } else {
- // Was cached.
- //
- String msg = "Rebind answer for '" + in + "' found in cache " + out;
- logger.log(TreeLogger.DEBUG, msg, null);
- }
- return out;
- }
- }
-
private class DistillerRebindPermutationOracle implements
RebindPermutationOracle {
- private CompilationRebindOracle[] rebindOracles;
+ private StaticPropertyOracle[] propertyOracles;
+ private RebindOracle[] rebindOracles;
public DistillerRebindPermutationOracle(ArtifactSet generatorArtifacts,
PropertyPermutations perms) {
- rebindOracles = new CompilationRebindOracle[perms.size()];
+ propertyOracles = new StaticPropertyOracle[perms.size()];
+ rebindOracles = new RebindOracle[perms.size()];
Property[] orderedProps = perms.getOrderedProperties();
for (int i = 0; i < rebindOracles.length; ++i) {
String[] orderedPropValues = perms.getOrderedPropertyValues(i);
- StaticPropertyOracle propOracle = new StaticPropertyOracle(
- orderedProps, orderedPropValues);
- rebindOracles[i] = new CompilationRebindOracle(generatorArtifacts,
- propOracle);
+ propertyOracles[i] = new StaticPropertyOracle(orderedProps,
+ orderedPropValues);
+ rebindOracles[i] = new StandardRebindOracle(compilationState,
+ propertyOracles[i], module, rules, genDir, generatorResourcesDir,
+ generatorArtifacts);
}
}
@@ -174,7 +133,7 @@
Set<String> answers = new HashSet<String>();
- for (CompilationRebindOracle rebindOracle : rebindOracles) {
+ for (RebindOracle rebindOracle : rebindOracles) {
String resultTypeName = rebindOracle.rebind(logger, requestTypeName);
answers.add(resultTypeName);
}
@@ -185,7 +144,11 @@
return rebindOracles.length;
}
- public CompilationRebindOracle getRebindOracle(int permNumber) {
+ public StaticPropertyOracle getPropertyOracle(int permNumber) {
+ return propertyOracles[permNumber];
+ }
+
+ public RebindOracle getRebindOracle(int permNumber) {
return rebindOracles[permNumber];
}
}
@@ -416,9 +379,8 @@
PerfLogger.start("Compiling " + permCount + " permutations");
Permutation[] perms = new Permutation[permCount];
for (int i = 0; i < permCount; ++i) {
- CompilationRebindOracle rebindOracle = rpo.getRebindOracle(i);
- perms[i] = new Permutation(i, rebindOracle,
- rebindOracle.getPropertyOracle());
+ perms[i] = new Permutation(i, rpo.getRebindOracle(i),
+ rpo.getPropertyOracle(i));
}
PermutationCompiler permCompiler = new PermutationCompiler(logger, jjs,
perms);
diff --git a/dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java b/dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java
index 0ee638c..3cf0c7f 100644
--- a/dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java
+++ b/dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java
@@ -28,9 +28,11 @@
import java.io.File;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
import java.util.Set;
/**
@@ -125,6 +127,8 @@
private final ArtifactSet artifactSet;
+ private final Map<String, String> cache = new HashMap<String, String>();
+
private final CompilationState compilationState;
private final File genDir;
@@ -152,13 +156,16 @@
public String rebind(TreeLogger logger, String typeName)
throws UnableToCompleteException {
- logger = Messages.TRACE_TOPLEVEL_REBIND.branch(logger, typeName, null);
+ String result = cache.get(typeName);
+ if (result == null) {
+ logger = Messages.TRACE_TOPLEVEL_REBIND.branch(logger, typeName, null);
- Rebinder rebinder = new Rebinder();
- String result = rebinder.rebind(logger, typeName);
+ Rebinder rebinder = new Rebinder();
+ result = rebinder.rebind(logger, typeName);
+ cache.put(typeName, result);
- Messages.TRACE_TOPLEVEL_REBIND_RESULT.log(logger, result, null);
-
+ Messages.TRACE_TOPLEVEL_REBIND_RESULT.log(logger, result, null);
+ }
return result;
}
}
diff --git a/user/test/com/google/gwt/dev/jjs/CompilerSuite.gwt.xml b/user/test/com/google/gwt/dev/jjs/CompilerSuite.gwt.xml
index c78dea1..da8fd47 100644
--- a/user/test/com/google/gwt/dev/jjs/CompilerSuite.gwt.xml
+++ b/user/test/com/google/gwt/dev/jjs/CompilerSuite.gwt.xml
@@ -13,5 +13,9 @@
<!-- limitations under the License. -->
<module>
- <source path='test'/>
+ <source path='test' />
+ <generate-with class="com.google.gwt.dev.jjs.UnstableGenerator">
+ <when-type-assignable
+ class="com.google.gwt.dev.jjs.test.UnstableGeneratorTest.UnstableResult" />
+ </generate-with>
</module>
diff --git a/user/test/com/google/gwt/dev/jjs/CompilerSuite.java b/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
index 9cec9d4..79e0b26 100644
--- a/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
+++ b/user/test/com/google/gwt/dev/jjs/CompilerSuite.java
@@ -38,6 +38,7 @@
import com.google.gwt.dev.jjs.test.MiscellaneousTest;
import com.google.gwt.dev.jjs.test.NativeLongTest;
import com.google.gwt.dev.jjs.test.ObjectIdentityTest;
+import com.google.gwt.dev.jjs.test.UnstableGeneratorTest;
import com.google.gwt.dev.jjs.test.VarargsTest;
import com.google.gwt.junit.tools.GWTTestSuite;
@@ -75,6 +76,7 @@
suite.addTestSuite(MiscellaneousTest.class);
suite.addTestSuite(NativeLongTest.class);
suite.addTestSuite(ObjectIdentityTest.class);
+ suite.addTestSuite(UnstableGeneratorTest.class);
suite.addTestSuite(VarargsTest.class);
// $JUnit-END$
diff --git a/user/test/com/google/gwt/dev/jjs/UnstableGenerator.java b/user/test/com/google/gwt/dev/jjs/UnstableGenerator.java
new file mode 100644
index 0000000..4ab21dd
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/UnstableGenerator.java
@@ -0,0 +1,65 @@
+/*
+ * Copyright 2008 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.jjs;
+
+import com.google.gwt.core.ext.Generator;
+import com.google.gwt.core.ext.GeneratorContext;
+import com.google.gwt.core.ext.TreeLogger;
+import com.google.gwt.core.ext.UnableToCompleteException;
+import com.google.gwt.core.ext.typeinfo.JClassType;
+import com.google.gwt.core.ext.typeinfo.NotFoundException;
+import com.google.gwt.core.ext.typeinfo.TypeOracle;
+import com.google.gwt.dev.jjs.test.UnstableGeneratorTest;
+
+import java.io.PrintWriter;
+
+/**
+ * Generates unstable results than change on each invocation.
+ */
+public class UnstableGenerator extends Generator {
+
+ private static int counter = 0;
+
+ @Override
+ public String generate(TreeLogger logger, GeneratorContext context,
+ String typeName) throws UnableToCompleteException {
+ try {
+ ++counter;
+ TypeOracle typeOracle = context.getTypeOracle();
+ JClassType inputType = typeOracle.getType(typeName);
+ JClassType intfType = typeOracle.getType(UnstableGeneratorTest.UnstableResult.class.getName().replace(
+ '$', '.'));
+ String packageName = inputType.getPackage().getName();
+ String className = inputType.getName().replace('.', '_') + "Impl"
+ + counter;
+ PrintWriter writer = context.tryCreate(logger, packageName, className);
+ if (writer == null) {
+ return null;
+ }
+ writer.println("package " + packageName + ";");
+ writer.println("public class " + className + " implements "
+ + intfType.getQualifiedSourceName() + " {");
+ writer.println(" public String get() { return \"foo" + counter + "\"; }");
+ writer.println("}");
+ context.commit(logger, writer);
+ return (packageName.length() == 0) ? className
+ : (packageName + "." + className);
+ } catch (NotFoundException e) {
+ logger.log(TreeLogger.ERROR, "Unable to find required client type", e);
+ throw new UnableToCompleteException();
+ }
+ }
+}
diff --git a/user/test/com/google/gwt/dev/jjs/test/UnstableGeneratorTest.java b/user/test/com/google/gwt/dev/jjs/test/UnstableGeneratorTest.java
new file mode 100644
index 0000000..b3b7e54
--- /dev/null
+++ b/user/test/com/google/gwt/dev/jjs/test/UnstableGeneratorTest.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright 2008 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.jjs.test;
+
+import com.google.gwt.core.client.GWT;
+import com.google.gwt.junit.client.GWTTestCase;
+
+/**
+ * Tests that we get a consistent result for a single compilation even in the
+ * face of an unstable generator.
+ */
+public class UnstableGeneratorTest extends GWTTestCase {
+
+ /**
+ * Marker interface for generating an unstable class.
+ */
+ public interface UnstableResult {
+ String get();
+ }
+
+ @Override
+ public String getModuleName() {
+ return "com.google.gwt.dev.jjs.CompilerSuite";
+ }
+
+ public void testMultipleRebindsWithUnstableGenerator() {
+ String firstResult = GWT.<UnstableResult> create(UnstableResult.class).get();
+ assertEquals(firstResult, GWT.<UnstableResult> create(UnstableResult.class).get());
+ assertEquals(firstResult, GWT.<UnstableResult> create(UnstableResult.class).get());
+ assertEquals(firstResult, GWT.<UnstableResult> create(UnstableResult.class).get());
+ assertEquals(firstResult, GWT.<UnstableResult> create(UnstableResult.class).get());
+ }
+}
