FYI, here are some timings of a web mode compile of Showcase with one
permutation and -draftCompile
PersistentCache off Archives off : 39.990s
PersistentCache off Archives on : 34.184s
PersistentCache on Archives off : 29.429s
PersistentCache on Archives on : 29.680s
This is pretty much as expected, as the CompileModule patch doesn't have
any effect on the code in the showcase project itself or generated code.
http://gwt-code-reviews.appspot.com/1448808/diff/1/dev/core/src/com/google/gwt/dev/CompileModule.java
File dev/core/src/com/google/gwt/dev/CompileModule.java (right):
http://gwt-code-reviews.appspot.com/1448808/diff/1/dev/core/src/com/google/gwt/dev/CompileModule.java#newcode271
dev/core/src/com/google/gwt/dev/CompileModule.java:271: + ex);
On 2011/06/03 03:53:58, zundel wrote:
needs to bail here too.
Done.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/CompileModule.java#newcode54
dev/core/src/com/google/gwt/dev/CompileModule.java:54: * are assumed
good and loaded first. CachedCompilationUnits that already exist
On 2011/06/03 19:48:40, jbrosenberg wrote:
Can we automatically re-order a set of modules so they will be
processed in
optimal order? Perhaps add a utility method to ModuleDefLoader?
Otherwise,
users could fail to get optimal benefit from the caching, and not be
aware of
it?
I think that would be nice and I'd like to do it, but after spending
about an hour trying to work through it, I'd rather it be in a follow-on
patch.
Unfortunately, ModuleDefLoader doesn't leave any trail of the
inheritance hierarchy behind at the moment, so first we'd have to go
through and something for that, then perform the determination of which
modules were "least dependent" on other modules (since dependencies can
be circular) and sort. Then, in order for this to work properly, we'd
have go through gwt-user and insure that all modules are properly
inheriting the other modules they truly depend on. There is no
enforcement of that right now (figure out a way to add warnings?)
For example (I'm just picking on logging, I'm sure it not the only one):
MyProject
--> Logging.gwt.xml --> no inherits of User
--> User.gwt.xml --> no inherits of Logging
From the inherits lines, it looks like you could Compile User without
Logging and Logging without User. But in fact, you can't compile
Logging as a top level module. Its just a mistake, but we'd need to go
in and patch it up to make the auto-sorting work properly.
If we got all that working, then we could just throw every .gwt.xml file
from gwt-user onto the command line (or point this tool at a directory)
and have it correctly create archive files - hooray!
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/CompileModule.java#newcode173
dev/core/src/com/google/gwt/dev/CompileModule.java:173: TreeLogger
nodeBranch = topBranch.branch(TreeLogger.ERROR, msg, null);
On 2011/06/03 19:48:40, jbrosenberg wrote:
Also add "moduleToCompile" attribute to this event?
Done.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/CompileModule.java#newcode185
dev/core/src/com/google/gwt/dev/CompileModule.java:185: return new
UnableToCompleteException();
On 2011/06/03 19:48:40, jbrosenberg wrote:
maybe change "module" to "dependentModule" here; or "subModule"?
Done.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/CompileModule.java#newcode191
dev/core/src/com/google/gwt/dev/CompileModule.java:191: public
CompileModule(CompileModuleOptions options) {
I shuffled the code so that if the topLevelModule.gwt is around, we will
load it, (but not keep track of the units in it like we do for other
pre-loaded modules)
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/CompileModule.java#newcode192
dev/core/src/com/google/gwt/dev/CompileModule.java:192: this.options =
new CompileModuleOptionsImpl(options);
I've changed the code to go ahead and load the top level module and
worked out the bug I was trying to prevent another way. A clean build
takes 17secs to execute the ant "precompile.modules" task. Before this
change, any change to the dependencies of "precompile.modules" would
cause the invocation to take 17secs again. After this change, a rebuild
with just one file with a whitespace change takes 9secs.
Unfortunately, this change isn't going to be a big win in all
environments. In some build environments the <foo>.gwt output file is
erased from the classpath before CompileModule is invoked.
One unspoken assumption I've used throughout this design is that there
is an external build tool, like ant, that is doing dependency checking
for us. See the <outofdate> clause in the build file. That build tool
can often evaulate staleness a lot better than this tool. For example,
this tool can't easily tell when the version of GWT has changed out from
underneath us.
I've added that assumption to the comments in the header.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java
File dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java (right):
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java#newcode94
dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java:94: private final
Set<URL> archiveURLs = new LinkedHashSet<URL>();
I don't really need a Set, the names coming in should be unique - I was
just being paranoid.
I hope its OK to leave them as URLS though, as converting these URLs to
URIs causes exceptions when the .gwt files are bundled in .jar files.
ARGH! Leaving them as URLs makes the resources easy to fetch too.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java#newcode141
dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java:141: public void
addCompilationUnitArchiveURL(URL url) {
On 2011/06/03 20:13:54, tobyr wrote:
Should this be package access?
Done.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java#newcode309
dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java:309:
On 2011/06/03 19:48:40, jbrosenberg wrote:
how about getAllCompilationUnitArchiveURLs()
Since it doesn't return a set of archives, but archiveURLs?
Done.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java#newcode310
dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java:310: public URL[]
getAllCompilationUnitArchives() {
On 2011/06/03 20:13:54, tobyr wrote:
Why not just use the Set? If you're being defensive, why not
Collections.unmodifiableSet()?
I was following precedent in this file. Updated to return an
unmodifiable collection instead.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java
File dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java (left):
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java#oldcode114
dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java:114: * {@link
#loadFromClassPath(logger, moduleName, false)}.
On 2011/06/03 20:13:54, tobyr wrote:
This should be {@link #loadFromClassPath(TreeLogger, String, boolean)
loadFromClassPath(logger, moduleName, false)}
Done.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java
File dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java (right):
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java#newcode71
dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java:71: public
static final String COMPILATION_UNIT_ARCHIVE_SUFFIX = ".gwt";
On 2011/06/03 20:13:54, tobyr wrote:
.gwt seems a bit generic. Some ideas: .gar (GWT archive), .gwt-mar
(GWT module
archive), .gwt-module-cache (self explanatory)
I chose:
.gwtar
Try to talk me out of it.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java#newcode267
dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java:267: String
compilationUnitArchiveName = slashedModuleName +
ModuleDefLoader.COMPILATION_UNIT_ARCHIVE_SUFFIX;
On 2011/06/03 20:13:54, tobyr wrote:
It seems a bit strange to me that all dependent modules are being
rolled up.
Doing that seems like you're going to end up with copies of
dependencies in
different modules.
Possibly. If that happened it would be inefficient, but I don't think
it would cause bugs.
I'm sort of relying on CompileModule being invoked from a build tool
with some kind of knowledge about what is being compiled. I thought
about some kind of analysis that would create separate files for all
inherited modules, but then though twice about it because some build
tools need to know the names of all the output files,
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/javac/CompilationProblemReporter.java
File
dev/core/src/com/google/gwt/dev/javac/CompilationProblemReporter.java
(right):
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/javac/CompilationProblemReporter.java#newcode53
dev/core/src/com/google/gwt/dev/javac/CompilationProblemReporter.java:53:
On 2011/06/03 19:48:40, jbrosenberg wrote:
javadoc?
Done. Updated another method w/ missing javadoc too.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/javac/CompilationUnitArchive.java
File dev/core/src/com/google/gwt/dev/javac/CompilationUnitArchive.java
(right):
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/javac/CompilationUnitArchive.java#newcode74
dev/core/src/com/google/gwt/dev/javac/CompilationUnitArchive.java:74:
private transient SortedMap<String, CompilationUnit> units;
On 2011/06/03 20:13:54, tobyr wrote:
If you only need sorting for deterministic output, should you just
make this a
HashMap and sort only on writeObject?
That sounds more efficient. Done.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/javac/CompilationUnitArchive.java#newcode75
dev/core/src/com/google/gwt/dev/javac/CompilationUnitArchive.java:75:
On 2011/06/03 19:48:40, jbrosenberg wrote:
javadoc? explanation for topModuleName param?
Done.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/src/com/google/gwt/dev/javac/CompilationUnitArchive.java#newcode103
dev/core/src/com/google/gwt/dev/javac/CompilationUnitArchive.java:103:
On 2011/06/03 19:48:40, jbrosenberg wrote:
need to read/write topModuleName?
defaultReadObject() and defaultWriteObject() handle that.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/test/com/google/gwt/dev/javac/CompilationUnitArchiveTest.java
File
dev/core/test/com/google/gwt/dev/javac/CompilationUnitArchiveTest.java
(right):
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/test/com/google/gwt/dev/javac/CompilationUnitArchiveTest.java#newcode92
dev/core/test/com/google/gwt/dev/javac/CompilationUnitArchiveTest.java:92:
On 2011/06/03 19:48:40, jbrosenberg wrote:
unit1 -> unit, m2->archive
Done.
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/test/com/google/gwt/dev/javac/typemodel/JDelegatingClassTypeTestBase.java
File
dev/core/test/com/google/gwt/dev/javac/typemodel/JDelegatingClassTypeTestBase.java
(right):
http://gwt-code-reviews.appspot.com/1448808/diff/6001/dev/core/test/com/google/gwt/dev/javac/typemodel/JDelegatingClassTypeTestBase.java#newcode538
dev/core/test/com/google/gwt/dev/javac/typemodel/JDelegatingClassTypeTestBase.java:538:
* Test method for
On 2011/06/03 19:48:40, jbrosenberg wrote:
Is this method new (JDelegatingClassType#getTopModuleName())? Does it
need to
be included in this patch?
Whoops! Eclipse refactoring gone bad.
http://gwt-code-reviews.appspot.com/1448808/diff/1036/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java
File dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java (right):
http://gwt-code-reviews.appspot.com/1448808/diff/1036/dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java#newcode3
dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java:3: *
sorry about the auto-format jitter in this file.
http://gwt-code-reviews.appspot.com/1448808/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
