http://gwt-code-reviews.appspot.com/1384807/diff/1/dev/core/src/com/google/gwt/dev/javac/CachedCompilationUnit.java
File dev/core/src/com/google/gwt/dev/javac/CachedCompilationUnit.java
(right):
http://gwt-code-reviews.appspot.com/1384807/diff/1/dev/core/src/com/google/gwt/dev/javac/CachedCompilationUnit.java#newcode166
dev/core/src/com/google/gwt/dev/javac/CachedCompilationUnit.java:166:
sourceToken = diskCache.writeByteArray(sourceBytes);
On 2011/03/24 22:42:16, scottb wrote:
Yeah, I did think about this, and I agree it's ugly. The reason I
left it this
way is that CU.getSource() is going to go away soon. But if you think
it's
worth it, we could add methods to DiskCache that write the size.
Alternatively, we could wrap the raw tokens in a wrapper object that
does
nothing but serialize itself, and allow serializable token holders to
use that
class and default serialization.
class CachedCompilationUnit {
DiskCacheToken ast; // not transient
DiskCacheToken source; // not transient
}
class DiskCacheToken {
transient long token;
readObject()/writeObject()
}
Thoughts?
I like the encapsulation idea. When I worked on CachedCompilationUnit,
serialzing all those source files was getting a bit confusing.
http://gwt-code-reviews.appspot.com/1384807/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
File dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1384807/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java#newcode204
dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:204:
final ArrayList<CompilationUnit> newlyBuiltUnits = new
ArrayList<CompilationUnit>();
On 2011/03/24 22:42:16, scottb wrote:
On 2011/03/24 17:43:42, zundel wrote:
> First of all, be aware that this block is going to have a lot of
conflicts
with
> the PersistentUnitCache patch that's up.
Guess I'll try to get this in quickly then :P
Too late - time to man, I mean merge up.
> I'm not 100% sure what time you're trying to save in here by putting
this in a
> thread; could you add a comment in the body of the thread as to what
is
> expensive? I guess its the calculation of contentId.
Actually, it's converting the GWT AST to serialized bytes that I'm
saving. Will
add a comment.
> Warning - I think you're
> in thread unsafeness land in ResourceCompilationUnitBuilder when the
compiler
> tries to lazily compute contentId, waiting until the source is read.
That happens on the main thread when JDT reads the source to do the
compile.
Content id is set then, and I can definitely read it because the
builder had to
pass through the buildQueue barriers.
> Did you measure the amount of time saved? It would be nice to see
some
> speedtracer calls around it.
I found that dropping events in here was too fine-grain to be useful,
but the
macro effect that I compared made a definitely difference. Really,
what this is
doing is avoiding the wall time penalty for serializing the build GWT
AST to
bytes by doing it concurrently.
I think there is a way to make some useful speedtracer output, but I'll
experiment once the code is merged.
http://gwt-code-reviews.appspot.com/1384807/diff/3003/dev/core/src/com/google/gwt/dev/javac/CompilationUnitImpl.java
File dev/core/src/com/google/gwt/dev/javac/CompilationUnitImpl.java
(right):
http://gwt-code-reviews.appspot.com/1384807/diff/3003/dev/core/src/com/google/gwt/dev/javac/CompilationUnitImpl.java#newcode65
dev/core/src/com/google/gwt/dev/javac/CompilationUnitImpl.java:65: try {
If we commit this as-is, are we going to suffer temporarily from
performing more work than we need to? We aren't using these types yet,
are we? Maybe protect behind a flag until we use them?
http://gwt-code-reviews.appspot.com/1384807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
