http://gwt-code-reviews.appspot.com/1215801/diff/1/4 File dev/core/src/com/google/gwt/dev/javac/StandardGeneratorContext.java (right):
http://gwt-code-reviews.appspot.com/1215801/diff/1/4#newcode231 dev/core/src/com/google/gwt/dev/javac/StandardGeneratorContext.java:231: eventsByGeneratorType.put("com.google.gwt.resources.rebind.context.InlineClientBundleGenerator", CompilerEventType.GENERATOR_CLIENT_BUNDLE); On 2010/12/14 14:33:58, zundel wrote:
line length > 100
Done. http://gwt-code-reviews.appspot.com/1215801/diff/1/4#newcode231 dev/core/src/com/google/gwt/dev/javac/StandardGeneratorContext.java:231: eventsByGeneratorType.put("com.google.gwt.resources.rebind.context.InlineClientBundleGenerator", CompilerEventType.GENERATOR_CLIENT_BUNDLE); On 2010/12/14 14:33:58, zundel wrote:
Wouldn't it be more robust to refer to these types by class constants?
I know it
would introduce dependencies through import statements, but in effect,
the code
does depend on these classes by using their package names like this,
and will
silently break if there is some refactoring.
It would be a reverse dependency (bad), and also wouldn't even compile. If something changes causing it to "fail", the result would be that the time would get lumped in as GENERATOR_OTHER and the data would include the generator class. I think that's fine.
Or maybe it would be better to include an extendable method for
Generator to
return a SpeedTracer event type.
I don't think we should head in that direction. I'd rather see us improve on tools to slice and dice events by data, so that I could decide after the fact that I want to see a breakdown of GENERATOR events by typename. http://gwt-code-reviews.appspot.com/1215801/diff/1/6 File dev/core/src/com/google/gwt/dev/shell/ModuleSpaceOOPHM.java (right): http://gwt-code-reviews.appspot.com/1215801/diff/1/6#newcode49 dev/core/src/com/google/gwt/dev/shell/ModuleSpaceOOPHM.java:49: if (jsniMethod.isScriptOnly()) { On 2010/12/14 14:33:58, zundel wrote:
I would appreciate a comment here that this is for a performance
optimization. I added a reference from JsniMethod.isScriptOnly to GwtScriptOnly which documents this as a performance optimization. http://gwt-code-reviews.appspot.com/1215801/diff/1/13 File user/src/com/google/gwt/user/client/rpc/impl/ReflectionHelper.java (right): http://gwt-code-reviews.appspot.com/1215801/diff/1/13#newcode27 user/src/com/google/gwt/user/client/rpc/impl/ReflectionHelper.java:27: // as the syntax is correct, e.g. "[[Ljava.lang.String;" On 2010/12/14 14:33:58, zundel wrote:
I'm not sure its so surprising the names match, since the SerializationUtils.getRpcTypeName() is explicitly documented as such.
Maybe
reference that method?
There's intentionally no direct relationship to SerializationUtils.getRpcTypeName and ReflectionHelper. I expect that we'll eventually move this class out of rpc. I added method javadoc and dropped the comment since it's clear from Class.forName javadoc that it accepts Class.getName syntax. http://gwt-code-reviews.appspot.com/1215801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
