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()); + } +}