http://gwt-code-reviews.appspot.com/1448808/diff/6001/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/6001/dev/core/src/com/google/gwt/dev/CompileModule.java#newcode54
dev/core/src/com/google/gwt/dev/CompileModule.java:54: *
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?

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: Set<String>
archivedResourcePaths = new HashSet<String>();
Also add "moduleToCompile" attribute to this event?

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:
SpeedTracerLogger.Event loadArchive =
maybe change "module" to "dependentModule" here; or "subModule"?

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: // Pre-populate
CompilationStateBuilder with .gwt files
Add a comment for this if case?

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: if
(archive.getTopModuleName().equals(moduleToCompile)) {
I still don't see how this doesn't essentially throw away any caching
for the
top level module.  Can't it load units contained in it's archive, and
write them
back, along with any newly compiled ones, when you write back the
archive?

Otherwise, it seems no previously compiled, cached units can be used for
any top
level module, ever?

And since the first time a module is compiled, no archives would exist
for it,
or any of it's sub-modules, it seems then no caching would ever be
reused going
forward (since all units end up in the top level module, if there are no
compiled archives for sub-modules to start with)?  Am I missing
something?

Recursively, this would apply for sub-modules too, no?  They'd always
want to
recompile themselves entirely?

Isn't it simpler to only ever put units for a given module in it's
archive (and not include units for sub-modules)?  Then things are not
dependent so much on the order modules are processed, and separate
top-level modules which might share dependencies on sub-modules won't
ever duplicate storage of cached units, etc.

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#newcode207
dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java:207: * Free up memory
no longer needed in later compile stages. After calling this
s/ResourceOraclewill/ResourceOracle will/

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:
how about getAllCompilationUnitArchiveURLs()

Since it doesn't return a set of archives, but archiveURLs?

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:

javadoc?

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#newcode75
dev/core/src/com/google/gwt/dev/javac/CompilationUnitArchive.java:75:
javadoc?  explanation for topModuleName param?

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:
need to read/write topModuleName?

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:

unit1 -> unit, m2->archive

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
Is this method new (JDelegatingClassType#getTopModuleName())?  Does it
need to be included in this patch?

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

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

Reply via email to