Enhancement to JsDuplicateFunctionRemover to remove duplicate virtual methods as well. That is, given
_.method1 = function(a) { body }
_.method2 = function(b) { body }
Hoist the duplicate virtual methods and replace with a named global function.
function c(a) { body }
_.method1 = c;
_.method2 = c;
Produces a 1%-3% improvement in code size when stack stripping is enabled.
Review at http://gwt-code-reviews.appspot.com/1454806
Review by: skybrian@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@11038 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
index 5902127..d6f4bd1 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
@@ -395,6 +395,8 @@
}
if (changed) {
JsUnusedFunctionRemover.exec(jsProgram);
+ // run again
+ JsObfuscateNamer.exec(jsProgram);
}
}
}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java b/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java
index 845ba4f..1bddd9a 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java
@@ -46,10 +46,17 @@
private final Map<JsName, JsName> duplicateOriginalMap = new IdentityHashMap<JsName, JsName>();
+ private final Map<JsFunction, JsFunction> duplicateMethodOriginalMap = new IdentityHashMap<JsFunction, JsFunction>();
+
+
private final Stack<JsNameRef> invocationQualifiers = new Stack<JsNameRef>();
+ // static / global methods
private final Map<String, JsName> uniqueBodies = new HashMap<String, JsName>();
+ // vtable methods
+ private final Map<String, JsFunction> uniqueMethodBodies = new HashMap<String, JsFunction>();
+
public DuplicateFunctionBodyRecorder() {
// Add sentinel to stop Stack.peek() from throwing exception.
invocationQualifiers.add(null);
@@ -84,20 +91,31 @@
return duplicateOriginalMap;
}
+ public Map<JsFunction, JsFunction> getDuplicateMethodMap() {
+ return duplicateMethodOriginalMap;
+ }
+
@Override
public boolean visit(JsFunction x, JsContext ctx) {
+ String fnSource = x.toSource();
+ String body = fnSource.substring(fnSource.indexOf("("));
/*
- * Don't process anonymous functions.
+ * Static function processed separate from virtual functions
*/
if (x.getName() != null) {
- String fnSource = x.toSource();
- String body = fnSource.substring(fnSource.indexOf("("));
JsName original = uniqueBodies.get(body);
if (original != null) {
duplicateOriginalMap.put(x.getName(), original);
} else {
uniqueBodies.put(body, x.getName());
}
+ } else if (x.isFromJava()) {
+ JsFunction original = uniqueMethodBodies.get(body);
+ if (original != null) {
+ duplicateMethodOriginalMap.put(x, original);
+ } else {
+ uniqueMethodBodies.put(body, x);
+ }
}
return true;
}
@@ -114,13 +132,27 @@
private class ReplaceDuplicateInvocationNameRefs extends JsModVisitor {
private final Set<JsName> blacklist;
+ private final Map<JsFunction, JsFunction> dupMethodMap;
+ private final Map<JsFunction, JsName> hoistMap;
private final Map<JsName, JsName> duplicateMap;
public ReplaceDuplicateInvocationNameRefs(Map<JsName, JsName> duplicateMap,
- Set<JsName> blacklist) {
+ Set<JsName> blacklist, Map<JsFunction, JsFunction> dupMethodMap,
+ Map<JsFunction, JsName> hoistMap) {
this.duplicateMap = duplicateMap;
this.blacklist = blacklist;
+ this.dupMethodMap = dupMethodMap;
+ this.hoistMap = hoistMap;
+ }
+
+ @Override
+ public void endVisit(JsFunction x, JsContext ctx) {
+ if (dupMethodMap.containsKey(x)) {
+ ctx.replaceMe(hoistMap.get(dupMethodMap.get(x)).makeRef(x.getSourceInfo()));
+ } else if (hoistMap.containsKey(x)) {
+ ctx.replaceMe(hoistMap.get(x).makeRef(x.getSourceInfo()));
+ }
}
@Override
@@ -152,8 +184,28 @@
private boolean execImpl(JsBlock fragment) {
DuplicateFunctionBodyRecorder dfbr = new DuplicateFunctionBodyRecorder();
dfbr.accept(fragment);
+ int count = 0;
+ Map<JsFunction, JsName> hoistMap = new HashMap<JsFunction, JsName>();
+ // Hoist all anonymous versions
+ Map<JsFunction, JsFunction> dupMethodMap = dfbr.getDuplicateMethodMap();
+ for (JsFunction x : dupMethodMap.values()) {
+ if (!hoistMap.containsKey(x)) {
+ // move function to top scope and re-declaring it with a unique name
+ JsName newName = program.getScope().declareName("_DUP" + count++);
+ JsFunction newFunc = new JsFunction(x.getSourceInfo(),
+ program.getScope(), newName, x.isFromJava());
+ // we're not using the old function anymore, we can use reuse the body instead of cloning it
+ newFunc.setBody(x.getBody());
+ // also copy the parameters from the old function
+ newFunc.getParameters().addAll(x.getParameters());
+ // add the new function to the top level list of statements
+ fragment.getStatements().add(newFunc.makeStmt());
+ hoistMap.put(x, newName);
+ }
+ }
+
ReplaceDuplicateInvocationNameRefs rdup = new ReplaceDuplicateInvocationNameRefs(
- dfbr.getDuplicateMap(), dfbr.getBlacklist());
+ dfbr.getDuplicateMap(), dfbr.getBlacklist(), dupMethodMap, hoistMap);
rdup.accept(fragment);
return rdup.didChange();
}
diff --git a/dev/core/test/com/google/gwt/dev/js/JsDuplicateFunctionRemoverTest.java b/dev/core/test/com/google/gwt/dev/js/JsDuplicateFunctionRemoverTest.java
index 9ce5659..f28cc80 100644
--- a/dev/core/test/com/google/gwt/dev/js/JsDuplicateFunctionRemoverTest.java
+++ b/dev/core/test/com/google/gwt/dev/js/JsDuplicateFunctionRemoverTest.java
@@ -15,6 +15,22 @@
*/
package com.google.gwt.dev.js;
+import com.google.gwt.dev.jjs.SourceOrigin;
+
+import com.google.gwt.dev.js.ast.JsContext;
+import com.google.gwt.dev.js.ast.JsFunction;
+import com.google.gwt.dev.js.ast.JsModVisitor;
+import com.google.gwt.dev.js.ast.JsProgram;
+import com.google.gwt.dev.js.ast.JsStatement;
+import com.google.gwt.dev.js.ast.JsVisitor;
+import com.google.gwt.dev.util.TextOutput;
+import com.google.gwt.dev.util.DefaultTextOutput;
+
+import java.lang.reflect.Method;
+import java.io.StringReader;
+import java.util.List;
+
+
/**
* Tests the JsStaticEval optimizer.
*/
@@ -35,8 +51,48 @@
optimize("function a(){};function b(){} a(); b();"));
}
+ public void testVirtualRemoveDuplicates() throws Exception {
+ JsProgram program = new JsProgram();
+ String js = "_.method1=function(){};_.method2=function(){};_.method1();_.method2();";
+ List<JsStatement> expected = JsParser.parse(SourceOrigin.UNKNOWN,
+ program.getScope(), new StringReader(js));
+ program.getGlobalBlock().getStatements().addAll(expected);
+ new JsModVisitor() {
+ public void endVisit(JsFunction func, JsContext ctx) {
+ func.setFromJava(true);
+ }
+ }.accept(program);
+
+ assertEquals("_.method1=_DUP0;_.method2=_DUP0;_.method1();_.method2();function _DUP0(){}\n",
+ optimize(program, JsSymbolResolver.class, JsDuplicateFunctionRemover.class,
+ JsUnusedFunctionRemover.class));
+ }
+
private String optimize(String js) throws Exception {
return optimize(js, JsSymbolResolver.class,
JsDuplicateFunctionRemover.class, JsUnusedFunctionRemover.class);
}
-}
\ No newline at end of file
+
+
+ /**
+ * Optimize a JS program.
+ *
+ * @param js the source program
+ * @param toExec a list of classes that implement
+ * <code>static void exec(JsProgram)</code>
+ * @return optimized JS
+ */
+ protected String optimize(JsProgram program, Class<?>... toExec) throws Exception {
+
+ for (Class<?> clazz : toExec) {
+ Method m = clazz.getMethod("exec", JsProgram.class);
+ m.invoke(null, program);
+ }
+
+ TextOutput text = new DefaultTextOutput(true);
+ JsVisitor generator = new JsSourceGenerationVisitor(text);
+
+ generator.accept(program);
+ return text.toString();
+ }
+}
