I saw 4.5% on the uncompressed size. Dan
On Tue, Jan 18, 2011 at 4:39 PM, Ray Cromwell <[email protected]> wrote: > I'm curious, has there been any measurement/data as to the effect this > has on output size? > > On Mon, Jan 17, 2011 at 1:18 PM, <[email protected]> wrote: >> LGTM, just nits. No need to re-review. >> >> >> http://gwt-code-reviews.appspot.com/1286801/diff/5002/6002 >> File dev/core/src/com/google/gwt/dev/js/JsDuplicateCaseFolder.java >> (right): >> >> http://gwt-code-reviews.appspot.com/1286801/diff/5002/6002#newcode143 >> dev/core/src/com/google/gwt/dev/js/JsDuplicateCaseFolder.java:143: >> sb.append("\0"); // separate statements with an otherwise illegal char >> I would actually use '\n' here so that the string is easily readable in >> a debugger. >> >> http://gwt-code-reviews.appspot.com/1286801/diff/5002/6003 >> File dev/core/src/com/google/gwt/dev/js/ast/JsBlock.java (right): >> >> http://gwt-code-reviews.appspot.com/1286801/diff/5002/6003#newcode50 >> dev/core/src/com/google/gwt/dev/js/ast/JsBlock.java:50: public boolean >> unconditionalControlBreak() { >> This work correctly because of the implementation of >> JBreakStatement.unconditionalControlBreak(), which never counts labelled >> break statements as breaks. Could you add a comment here or there (or >> both?) about this dependency between the two? >> >> http://gwt-code-reviews.appspot.com/1286801/diff/5002/6004 >> File dev/core/src/com/google/gwt/dev/js/ast/JsIf.java (right): >> >> http://gwt-code-reviews.appspot.com/1286801/diff/5002/6004#newcode86 >> dev/core/src/com/google/gwt/dev/js/ast/JsIf.java:86: } >> This is handy, but DeadCodeElimination should have already taken care of >> these cases. I would argue for going with the computationally cheaper >> (and simpler) implementation here if it doesn't actually improve the >> output. >> >> http://gwt-code-reviews.appspot.com/1286801/diff/5002/6005 >> File dev/core/test/com/google/gwt/dev/js/JsDuplicateCaseFolderTest.java >> (right): >> >> http://gwt-code-reviews.appspot.com/1286801/diff/5002/6005#newcode85 >> dev/core/test/com/google/gwt/dev/js/JsDuplicateCaseFolderTest.java:85: >> private void check(String input, String expected) throws Exception { >> Total nit, but I would reverse the argument order to match the order of >> assertEquals(). >> >> http://gwt-code-reviews.appspot.com/1286801/diff/5002/6005#newcode87 >> dev/core/test/com/google/gwt/dev/js/JsDuplicateCaseFolderTest.java:87: >> String optimized = optimize(expected); >> You probably don't actually want to optimize this, right? Suggest: >> >> // Generate canonical expected output (no optimizers) >> String expectedOutput = super.optimize(expected, new Class[0]); >> >> http://gwt-code-reviews.appspot.com/1286801/show >> >> -- >> http://groups.google.com/group/Google-Web-Toolkit-Contributors >> > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
