http://gwt-code-reviews.appspot.com/1454806/diff/1/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java File dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java (right):
http://gwt-code-reviews.appspot.com/1454806/diff/1/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java#newcode135 dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java:135: private Map<JsFunction, JsFunction> dupMethodMap; On 2011/06/29 00:38:34, zundel wrote:
final
Done. http://gwt-code-reviews.appspot.com/1454806/diff/1/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java#newcode136 dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java:136: private Map<JsFunction, JsName> hoistMap; On 2011/06/29 00:38:34, zundel wrote:
final
Done. http://gwt-code-reviews.appspot.com/1454806/diff/1/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java#newcode193 dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java:193: JsName newName = program.getScope().declareName("_DUP" + count++); On 2011/06/29 00:38:34, zundel wrote:
I would appreciate a brief summary of hoisting in a comment here, or a
pointer
to another place in the code where it is described.
I'll add a comment. Basically, we need to declare a new top level function and given it a unique name (which will be obfuscated later). Normally, you'd want something like the CloneVisitor to clone the function body, but we're not actually creating a duplicate, we're just moving it, so we can get away with just reusing the original body. So we create a new JsFunction, this time declared with a new name and a new scope, but we just reference the body of the old function being hoisted (which will no longer be referenced by the AST) http://gwt-code-reviews.appspot.com/1454806/diff/1/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java#newcode196 dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java:196: newFunc.setBody(x.getBody()); It's not being made static. The way JS binds 'this', it just works, e.g. _.x = function x() { this.x = 10; } _.y = function y() { this.x = 10; } function _DUP1() { this.x = 10; } _.x = DUP1; _.y = DUP1; When calling either m.x() or m.y(), 'this' will get bound properly. On 2011/06/29 00:38:34, zundel wrote:
dumb question here - what are you doing to make this method static?
what if the
method body has references to 'this'?
http://gwt-code-reviews.appspot.com/1454806/diff/1/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java#newcode202 dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java:202: } I might do that in a future patch, it would only yield a benefit for methods which don't reference 'this' (e.g. a virtual method and a static method don't reference 'this', since in one it will appear as 'this' and in the other as 'this$static'.) Given the large number of duplicate constructors, and duplicate subtrees, my next goal after this lands is to work on a general purpose anti-cloning optimization that works at any granularity, not just function level. On 2011/06/29 00:38:34, zundel wrote:
if you wanted to squeeze the last ounce out of this, at this point you
might
combine duplicate bodies between the bodies of the hoisted methods and
the
static bodies in uniqueBodies.
http://gwt-code-reviews.appspot.com/1454806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
