On 2010/04/02 16:20:48, Lex wrote:
The one thing that needs looking at in this patch is the computation
of the
target clinit to get it to the best result on one iteration of recomputeAfterOptimization().
I *think* it does, please double-check my logic below and let me know if still see a problem.
Broadly, I am struck by how many different pieces of code have to be
careful
about clinits and had to be changed with this patch. It would be nice
to put
the clinit calls into the tree explicitly, so that not so many parts
of the
system have to check for an implicit clinit call.
Yeah, totally agree. http://gwt-code-reviews.appspot.com/184802/diff/1/2 File dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java (right): http://gwt-code-reviews.appspot.com/184802/diff/1/2#newcode130 dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java:130: * generalizes for say siblings targeting the same superclass.) On 2010/04/02 16:20:48, Lex wrote:
It's easier to read if it's written directly: "The clinit for the
source of the
reference must already have run, so if it's the same as this one,
there it must
have already run. One example is a reference from a subclass to
something in a
superclass."
Done. http://gwt-code-reviews.appspot.com/184802/diff/1/2#newcode275 dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java:275: */ On 2010/04/02 16:20:48, Lex wrote:
Intentional?
No, for some reason Eclipse's autoformatter barfs on this. http://gwt-code-reviews.appspot.com/184802/diff/1/2#newcode294 dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java:294: void setClinitTo(JDeclaredType newClinitTarget) { On 2010/04/02 16:20:48, Lex wrote:
The Java idiom is "setClinit".
Yeah, but that would be misleading since it would imply that you'd pass in the target clinit as the argument. This is really short for "set my clinit to be the target class's clinit". Maybe "setClinitTarget"? http://gwt-code-reviews.appspot.com/184802/diff/1/2#newcode299 dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java:299: removeClinit(); On 2010/04/02 16:20:48, Lex wrote:
If the target is another class, shouldn't the clinit of this one also
be
removed? Pruner is a really heavyweight optimization, so in general
it's not
good to wait for it.
I don't understand what you're saying here? Setting clinitTarget to the other class on 308 is what "removes" our clinit. http://gwt-code-reviews.appspot.com/184802/diff/1/3 File dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java (right): http://gwt-code-reviews.appspot.com/184802/diff/1/3#newcode654 dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java:654: type.setClinitTo(target); On 2010/04/02 16:20:48, Lex wrote:
Can't the target have another target, which has another one?
I don't *think* so but please double-check my logic: A class can only ever get retargeted to a super class. But forcing the superclass to be computed first on 645, by the time we get here we should be confident that the superclass has already gotten the tightest clinit binding possible at this time. http://gwt-code-reviews.appspot.com/184802/diff/1/3#newcode659 dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java:659: private JDeclaredType computeHasClinitTargetRecursive(JDeclaredType type, On 2010/04/02 16:20:48, Lex wrote:
I guess it's computing the target, not whether it "has" a target.
I actually did exactly that at one point, but it had the unfortunately effect of putting these methods into a different sort order and making a good diff impossible. Should I rename in a follow-on commit? http://gwt-code-reviews.appspot.com/184802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe, reply using "remove me" as the subject.
