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); seems awkward to encode/decode a byte array two separate ways like this. DiskCache knows the size, it just doesn't encode it when we call transferToStream. 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#newcode129 dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:129: builder.setClasses(compiledClasses).setTypes(types).setDependencies(dependencies) (aside: this is what I get out of the eclipse formatter too, but the other instance of using the Builder pattern is much more nicely formatted.) 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>(); First of all, be aware that this block is going to have a lot of conflicts with the PersistentUnitCache patch that's 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. 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. Did you measure the amount of time saved? It would be nice to see some speedtracer calls around it. http://gwt-code-reviews.appspot.com/1384807/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java#newcode205 dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:205: final CompilationUnitBuilder sentinal = CompilationUnitBuilder.create((GeneratedUnit) null); spelling: sentinel http://gwt-code-reviews.appspot.com/1384807/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java#newcode253 dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java:253: } null out buildQueue? http://gwt-code-reviews.appspot.com/1384807/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationUnit.java File dev/core/src/com/google/gwt/dev/javac/CompilationUnit.java (right): http://gwt-code-reviews.appspot.com/1384807/diff/1/dev/core/src/com/google/gwt/dev/javac/CompilationUnit.java#newcode316 dev/core/src/com/google/gwt/dev/javac/CompilationUnit.java:316: throw new RuntimeException("Unexpected error deserializing AST", e); this might actually happen if the serialized object is old and the compiler internals have changed - want to put the unit name in the exception so we know what to clean up? http://gwt-code-reviews.appspot.com/1384807/diff/1/user/test/com/google/gwt/dev/jjs/GwtAstBuilderTest.java File user/test/com/google/gwt/dev/jjs/GwtAstBuilderTest.java (right): http://gwt-code-reviews.appspot.com/1384807/diff/1/user/test/com/google/gwt/dev/jjs/GwtAstBuilderTest.java#newcode145 user/test/com/google/gwt/dev/jjs/GwtAstBuilderTest.java:145: } Shouldn't you also loop through the compiled classes in compilationState and make sure there is a corresponding deserialized type? This test won't catch if there are values in compilationState that didn't make it through the serialize/deserialize process. http://gwt-code-reviews.appspot.com/1384807/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
