Mostly LGTM. The only (marginally) substantive complaint is that it may
make sense to rework the property parsing around lines 285-293 so that it's
done once, statically, rather than re-parsing the String->int conversion for
every TypeSerializer we create, especially if (as I suggest as a nit) you
change to use the System.getProperty(String propname, String default)
formulation. Not that parsing a short string into int is any large amount
lot of work, but we may end up doing it often...
Nits
- Can we note, probably in the comments around lines 58-66, that the
shard size is measured in number of reachable types, not in e.g.
characters? (Yeah, I know characters would be hard to get at *a priori*,
but since our issue seems to be string length of the JSNI body, that's what
I expected size to be measured in.)
- Also mentioned above, line 285-286, why avoid the
System.getProperty(String prop, String default) formulation? (You'd have to
make the default a string, yes, but it saves you the if-null test.)
- I'll take your word for the style-correctness of the added space on
line 190.
On Tue, Sep 30, 2008 at 2:59 PM, John Tamplin <[EMAIL PROTECTED]> wrote:
> [fix bad GWTC address]
>
>
> Please review the attached patch for trunk, which fixes a hosted mode crash
> with a large set of serializable types.
>
> On Linux hosted mode, a very large list of serializable types can cause a
> crash inside Mozilla. This appears to be related to the size of the JSNI
> function being evaluated to inject the code into the browser.
>
> This patch adds a Java system property to control this behavior, and code
> to split the createMethodMap if the numer of types exceeeds the specified
> threshold. If no parameter is supplied, the output code is unchanged.
> After we get some feedback on exactly what this threshold should be, we may
> change it to a reasonable default. This parameter may also go away once
> OOPHM is merged into trunk if it is no longer needed. So, this should be
> considered a short-term patch.
>
> The relevant generated code previously looked like this:
>
> public static native JavaScriptObject createMethodMap() /*-{
> return {
> "[Lcom.google.gwt.sample.dynatable.client.Person;/3792876907": [
> @com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::instantiate(Lcom/google/gwt/user/client/rpc/SerializationStreamReader;),
>
> @com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::deserialize(Lcom/google/gwt/user/client/rpc/SerializationStreamReader;[Lcom/google/gwt/sample/dynatable/client/Person;),
>
> @com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::serialize(Lcom/google/gwt/user/client/rpc/SerializationStreamWriter;[Lcom/google/gwt/sample/dynatable/client/Person;)
> ],
> ...
> };
> }-*/;
>
> it now looks like this if the threshold is exceeded:
>
> public static JavaScriptObject createMethodMap() {
> JavaScriptObject map = JavaScriptObject.createObject();
> createMethodMap_0(map);
> createMethodMap_50(map);
> ...
> return map;
> }
>
> public static native void createMethodMap_0(JavaScriptObject map) /*-{
> map["[Lcom.google.gwt.sample.dynatable.client.Person;/3792876907"]=[
> @com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::instantiate(Lcom/google/gwt/user/client/rpc/SerializationStreamReader;),
>
> @com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::deserialize(Lcom/google/gwt/user/client/rpc/SerializationStreamReader;[Lcom/google/gwt/sample/dynatable/client/Person;),
>
> @com.google.gwt.sample.dynatable.client.Person_Array_Rank_1_FieldSerializer::serialize(Lcom/google/gwt/user/client/rpc/SerializationStreamWriter;[Lcom/google/gwt/sample/dynatable/client/Person;)
> ];
> }-*/;
>
> Note that the resulting code will be slightly larger and slower than
> before, so this should only be used for Linux hosted mode where needed.
> Since the default behavior is unchanged, if you do nothing there will be no
> impact on the output.
>
> --
> John A. Tamplin
> Software Engineer (GWT), Google
>
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
