[gwt-contrib] Re: RR : Declassifying RPC / Removing all type names from compilation
I know this is a bit late, but is it true that we will now need to be more aggressive about deploying servlet classes? Is the obfuscated name mapping stable if new type names are added? On Wed, Feb 18, 2009 at 8:15 PM, BobV b...@google.com wrote: Thanks for the review. SerializerBase contains the comment Relies on monotonic behavior of hashcodes in web mode. I see no problem with this (and I've wanted to do so for JRE stuff at times), but I don't think we've ever explicitly stated this as a requirement of System.identityHashCode(). We should, at a minimum, document this requirement in our implementation of identityHashCode()/Impl.getHashCode(), so we don't inadvertently break it. Comments updated. SerializerBase.MethodMap is a JSO whose methods are non-final. Do you know why this is working? Do we only run JSO constraint-checking in hosted mode? MethodMap is a final class. Nits: ClientSerializationStreamWriter:57,82,96: JSNI reformat cruft. StandardSerializationPolicy:55: Double semicolons. ServerSerializationStreamWriter:412: Comment reformatting cruft (right-shifted notes). Fixed cruft. All that said, once you feel comfortable with it go ahead and commit away. I've run it against some pretty big projects without incident thus far, so... Good work! Merged to trunk at r4790. -- Bob Vawter Google Web Toolkit Team -- Eric Z. Ayers - GWT Team - Atlanta, GA USA http://code.google.com/webtoolkit/ --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: RR : Declassifying RPC / Removing all type names from compilation
On Tue, Feb 17, 2009 at 5:46 PM, BobV b...@google.com wrote: On Wed, Feb 18, 2009 at 8:50 AM, Joel Webber j...@google.com wrote: ControlFlowAnalyzer: - Just to make sure I understand this correctly, can you explain why we had to move the class-literal rescue code to JVariable? Because of ::class references; access to class literals from JSNI is handled as though it were a field reference. I could change JsniMethodBody.traverse() to examine it's field references and construct JClassLiterals to pass into JVisitor.accept() if you think it would be more clear as to what's gong on. Got it, that makes sense. JProgram/CompilingClassLoader: - It feels like there's got to be some way to merge the implementations of getTypeFromJsniRef() and getClassFromBinaryName(). Feel free to tell me it's not worth it, though. They operate on totally different data-types. getTypeFromJsniRef() needs access to JProgram fields, but getClassFromBinaryName() doesn't seem appropriate to add to JProgram. Fair enough. They look so close if you squint a little bit, but it's not worth convoluting the code to try and shove them together. It's not like someone's going to invent a new primitive Java type anytime soon. JsInliner: - (861) Why the comment about clinits being folded into the JsProgram body? I think I'm missing something here -- are we calling clinits in a context where they might get inlined directly into the outer scope? ClassLiteralHolder's clinit is effectively inlined into the JProgram in GenerateJavaScriptAST.generateClassLiterals. Understood. In the jjs tests, is there any eay way to share all those class name tests (i.e. startsWith(Class$)) without getting into heavy refactoring? It seems like we're only going to run into this sort of thing more as we start implementing optional optimizations (e.g. disable cast-checking), and it would be good to start a sensible pattern for these kinds of tests. I agree that there should be some common way of querying wether or not a particular optimization is applied for those tests that test for specific optimizations. Implement System.getProperties() to return compiler flags and module properties and a utility class to interpret them? Mostly just an observation. Something like that could work. Now's probably not the time, but I wanted to put the idea out there so we didn't forget about it. A few more comments follow: SerializerBase contains the comment Relies on monotonic behavior of hashcodes in web mode. I see no problem with this (and I've wanted to do so for JRE stuff at times), but I don't think we've ever explicitly stated this as a requirement of System.identityHashCode(). We should, at a minimum, document this requirement in our implementation of identityHashCode()/Impl.getHashCode(), so we don't inadvertently break it. SerializerBase.MethodMap is a JSO whose methods are non-final. Do you know why this is working? Do we only run JSO constraint-checking in hosted mode? Nits:ClientSerializationStreamWriter:57,82,96: JSNI reformat cruft. StandardSerializationPolicy:55: Double semicolons. ServerSerializationStreamWriter:412: Comment reformatting cruft (right-shifted notes). All that said, once you feel comfortable with it go ahead and commit away. I've run it against some pretty big projects without incident thus far, so... Good work! --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: RR : Declassifying RPC / Removing all type names from compilation
Thanks for the review. SerializerBase contains the comment Relies on monotonic behavior of hashcodes in web mode. I see no problem with this (and I've wanted to do so for JRE stuff at times), but I don't think we've ever explicitly stated this as a requirement of System.identityHashCode(). We should, at a minimum, document this requirement in our implementation of identityHashCode()/Impl.getHashCode(), so we don't inadvertently break it. Comments updated. SerializerBase.MethodMap is a JSO whose methods are non-final. Do you know why this is working? Do we only run JSO constraint-checking in hosted mode? MethodMap is a final class. Nits: ClientSerializationStreamWriter:57,82,96: JSNI reformat cruft. StandardSerializationPolicy:55: Double semicolons. ServerSerializationStreamWriter:412: Comment reformatting cruft (right-shifted notes). Fixed cruft. All that said, once you feel comfortable with it go ahead and commit away. I've run it against some pretty big projects without incident thus far, so... Good work! Merged to trunk at r4790. -- Bob Vawter Google Web Toolkit Team --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: RR : Declassifying RPC / Removing all type names from compilation
On Fri, Feb 6, 2009 at 5:10 PM, BobV b...@google.com wrote: On Fri, Feb 6, 2009 at 4:29 PM, Scott Blum sco...@google.com wrote: FYI: this will break some JRE code that depends on inspecting Class names of array types to do optimized things. This has been fixed in the branch. I've added a separate web-mode test pass to this branch to check the -XdisableClassMetadata flag. @ecc, this reduces showcase by about 10%. I would suspect that in an RPC-heavy application, this flag combined with enabling obfuscated idents in RPC payloads, should produce similar savings. -- Bob Vawter Google Web Toolkit Team --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: RR : Declassifying RPC / Removing all type names from compilation
branch to check the -XdisableClassMetadata flag. @ecc, this reduces showcase by about 10%. I would suspect that in an RPC-heavy application, this flag combined with enabling obfuscated idents in RPC payloads, should produce similar savings. WOOT! -- Bob Vawter Google Web Toolkit Team -- There are only 10 types of people in the world: Those who understand binary, and those who don't --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: RR : Declassifying RPC / Removing all type names from compilation
Joel has agreed to handle the review of this branch. Now that all of the web-mode tests pass with -XdisableClassMetadata, I'm calling the branch done-enough for review and integration. Here's a quick summary of tha changes made. svn merge -r4602:HEAD http://google-web-toolkit.googlecode.com/svn/changes/bobv/elide_rpc_type_names_r4602 Major changes: - Added configuration property to change generated RPC code to replace type names on the wire with short idents. - Allow JSNI to refer to class literals via (@com.foo.Bar::class) syntax. - Adds an -XdisableClassMetadata flag to reduce the functionality of java.lang.Class in compiled code. - Class.getName() - returns Class$hashCode() - Class.getSuperclass() - returns null Changes grouped by functional area: Add -XdisableClassMetadata flag: dev/core/src/com/google/gwt/dev/HostedModeBase.java |2 2 + 0 - 0 ! dev/core/src/com/google/gwt/dev/Precompile.java | 10 10 +0 - 0 ! dev/core/src/com/google/gwt/dev/jjs/JJSOptions.java |5 3 + 2 - 0 ! dev/core/src/com/google/gwt/dev/jjs/JJSOptionsImpl.java | 10 10 +0 - 0 ! dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java |2 1 + 1 - 0 ! dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerDisableClassMetadata.java | 4646 +0 - 0 ! dev/core/src/com/google/gwt/dev/util/arg/OptionDisableClassMetadata.java | 2525 +0 - 0 ! Just option-handling code to define the flag. Allow JSNI access to ::class literals: dev/core/src/com/google/gwt/dev/jjs/ast/JArrayType.java |1 1 + 0 - 0 ! dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java | 35 21 +14 -0 ! dev/core/src/com/google/gwt/dev/js/rhino/TokenStream.java | 11 11 +0 - 0 ! dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java | 46 41 +5 - 0 ! dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java |4 3 + 1 - 0 ! dev/core/src/com/google/gwt/dev/shell/JavaDispatchImpl.java | 25 22 +3 - 0 ! dev/core/src/com/google/gwt/dev/shell/SyntheticClassMember.java | 46 46 +0 - 0 ! dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java | 42 42 +0 - 0 ! The idea is that in web mode, the compiler will implicitly add a field named ::class to every type that's defined. It's necessary to have JProgram traverse all defined JArrayTypes as well as having JArrayType traverse its fields to make this just work with existing JSNI dispatch code. In hosted mode, we make ::class act as though there's actually a Field in the class with that name. The SyntheticClassMember is used since Field is a final type. TokenStream is modified slightly to allow @com.foo.Bar[][]::class syntax for referring to array types. Primitive types are referenced as @Z::class. Implement metadata removal: /dev/core/src/com/google/gwt/dev/jjs/ast/JClassLiteral.java | 2322 +1 - 0 ! dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java | 76 65 +11 -0 ! user/super/com/google/gwt/emul/java/lang/Class.java | 26 22 +4 - 0 ! user/super/com/google/gwt/emul/java/lang/Enum.java |7 6 + 1 - 0 ! user/super/com/google/gwt/emul/java/lang/System.java | 21 12 +9 - 0 ! user/super/com/google/gwt/emul/java/util/Arrays.java |8 4 + 4 - 0 ! Class literals for array types are now able to specify their component type, which removes the dependency on type names from old JRE emulation code. GenerateJavaAST rewrites java.lang.Class to remove reads from the typename field. Calls to Class.createForArray() have an extra parameter specifying the component type. Class literals for enum types are always created with Class.createForEnum(). In the general case, the generated JS for a regular Class literal is simply foo_classList = new Class();. Generated code improvements: dev/core/src/com/google/gwt/dev/js/JsInliner.java | 172 139 + 33 -0 ! Reduces var foo; foo = bar; return foo; to bar. Allows inlining of expressions into the top-level JsProgram, except for top-level function invocations. This allows class literal construction for regular classes to be new Class(). Run web-mode tests with -XdisableClassMetadata flag on: user/build.xml |9 9 + 0 - 0 ! Remove type names from RPC: user/src/com/google/gwt/user/RemoteService.gwt.xml |5
[gwt-contrib] Re: RR : Declassifying RPC / Removing all type names from compilation
Do you have any numbers of how much this shrinks compiled output, as this seems like a terrific change! Thanks for doing it :-). Cheers, Emily On Fri, Feb 6, 2009 at 1:21 PM, BobV b...@google.com wrote: On Thu, Feb 5, 2009 at 12:28 AM, BobV b...@google.com wrote: http://google-web-toolkit.googlecode.com/svn/changes/bobv/elide_rpc_type_names_r4602 Future changes: - A compiler flag or optional module to neuter Class.getName(). I've added an -XdisableClassMetadata flag to this branch. This disables: - Class.getName() - returns Class$hashCode() - Class.getSuperclass() - returns null - Class.toString() - returns Object.toString() You can still use class.getName() as an equality comparison, but not for anything that requires semantic meaning; the relationships between the strings will be constant throughout the lifetime of the module. A.class == A.class != B.class A.class.getName() == A.class.getName() != B.class.getName() With this flag enabled, class literal setup for non-enum types is a simple JS new operation. It reduces the total amount of code generated in a showcase compile by 10% based on find war -name '*.cache.html' -o -name '*.cache.js' | xargs wc. -- Bob Vawter Google Web Toolkit Team -- There are only 10 types of people in the world: Those who understand binary, and those who don't --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: RR : Declassifying RPC / Removing all type names from compilation
On Thu, Feb 5, 2009 at 12:28 AM, BobV b...@google.com wrote: http://google-web-toolkit.googlecode.com/svn/changes/bobv/elide_rpc_type_names_r4602 Future changes: - A compiler flag or optional module to neuter Class.getName(). I've added an -XdisableClassMetadata flag to this branch. This disables: - Class.getName() - returns Class$hashCode() - Class.getSuperclass() - returns null - Class.toString() - returns Object.toString() You can still use class.getName() as an equality comparison, but not for anything that requires semantic meaning; the relationships between the strings will be constant throughout the lifetime of the module. A.class == A.class != B.class A.class.getName() == A.class.getName() != B.class.getName() With this flag enabled, class literal setup for non-enum types is a simple JS new operation. It reduces the total amount of code generated in a showcase compile by 10% based on find war -name '*.cache.html' -o -name '*.cache.js' | xargs wc. -- Bob Vawter Google Web Toolkit Team --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
[gwt-contrib] Re: RR : Declassifying RPC / Removing all type names from compilation
FYI: this will break some JRE code that depends on inspecting Class names of array types to do optimized things. On Fri, Feb 6, 2009 at 1:21 PM, BobV b...@google.com wrote: On Thu, Feb 5, 2009 at 12:28 AM, BobV b...@google.com wrote: http://google-web-toolkit.googlecode.com/svn/changes/bobv/elide_rpc_type_names_r4602 Future changes: - A compiler flag or optional module to neuter Class.getName(). I've added an -XdisableClassMetadata flag to this branch. This disables: - Class.getName() - returns Class$hashCode() - Class.getSuperclass() - returns null - Class.toString() - returns Object.toString() You can still use class.getName() as an equality comparison, but not for anything that requires semantic meaning; the relationships between the strings will be constant throughout the lifetime of the module. A.class == A.class != B.class A.class.getName() == A.class.getName() != B.class.getName() With this flag enabled, class literal setup for non-enum types is a simple JS new operation. It reduces the total amount of code generated in a showcase compile by 10% based on find war -name '*.cache.html' -o -name '*.cache.js' | xargs wc. -- Bob Vawter Google Web Toolkit Team --~--~-~--~~~---~--~~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~--~~~~--~~--~--~---
