Rolling back *** Reason for rollback *** Causing test failures *** Original change description *** Optimize redundant 'switch' statements Review at http://gwt-code-reviews.appspot.com/1286801 git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@9573 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 7aeb7f4..e795adb 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java +++ b/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
@@ -107,7 +107,6 @@ import com.google.gwt.dev.js.EvalFunctionsAtTopScope; import com.google.gwt.dev.js.JsBreakUpLargeVarStatements; import com.google.gwt.dev.js.JsCoerceIntShift; -import com.google.gwt.dev.js.JsDuplicateCaseFolder; import com.google.gwt.dev.js.JsDuplicateFunctionRemover; import com.google.gwt.dev.js.JsIEBlockSizeVisitor; import com.google.gwt.dev.js.JsInliner; @@ -323,11 +322,6 @@ // (9) Optimize the JS AST. if (optimizationLevel > OptionOptimize.OPTIMIZE_LEVEL_DRAFT) { optimizeJs(options, jsProgram); - - /* - * Coalesce redundant labels in switch statements. - */ - JsDuplicateCaseFolder.exec(jsProgram); } /* @@ -578,7 +572,7 @@ Memory.maybeDumpMemory("AstOnly"); AstDumper.maybeDumpAST(jprogram); - + // See if we should run the EnumNameObfuscator if (module != null) { ConfigurationProperty enumNameObfuscationProp = (ConfigurationProperty) module.getProperties().find( @@ -828,12 +822,12 @@ // remove same parameters value stats.add(SameParameterValueOptimizer.exec(jprogram).recordVisits( numNodes)); - + /* * enum ordinalization * TODO(jbrosenberg): graduate this out of the 'isAggressivelyOptimize' * block, over time. - */ + */ stats.add(EnumOrdinalizer.exec(jprogram).recordVisits(numNodes)); }
diff --git a/dev/core/src/com/google/gwt/dev/js/JsDuplicateCaseFolder.java b/dev/core/src/com/google/gwt/dev/js/JsDuplicateCaseFolder.java deleted file mode 100644 index 2d78606..0000000 --- a/dev/core/src/com/google/gwt/dev/js/JsDuplicateCaseFolder.java +++ /dev/null
@@ -1,174 +0,0 @@ -/* - * Copyright 2011 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.js; - -import com.google.gwt.dev.js.ast.JsBlock; -import com.google.gwt.dev.js.ast.JsContext; -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.JsSwitch; -import com.google.gwt.dev.js.ast.JsSwitchMember; - -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; - -/** - * Combine case labels with identical bodies. Case bodies that may fall through - * to the following case label and case bodies following a possible fallthrough - * are left undisturbed. - * - * For example, consider the following input: - * - * <pre> - * switch (x) { - * case 0: y = 17; break; - * case 1: if (z == 0) { y = 18; break; } else { y = 19 } // fallthrough else - * case 2: return 22; - * case 3: if (z == 0) { y = 18; break; } else { y = 19 } // fallthrough else - * case 4: y = 17; break; - * case 5: y = 17; break; - * case 6: return 22; - * } - * </pre> - * - * This will be transformed into: - * - * <pre> - * switch (x) { - * case 0: y = 17; break; - * case 1: if (z == 0) { y = 18; break; } else { y = 19 } - * case 6: case 2: return 22; - * case 3: if (z == 0) { y = 18; break; } else { y = 19 } - * case 5: case 4: y = 17; break; - * } - * - * <pre> - * - * Cases (2, 6) and (4, 5) have been coalesced. Note that case 0 has not been - * combined with cases 4 and 5 since case 4 cannot be moved due to the potential - * fallthrough from case 3, and we currently only coalesce a given cases with a - * preceding case and so cannot move case 0 downward. - * - * Although this pattern is unlikely to occur frequently in hand-written code, - * it can account for a significant amount of space in generated code. - */ -public class JsDuplicateCaseFolder { - - private class DuplicateCaseFolder extends JsModVisitor { - - public DuplicateCaseFolder() { - } - - @Override - public boolean visit(JsSwitch x, JsContext<JsStatement> ctx) { - boolean modified = false; - - // A map from case body source code to the original case label - // in which they appeared - Map<String, JsSwitchMember> seen = new HashMap<String, JsSwitchMember>(); - - // Original list of members - List<JsSwitchMember> cases = x.getCases(); - // Coalesced list of members - List<JsSwitchMember> newCases = new LinkedList<JsSwitchMember>(); - - // Keep track of whether the previous case can fall through - // to the current case - boolean hasPreviousFallthrough = false; - - // Iterate over members and locate ones with bodies identical to - // previous members - for (JsSwitchMember member : cases) { - List<JsStatement> stmts = member.getStmts(); - - // Don't rewrite any cases that might fall through - if (!unconditionalControlBreak(stmts)) { - hasPreviousFallthrough = true; - // copy the case into the output - newCases.add(member); - continue; - } - - String body = toSource(stmts); - JsSwitchMember previousCase = seen.get(body); - if (previousCase == null || hasPreviousFallthrough) { - // Don't coalesce a case that can be reached via fallthrough - // from the previous case - newCases.add(member); - seen.put(body, member); - } else { - // Locate the position of the case that this case is to be - // coalesced with. Note: linear search in output list - int index = newCases.indexOf(previousCase); - - // Empty the case body and insert the case label into the output - member.getStmts().clear(); - newCases.add(index, member); - modified = true; - } - - hasPreviousFallthrough = false; - } - - // Rewrite the AST if any cases have changed - if (modified) { - didChange = true; - cases.clear(); - cases.addAll(newCases); - } - - return true; - } - - private String toSource(List<JsStatement> stmts) { - StringBuilder sb = new StringBuilder(); - for (JsStatement stmt : stmts) { - sb.append(stmt.toSource()); - sb.append("\n"); // separate statements - } - return sb.toString(); - } - - /** - * See {@link JsStatement#unconditionalControlBreak()}. - */ - private boolean unconditionalControlBreak(List<JsStatement> stmts) { - for (JsStatement stmt : stmts) { - if (stmt.unconditionalControlBreak()) { - return true; - } - } - return false; - } - } - - // Needed for OptimizerTestBase - public static boolean exec(JsProgram program) { - return new JsDuplicateCaseFolder().execImpl(program.getFragmentBlock(0)); - } - - public JsDuplicateCaseFolder() { - } - - private boolean execImpl(JsBlock fragment) { - DuplicateCaseFolder dcf = new DuplicateCaseFolder(); - dcf.accept(fragment); - return dcf.didChange(); - } -}
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsBlock.java b/dev/core/src/com/google/gwt/dev/js/ast/JsBlock.java index 0cbf91c..a4a06c8 100644 --- a/dev/core/src/com/google/gwt/dev/js/ast/JsBlock.java +++ b/dev/core/src/com/google/gwt/dev/js/ast/JsBlock.java
@@ -45,14 +45,4 @@ } v.endVisit(this, ctx); } - - @Override - public boolean unconditionalControlBreak() { - for (JsStatement stmt : stmts) { - if (stmt.unconditionalControlBreak()) { - return true; - } - } - return false; - } }
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsBreak.java b/dev/core/src/com/google/gwt/dev/js/ast/JsBreak.java index 5c63bde..5c2fb73 100644 --- a/dev/core/src/com/google/gwt/dev/js/ast/JsBreak.java +++ b/dev/core/src/com/google/gwt/dev/js/ast/JsBreak.java
@@ -48,6 +48,6 @@ @Override public boolean unconditionalControlBreak() { - return label == null; + return true; } }
diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsIf.java b/dev/core/src/com/google/gwt/dev/js/ast/JsIf.java index b1b9d3d..2440155 100644 --- a/dev/core/src/com/google/gwt/dev/js/ast/JsIf.java +++ b/dev/core/src/com/google/gwt/dev/js/ast/JsIf.java
@@ -74,17 +74,4 @@ } v.endVisit(this, ctx); } - - @Override - public boolean unconditionalControlBreak() { - boolean thenBreaks = thenStmt != null - && thenStmt.unconditionalControlBreak(); - boolean elseBreaks = elseStmt != null - && elseStmt.unconditionalControlBreak(); - if (thenBreaks && elseBreaks) { - // both branches have an unconditional break - return true; - } - return false; - } }
diff --git a/dev/core/test/com/google/gwt/dev/js/JsDuplicateCaseFolderTest.java b/dev/core/test/com/google/gwt/dev/js/JsDuplicateCaseFolderTest.java deleted file mode 100644 index 356f123..0000000 --- a/dev/core/test/com/google/gwt/dev/js/JsDuplicateCaseFolderTest.java +++ /dev/null
@@ -1,95 +0,0 @@ -/* - * Copyright 2011 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.js; - -/** - * Tests the JsDuplicateCaseFolder optimizer. - */ -public class JsDuplicateCaseFolderTest extends OptimizerTestBase { - - public void test1() throws Exception { - String input = "function a(x){switch(x){case 0:return 17;case 12:case 1:return 18;case 2:return 17;case 3:return 18;}}"; - String expected = "function a(x){switch(x){case 2:case 0:return 17;case 12:case 3:case 1:return 18;}}\n"; - check(expected, input); - } - - public void test1b() throws Exception { - String input = "function a(x){switch(x){case 0:return 17;case 12:case 1:return 18;default:return 17;case 3:return 18;}}"; - String expected = "function a(x){switch(x){default:case 0:return 17;case 12:case 3:case 1:return 18;}}\n"; - check(expected, input); - } - - public void test2() throws Exception { - // Don't coalesces cases 0 and 2 since 2 can be reached via fallthrough from 1 - String input = "function a(x){var y;switch(x){case 0:return 17;case 1:y=18;case 2:return 17;case 3:y=18;}return y}"; - String expected = "function a(x){var y;switch(x){case 0:return 17;case 1:y=18;case 2:return 17;case 3:y=18;}return y}\n"; - check(expected, input); - } - - public void test3() throws Exception { - // cases 1 and 3 can fall through, don't coalesce - String input = "function a(x){var y;switch(x){case 0:return 17;case 1:y=18;break;case 2:return 17;case 3:y=18;break;}return y}"; - String expected = "function a(x){var y;switch(x){case 2:case 0:return 17;case 3:case 1:y=18;break;}return y}\n"; - check(expected, input); - } - - public void test4() throws Exception { - // cases 1 and 3 may be coalesced - String input = "function a(x,z){var y;switch(x){case 0:return 17;case 1:if (z==0){y=18;break}else{y=19;break}case 2:return 17;case 3:if(z==0){y=18;break}else{y=19;break}}return y}"; - String expected = "function a(x,z){var y;switch(x){case 2:case 0:return 17;case 3:case 1:if(z==0){y=18;break}else{y=19;break}}return y}\n"; - check(expected, input); - } - - public void test4b() throws Exception { - // cases 1 and 3 may be coalesced - // ensure additional fallthroughs are handled correctly - String input = "function a(x,z){var y;switch(x){case 0:case 22:return 17;case 100:case 1:if (z==0){y=18;break}else{y=19;return 20}case 2:return 17;case 200:case 3:if(z==0){y=18;break}else{y=19;return 20}}return y}"; - String expected = "function a(x,z){var y;switch(x){case 0:case 2:case 22:return 17;case 100:case 1:if(z==0){y=18;break}else{y=19;return 20}case 200:case 3:if(z==0){y=18;break}else{y=19;return 20}}return y}\n"; - check(expected, input); - } - - public void test5() throws Exception { - // cases 1 and 3 can fall through due to no else clause, don't coalesce - String input = "function a(x,z){var y;switch(x){case 0:return 17;case 1:if (z==0){y=18;break}else{y=19}case 2:return 17;case 3:if(z==0){y=18;break}else{y=19}}return y}"; - String expected = "function a(x,z){var y;switch(x){case 0:return 17;case 1:if(z==0){y=18;break}else{y=19}case 2:return 17;case 3:if(z==0){y=18;break}else{y=19}}return y}\n"; - check(expected, input); - } - - public void test6() throws Exception { - // cases 1 and 3 can fall through due to no else clause, don't coalesce - String input = "function a(x,z){var y;switch(x){case 0:y=17;break;case 1:if(z==0){y=18;break}else{y=19}case 2:return 22;case 3:if(z==0){y=18;break}else{y=19}case 4:y=17;break;case 5:y=17;break;case 6:return 22;}return y}"; - String expected = "function a(x,z){var y;switch(x){case 0:y=17;break;case 1:if(z==0){y=18;break}else{y=19}case 6:case 2:return 22;case 3:if(z==0){y=18;break}else{y=19}case 5:case 4:y=17;break;}return y}\n"; - check(expected, input); - } - - public void test6b() throws Exception { - // cases 1 and 3 can fall through due to no else clause, don't coalesce - String input = "function a(x,z){var y;switch(x){case 0:y=17;break;case 1:if(z==0){y=18;break}else{y=19}default:return 22;case 3:if(z==0){y=18;break}else{y=19}case 4:y=17;break;case 5:y=17;break;case 6:return 22;}return y}"; - String expected = "function a(x,z){var y;switch(x){case 0:y=17;break;case 1:if(z==0){y=18;break}else{y=19}case 6:default:return 22;case 3:if(z==0){y=18;break}else{y=19}case 5:case 4:y=17;break;}return y}\n"; - check(expected, input); - } - - private void check(String expected, String input) throws Exception { - // Pass the expected code through the optimizer to normalize it - expected = super.optimize(expected, new Class[0]); - String output = optimize(input); - assertEquals(expected, output); - } - - private String optimize(String js) throws Exception { - return optimize(js, JsDuplicateCaseFolder.class); - } -}