Fixes memory leak in JdtCompiler.
Makes sure that INameEnvironmentImpl does not retain unnecessary
memory reachable from JdtCompiler. Also removes unused
AdditionalTypeProviderDelegate.
Bug: #9596
Change-Id: I98f0629425f6c0eb6f23d08eda0d8d0e981483c9
Bug-Link: https://github.com/gwtproject/gwt/issues/9596
diff --git a/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java b/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
index d64f1ff..39f07d0 100644
--- a/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
+++ b/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
@@ -19,7 +19,6 @@
import com.google.gwt.core.ext.TreeLogger.Type;
import com.google.gwt.core.ext.UnableToCompleteException;
import com.google.gwt.dev.CompilerContext;
-import com.google.gwt.dev.javac.JdtCompiler.AdditionalTypeProviderDelegate;
import com.google.gwt.dev.javac.JdtCompiler.UnitProcessor;
import com.google.gwt.dev.javac.typemodel.TypeOracle;
import com.google.gwt.dev.jjs.CorrelationFactory.DummyCorrelationFactory;
@@ -206,13 +205,11 @@
private CompilerContext compilerContext;
- public CompileMoreLater(
- CompilerContext compilerContext, AdditionalTypeProviderDelegate delegate) {
+ public CompileMoreLater(CompilerContext compilerContext) {
this.compilerContext = compilerContext;
this.compiler = new JdtCompiler(
compilerContext, new UnitProcessorImpl());
this.suppressErrors = !compilerContext.getOptions().isStrict();
- compiler.setAdditionalTypeProviderDelegate(delegate);
}
/**
@@ -459,24 +456,12 @@
*
* @throws UnableToCompleteException if the compiler aborts (not a normal compile error).
*/
- public static CompilationState buildFrom(
- TreeLogger logger, CompilerContext compilerContext, Set<Resource> resources)
- throws UnableToCompleteException {
- return buildFrom(logger, compilerContext, resources, null);
- }
-
- /**
- * Compiles the given source files and adds them to the CompilationState. See
- * {@link CompileMoreLater#compile} for details.
- *
- * @throws UnableToCompleteException if the compiler aborts (not a normal compile error).
- */
public static CompilationState buildFrom(TreeLogger logger, CompilerContext compilerContext,
- Set<Resource> resources, AdditionalTypeProviderDelegate delegate)
+ Set<Resource> resources)
throws UnableToCompleteException {
Event event = SpeedTracerLogger.start(DevModeEventType.CSB_BUILD_FROM_ORACLE);
try {
- return instance.doBuildFrom(logger, compilerContext, resources, delegate);
+ return instance.doBuildFrom(logger, compilerContext, resources);
} finally {
event.end();
}
@@ -489,8 +474,7 @@
* TODO: maybe use a finer brush than to synchronize the whole thing.
*/
public synchronized CompilationState doBuildFrom(TreeLogger logger,
- CompilerContext compilerContext, Set<Resource> resources,
- AdditionalTypeProviderDelegate compilerDelegate)
+ CompilerContext compilerContext, Set<Resource> resources)
throws UnableToCompleteException {
UnitCache unitCache = compilerContext.getUnitCache();
assert unitCache != null : "CompilerContext should always contain a unit cache.";
@@ -501,7 +485,7 @@
// Units we don't want to rebuild unless we have to.
Map<CompilationUnitBuilder, CompilationUnit> cachedUnits = Maps.newIdentityHashMap();
- CompileMoreLater compileMoreLater = new CompileMoreLater(compilerContext, compilerDelegate);
+ CompileMoreLater compileMoreLater = new CompileMoreLater(compilerContext);
// For each incoming Java source file...
for (Resource resource : resources) {
@@ -588,12 +572,6 @@
return new ContentId(Shared.getTypeName(resource), Util.computeStrongName(content));
}
- public CompilationState doBuildFrom(
- TreeLogger logger, CompilerContext compilerContext, Set<Resource> resources)
- throws UnableToCompleteException {
- return doBuildFrom(logger, compilerContext, resources, null);
- }
-
/**
* Compile new generated units into an existing state.
*
diff --git a/dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java b/dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java
index 88fc310..911bd5a 100644
--- a/dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java
+++ b/dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java
@@ -95,31 +95,6 @@
* Manages the process of compiling {@link CompilationUnit}s.
*/
public class JdtCompiler {
- /**
- * Provides hooks for changing the behavior of the JdtCompiler when unknown
- * types are encountered during compilation. Currently used for allowing
- * external tools to provide source lazily when undefined references appear.
- */
- public interface AdditionalTypeProviderDelegate {
- /**
- * Checks for additional packages which may contain additional compilation
- * units.
- *
- * @param slashedPackageName the '/' separated name of the package to find
- * @return <code>true</code> if such a package exists
- */
- boolean doFindAdditionalPackage(String slashedPackageName);
-
- /**
- * Finds a new compilation unit on-the-fly for the requested type, if there
- * is an alternate mechanism for doing so.
- *
- * @param binaryName the binary name of the requested type
- * @return a unit answering the name, or <code>null</code> if no such unit
- * can be created
- */
- GeneratedUnit doFindAdditionalType(String binaryName);
- }
/**
* A default processor that simply collects build units.
@@ -262,7 +237,8 @@
private int abortCount = 0;
public CompilerImpl(TreeLogger logger, CompilerOptions compilerOptions) {
- super(new INameEnvironmentImpl(), DefaultErrorHandlingPolicies.proceedWithAllProblems(),
+ super(new INameEnvironmentImpl(packages, internalTypes),
+ DefaultErrorHandlingPolicies.proceedWithAllProblems(),
compilerOptions, new ICompilerRequestorImpl(), new DefaultProblemFactory(
Locale.getDefault()));
this.logger = logger;
@@ -376,7 +352,26 @@
/**
* How JDT receives files from the environment.
*/
- private class INameEnvironmentImpl implements INameEnvironment {
+ private static class INameEnvironmentImpl implements INameEnvironment {
+
+ /**
+ * Remembers types that have been found in the classpath or not found at all to avoid
+ * unnecessary resource scanning.
+ */
+ private final Map<String, NameEnvironmentAnswer> cachedClassPathAnswerByInternalName =
+ Maps.newHashMap();
+
+ private final Set<String> packages;
+
+ private final Set<String> notPackages = new HashSet<String>();
+
+ private final Map<String, CompiledClass> internalTypes;
+
+ public INameEnvironmentImpl(Set<String> packages, Map<String, CompiledClass> internalTypes) {
+ this.packages = packages;
+ this.internalTypes = internalTypes;
+ }
+
@Override
public void cleanup() {
}
@@ -401,11 +396,6 @@
return cachedAnswer;
}
- NameEnvironmentAnswer additionalProviderAnswer = findTypeInAdditionalProvider(internalName);
- if (additionalProviderAnswer != null) {
- return additionalProviderAnswer;
- }
-
NameEnvironmentAnswer classPathAnswer = findTypeInClassPath(internalName);
if (classPathAnswer != null) {
@@ -520,19 +510,6 @@
}
}
- private NameEnvironmentAnswer findTypeInAdditionalProvider(String internalName) {
- if (additionalTypeProviderDelegate == null) {
- return null;
- }
-
- GeneratedUnit unit = additionalTypeProviderDelegate.doFindAdditionalType(internalName);
- if (unit == null) {
- return null;
- }
-
- return new NameEnvironmentAnswer(new Adapter(CompilationUnitBuilder.create(unit)), null);
- }
-
private NameEnvironmentAnswer findTypeInClassPath(String internalName) {
// If the class was previously queried here return the cached result.
@@ -611,14 +588,9 @@
if (notPackages.contains(slashedPackageName)) {
return false;
}
- if ((additionalTypeProviderDelegate != null && additionalTypeProviderDelegate
- .doFindAdditionalPackage(slashedPackageName))) {
- addPackages(slashedPackageName);
- return true;
- }
// Include class loader check for binary-only annotations.
if (caseSensitivePathExists(slashedPackageName)) {
- addPackages(slashedPackageName);
+ addPackages(packages, slashedPackageName);
return true;
} else {
notPackages.add(slashedPackageName);
@@ -765,21 +737,12 @@
}
}
- private AdditionalTypeProviderDelegate additionalTypeProviderDelegate;
-
/**
* Maps internal names to compiled classes.
*/
private final Map<String, CompiledClass> internalTypes = new HashMap<String, CompiledClass>();
/**
- * Remembers types that have been found in the classpath or not found at all to avoid unnecessary
- * resource scanning.
- */
- private final Map<String, NameEnvironmentAnswer> cachedClassPathAnswerByInternalName =
- Maps.newHashMap();
-
- /**
* Remembers types that where not found during resolution to avoid unnecessary file scanning.
*/
private final Set<String> unresolvableReferences = Sets.newHashSet();
@@ -789,8 +752,6 @@
*/
private transient CompilerImpl compilerImpl;
- private final Set<String> notPackages = new HashSet<String>();
-
private final Set<String> packages = new HashSet<String>();
private final UnitProcessor processor;
@@ -1070,10 +1031,6 @@
return typeBinding;
}
- public void setAdditionalTypeProviderDelegate(AdditionalTypeProviderDelegate newDelegate) {
- additionalTypeProviderDelegate = newDelegate;
- }
-
/**
* Sets whether the compiler should remove unused imports.
*/
@@ -1088,6 +1045,10 @@
}
private void addPackages(String slashedPackageName) {
+ addPackages(packages, slashedPackageName);
+ }
+
+ private static void addPackages(Set<String> packages, String slashedPackageName) {
while (packages.add(slashedPackageName)) {
int pos = slashedPackageName.lastIndexOf('/');
if (pos > 0) {
diff --git a/dev/core/src/com/google/gwt/dev/javac/testing/GeneratorContextBuilder.java b/dev/core/src/com/google/gwt/dev/javac/testing/GeneratorContextBuilder.java
index f0f512d..6a0ef30 100644
--- a/dev/core/src/com/google/gwt/dev/javac/testing/GeneratorContextBuilder.java
+++ b/dev/core/src/com/google/gwt/dev/javac/testing/GeneratorContextBuilder.java
@@ -107,7 +107,7 @@
private CompilationState buildCompilationState() throws UnableToCompleteException {
TreeLogger logger = treeLogger != null ? treeLogger : createLogger();
- return new CompilationStateBuilder().doBuildFrom(logger, compilerContext, resources, null);
+ return new CompilationStateBuilder().doBuildFrom(logger, compilerContext, resources);
}
private TreeLogger createLogger() {
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/AdditionalTypeProviderDelegateTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/AdditionalTypeProviderDelegateTest.java
deleted file mode 100644
index 3609782..0000000
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/AdditionalTypeProviderDelegateTest.java
+++ /dev/null
@@ -1,242 +0,0 @@
-/*
- * Copyright 2010 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.impl;
-
-import com.google.gwt.core.ext.TreeLogger;
-import com.google.gwt.core.ext.UnableToCompleteException;
-import com.google.gwt.dev.javac.GeneratedUnit;
-import com.google.gwt.dev.javac.JdtCompiler.AdditionalTypeProviderDelegate;
-import com.google.gwt.dev.javac.testing.impl.MockJavaResource;
-import com.google.gwt.dev.jjs.ast.JDeclaredType;
-import com.google.gwt.dev.jjs.ast.JMethod;
-import com.google.gwt.dev.jjs.ast.JProgram;
-
-/**
- * Tests that a AdditionalTypeProviderDelegate correctly gets control when an unknown
- * class is found, and that source for an unknown class gets correctly parsed.
- */
-public class AdditionalTypeProviderDelegateTest extends OptimizerTestBase {
-
- /**
- * Compilation unit for a class generated at runtime when an unknown
- * reference appears.
- */
- private class JavaWrapperCompilationUnit implements GeneratedUnit {
- private final long createTime = System.currentTimeMillis();
-
- @Override
- public String optionalFileLocation() {
- return null; // not used
- }
-
- @Override
- public String getStrongHash() {
- return "InsertedClass";
- }
-
- @Override
- public long creationTime() {
- return createTime;
- }
-
- @Override
- public String getSource() {
- String classSource =
- "package myPackage;\n" +
- "public class InsertedClass {\n" +
- " public static int getSmallNumber() {\n" +
- " return 5;\n" +
- " }\n" +
- "}";
- return classSource;
- }
-
- @Override
- public String getSourceMapPath() {
- // The named file location requires a non-Java extension,
- // or else the file won't get compiled correctly.
- return "myPackage/InsertedClass.notjava";
- }
-
- @Override
- public String getTypeName() {
- return "myPackage.InsertedClass";
- }
- @Override
- public long getSourceToken() {
- return -1;
- }
- }
-
- public boolean insertInsertedClass = false;
-
- @Override
- public void setUp() {
- // Create a source class that passes fine (just to test infrastructure.)
- sourceOracle.addOrReplace(new MockJavaResource("test.A") {
- @Override
- public CharSequence getContent() {
- StringBuilder code = new StringBuilder();
- code.append("package test;\n");
- code.append("class A {\n");
- code.append(" void myFunc() {\n");
- code.append(" }\n");
- code.append("}\n");
- return code;
- }
- });
-
- // Create a source file containing a reference to a class in another
- // package that we don't yet know about. That code will be inserted
- // by the AdditionalTypeProviderDelegate.
- sourceOracle.addOrReplace(new MockJavaResource("test.B") {
- @Override
- public CharSequence getContent() {
- StringBuilder code = new StringBuilder();
- code.append("package test;\n");
- code.append("import myPackage.InsertedClass;");
- code.append("class B {\n");
- code.append(" int func() {\n");
- // Reference an unknown class that will be substituted on the fly.
- code.append(" return myPackage.InsertedClass.getSmallNumber();\n");
- code.append(" }\n");
- code.append("}\n");
- return code;
- }
- });
-
- // Create a source file containing a reference to a class in another
- // package, but that lacks an import directive. Are we creating the
- // class anyway?
- sourceOracle.addOrReplace(new MockJavaResource("test.B1") {
- @Override
- public CharSequence getContent() {
- StringBuilder code = new StringBuilder();
- code.append("package test;\n");
- code.append("class B1 {\n");
- code.append(" int func() {\n");
- // Reference an unknown class that will be substituted on the fly.
- code.append(" return myPackage.InsertedClass.getSmallNumber();\n");
- code.append(" }\n");
- code.append("}\n");
- return code;
- }
- });
- }
-
- public void testInsertedClass() throws UnableToCompleteException {
- JProgram program = compileSnippet("void", "new test.B().func();", true);
-
- // Make sure the compiled classes appeared.
- JDeclaredType bType = findDeclaredType(program, "test.B");
- assertNotNull("Unknown type B", bType);
- JDeclaredType insertedClassType = findDeclaredType(program, "myPackage.InsertedClass");
- assertNotNull("Unknown type InsertedClass", insertedClassType);
- }
-
- public void testInsertedClass2() throws UnableToCompleteException {
- JProgram program = compileSnippet("void", "new test.B1().func();", true);
-
- // Make sure the compiled classes appeared.
- JDeclaredType bType = findDeclaredType(program, "test.B1");
- assertNotNull("Unknown type B1", bType);
- JDeclaredType insertedClassType = findDeclaredType(program, "myPackage.InsertedClass");
- assertNotNull("Unknown type InsertedClass", insertedClassType);
- }
-
- // Make sure regular code not using the AdditionalTypeProviderDelegate still works.
- public void testSimpleParse() throws UnableToCompleteException {
- JProgram program = compileSnippet("void", "new test.A();", true);
- JDeclaredType goodClassType = findDeclaredType(program, "test.A");
- assertNotNull("Unknown class A", goodClassType);
- }
-
- public void testUnknownClass() {
- // Create a source file containing a reference to an unknown class.
- sourceOracle.addOrReplace(new MockJavaResource("test.C") {
- @Override
- public CharSequence getContent() {
- StringBuilder code = new StringBuilder();
- code.append("package test;\n");
- code.append("import myPackage.UnknownClass;");
- code.append("class C {\n");
- code.append(" int func() {\n");
- // Reference an unknown class.
- code.append(" return myPackage.UnknownClass.getSmallNumber();\n");
- code.append(" }\n");
- code.append("}\n");
- return code;
- }
- });
- try {
- compileSnippet("void", "new test.C();", true);
- fail("Shouldn't have compiled");
- } catch (UnableToCompleteException expected) {
- }
- }
-
- public void testUnknownClassNoImport() {
- // Create a source file with a reference to an unknown class and no
- // import statement.
- sourceOracle.addOrReplace(new MockJavaResource("test.D") {
- @Override
- public CharSequence getContent() {
- StringBuilder code = new StringBuilder();
- code.append("package test;\n");
- code.append("class D {\n");
- code.append(" int func() {\n");
- // Reference an unknown class.
- code.append(" return myPackage.UnknownClass.getSmallNumber();\n");
- code.append(" }\n");
- code.append("}\n");
- return code;
- }
- });
- try {
- compileSnippet("void", "new test.D();", true);
- fail("Shouldn't have compiled");
- } catch (UnableToCompleteException expected) {
- }
- }
-
- @Override
- protected AdditionalTypeProviderDelegate getAdditionalTypeProviderDelegate() {
- // We'll provide a simple compiler delegate that will provide source
- // for a class called myPackage.InsertedClass.
- return new AdditionalTypeProviderDelegate() {
- @Override
- public boolean doFindAdditionalPackage(String slashedPackageName) {
- if (slashedPackageName.compareTo("myPackage") == 0) {
- return true;
- }
- return false;
- }
-
- @Override
- public GeneratedUnit doFindAdditionalType(String binaryName) {
- if (binaryName.compareTo("myPackage/InsertedClass") == 0) {
- return new JavaWrapperCompilationUnit();
- }
- return null;
- }
- };
- }
-
- @Override
- protected boolean doOptimizeMethod(TreeLogger logger, JProgram program, JMethod method) {
- return false;
- }
-}
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/FullCompileTestBase.java b/dev/core/test/com/google/gwt/dev/jjs/impl/FullCompileTestBase.java
index 3246258..79c7cd8 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/FullCompileTestBase.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/FullCompileTestBase.java
@@ -82,8 +82,7 @@
CompilerContext compilerContext = provideCompilerContext();
CompilationState state =
- CompilationStateBuilder.buildFrom(logger, compilerContext,
- sourceOracle.getResources(), getAdditionalTypeProviderDelegate());
+ CompilationStateBuilder.buildFrom(logger, compilerContext, sourceOracle.getResources());
ConfigurationProperties config = new ConfigurationProperties(Lists.newArrayList(
configurationProperties));
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/GwtAstBuilderTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/GwtAstBuilderTest.java
index 2e0c624..c4837a2 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/GwtAstBuilderTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/GwtAstBuilderTest.java
@@ -302,16 +302,14 @@
}).build();
compilerContext.getOptions().setSourceLevel(sourceLevel);
compilerContext.getOptions().setStrict(true);
- CompilationState state = CompilationStateBuilder.buildFrom(logger, compilerContext, sources,
- getAdditionalTypeProviderDelegate());
+ CompilationState state = CompilationStateBuilder.buildFrom(logger, compilerContext, sources);
return state;
}
private JProgram compileProgram(String entryType) throws UnableToCompleteException {
CompilerContext compilerContext = provideCompilerContext();
;
- CompilationState state = CompilationStateBuilder.buildFrom(logger, compilerContext, sources,
- getAdditionalTypeProviderDelegate());
+ CompilationState state = CompilationStateBuilder.buildFrom(logger, compilerContext, sources);
JProgram program = JavaAstConstructor.construct(logger, state, compilerContext,
null, entryType, "com.google.gwt.lang.Exceptions");
return program;
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/JJSTestBase.java b/dev/core/test/com/google/gwt/dev/jjs/impl/JJSTestBase.java
index a1f22f3..74ff099 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/JJSTestBase.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/JJSTestBase.java
@@ -23,7 +23,6 @@
import com.google.gwt.dev.javac.CheckerTestCase;
import com.google.gwt.dev.javac.CompilationState;
import com.google.gwt.dev.javac.CompilationStateBuilder;
-import com.google.gwt.dev.javac.JdtCompiler.AdditionalTypeProviderDelegate;
import com.google.gwt.dev.javac.testing.impl.MockJavaResource;
import com.google.gwt.dev.javac.testing.impl.MockResourceOracle;
import com.google.gwt.dev.jjs.JavaAstConstructor;
@@ -333,8 +332,7 @@
logger = this.logger;
}
CompilationState state =
- CompilationStateBuilder.buildFrom(logger, compilerContext,
- sourceOracle.getResources(), getAdditionalTypeProviderDelegate());
+ CompilationStateBuilder.buildFrom(logger, compilerContext, sourceOracle.getResources());
JProgram program =
JavaAstConstructor.construct(logger, state, compilerContext,
null, "test.EntryPoint", "com.google.gwt.lang.Exceptions");
@@ -361,14 +359,6 @@
}
/**
- * Return an AdditionalTypeProviderDelegate that will be able to provide
- * new sources for unknown classnames.
- */
- protected AdditionalTypeProviderDelegate getAdditionalTypeProviderDelegate() {
- return null;
- }
-
- /**
* Java source level compatibility option.
*/
protected SourceLevel sourceLevel = SourceLevel.DEFAULT_SOURCE_LEVEL;
diff --git a/dev/core/test/com/google/gwt/dev/js/JsStackEmulatorTest.java b/dev/core/test/com/google/gwt/dev/js/JsStackEmulatorTest.java
index 1114d41..330b039 100644
--- a/dev/core/test/com/google/gwt/dev/js/JsStackEmulatorTest.java
+++ b/dev/core/test/com/google/gwt/dev/js/JsStackEmulatorTest.java
@@ -265,8 +265,7 @@
recordLineNumbersProp));
CompilationState state =
- CompilationStateBuilder.buildFrom(logger, context,
- sourceOracle.getResources(), null);
+ CompilationStateBuilder.buildFrom(logger, context, sourceOracle.getResources());
JProgram jProgram = AstConstructor.construct(logger, state, options, config);
jProgram.addEntryMethod(findMethod(jProgram, "onModuleLoad"));
