[gwt-contrib] Re: Remove duplicate loop over cached units. (issue1463802)

[stephen . haberman]

Ping.

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

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

[gwt-contrib] Re: Remove duplicate loop over cached units. (issue1463802)

[stephen . haberman]

On 2011/08/15 13:27:47, zundel wrote:

I have half a mind to get rid of setting allValidClasses from the
other two places.


Done in the updated patch set.



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

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

[gwt-contrib] Re: Remove duplicate loop over cached units. (issue1463802)

[stephen . haberman]

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

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

[gwt-contrib] Re: Remove duplicate loop over cached units. (issue1463802)

[stephen . haberman]

http://gwt-code-reviews.appspot.com/1463802/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
File dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
(left):

http://gwt-code-reviews.appspot.com/1463802/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java#oldcode204
dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:204:
}

But the cost of iterating the units to make sure they are in the cache

should be small (agreed?), and it keeps the contract of the method

True.



I have half a mind to get rid of setting allValidClasses from the

other two places.

Yeah, I think that's a better fix.

I think those are the only two callers of CompileMoreLater.addValidUnit,
so that could go away too.

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

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

[gwt-contrib] Re: Remove duplicate loop over cached units. (issue1463802)

[zundel]

http://gwt-code-reviews.appspot.com/1463802/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
File dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
(left):

http://gwt-code-reviews.appspot.com/1463802/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java#oldcode204
dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:204:
}
If this was a private method, I'd say it was worth optimizing this.
But the cost of iterating the units to make sure they are in the cache
should be small (agreed?), and it keeps the contract of the method - if
you don't ensure the cachedUnits are in allValidClasses, the method
won't work.

I have half a mind to get rid of setting allValidClasses from the other
two places.

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

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