http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
File dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
(right):

http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode84
dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:84: *
Marks as "referenced" any types, methods, and fields that are reachable.
On 2011/05/20 20:14:17, jbrosenberg wrote:
s/any the classes/any classes/

Done.

http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode382
dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:382:
Done, factored out the into a well-documented method,
rescueArgumentsIfParametersCanBeRead.

http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode385
dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:385:
if (program.staticImplFor(method) != null) {
On 2011/05/20 20:14:17, jbrosenberg wrote:
Pruner.CleanupRefsVisitor

Done.

http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode772
dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:772:
On 2011/05/20 20:14:17, jbrosenberg wrote:
Perhaps add a comment for this map, similar to the "Schrodinger's"
comment below
for membersLiveExceptForInstantiability.  I'm a bit confused by the
name
"argsLiveExceptForParamRead", how about "argsToAcceptIfParamRead".

Done.

http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode787
dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:787:
*/
On 2011/05/20 20:14:17, jbrosenberg wrote:
How about "membersToRescueIfInstantiable"

Done.

http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
File dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java (right):

http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java#newcode421
dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java:421: *
devirtualizations. If the instance method has been pruned, then it's
On 2011/05/20 20:14:17, jbrosenberg wrote:
Here I think it's assumed "saveCodeGenTypes == true" = "it's the final
pass".
Perhaps this should be made more clear, or the comment updated to
clarify.

Done.

http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java#newcode570
dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java:570: // Retain the
original arguments, they will be evaluated for side effects.
Yeah, when I went and reread the spec, I discovered that arguments are
evaluated before NPE is checked. :/

http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java#newcode600
dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java:600:
On 2011/05/20 20:14:17, jbrosenberg wrote:
Is it now certain that the Pruner should not need any subsequent
passes to
complete all possible pruning?  Or is it "most of the way there", and
this is
now fair to cut it off to save time?

It's not 100%, but it's close enough that not iterating on Pruner
doesn't add more passes to the outer optimization loop for the compiles
I've tried.

http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java
File dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java (right):

http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java#newcode145
dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java:145:
program.addEntryMethod(findMainMethod(program));
The test contains one of the few cases that still tickles through,
namely the transformation from x.foo() -> null.nullMethod() where x is
eventually pruned.  It seemed more expedient to realize the immediate
benefit and let someone else solve the last case later.  It was tricky
enough that I would have needed to invest more time than I had on it.

We don't have a test for the 'false' case.

http://gwt-code-reviews.appspot.com/1436802/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to