[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
-~----------~----~----~----~------~----~------~--~---

Index: user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java
===================================================================
--- user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java	(revision 3687)
+++ user/src/com/google/gwt/user/rebind/rpc/TypeSerializerCreator.java	(working copy)
@@ -19,6 +19,7 @@
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.core.ext.GeneratorContext;
 import com.google.gwt.core.ext.TreeLogger;
+import com.google.gwt.core.ext.UnableToCompleteException;
 import com.google.gwt.core.ext.typeinfo.JClassType;
 import com.google.gwt.core.ext.typeinfo.JMethod;
 import com.google.gwt.core.ext.typeinfo.JParameterizedType;
@@ -32,6 +33,8 @@
 import com.google.gwt.user.rebind.SourceWriter;
 
 import java.io.PrintWriter;
+import java.util.ArrayList;
+import java.util.List;
 
 /**
  * This class generates a class with name 'typeSerializerClassName' that is able
@@ -52,6 +55,17 @@
       + "SerializationStreamWriter streamWriter, Object instance, String typeSignature)"
       + " throws SerializationException";
 
+  /**
+   * Default number of types to split createMethodMap entries into.  Zero means
+   * no sharding occurs.
+   */
+  private static final int DEFAULT_CREATEMETHODMAP_SHARD_SIZE = 0;
+
+  /**
+   * Java system property name to override the above.
+   */
+  private static final String GWT_CREATEMETHODMAP_SHARD_SIZE = "gwt.typecreator.shard.size";
+
   private final GeneratorContext context;
 
   private final JType[] serializableTypes;
@@ -77,7 +91,7 @@
     srcWriter = getSourceWriter(logger, context);
   }
 
-  public String realize(TreeLogger logger) {
+  public String realize(TreeLogger logger) throws UnableToCompleteException {
     logger = logger.branch(TreeLogger.DEBUG,
         "Generating TypeSerializer for service interface '"
             + getTypeSerializerClassName() + "'", null);
@@ -92,7 +106,7 @@
 
     writeCreateMethods();
 
-    writeCreateMethodMapMethod();
+    writeCreateMethodMapMethod(logger);
 
     writeCreateSignatureMapMethod();
 
@@ -173,13 +187,13 @@
       packageName = className.substring(0, index);
       className = className.substring(index + 1, className.length());
     }
-    return new String[]{packageName, className};
+    return new String[] {packageName, className};
   }
-  
+
   private JType[] getSerializableTypes() {
     return serializableTypes;
   }
-  
+
   private SourceWriter getSourceWriter(TreeLogger logger, GeneratorContext ctx) {
     String name[] = getPackageAndClassName(getTypeSerializerClassName());
     String packageName = name[0];
@@ -207,6 +221,16 @@
   }
 
   /**
+   * @param type
+   * @return
+   */
+  private String getTypeString(JType type) {
+    String typeString = serializationOracle.getSerializedTypeName(type) + "/"
+        + serializationOracle.getSerializationSignature(type);
+    return typeString;
+  }
+
+  /**
    * Return <code>true</code> if this type is concrete and has a custom field
    * serializer that does not declare an instantiate method.
    * 
@@ -239,98 +263,49 @@
     return true;
   }
 
-  private void writeCreateMethodMapMethod() {
-    srcWriter.println("@SuppressWarnings(\"restriction\")");
-    srcWriter.println("private static native JavaScriptObject createMethodMap() /*-" + '{');
-    {
-      srcWriter.indent();
-      srcWriter.println("return {");
-      JType[] types = getSerializableTypes();
-      boolean needComma = false;
-      for (int index = 0; index < types.length; ++index) {
-        JType type = types[index];
-        if (!serializationOracle.maybeInstantiated(type)) {
-          continue;
+  /**
+   * Generate the createMethodMap function, possibly splitting it into smaller
+   * pieces if necessary to avoid an old Mozilla crash when dealing with
+   * excessively large JS functions.
+   * 
+   * @param logger TreeLogger instance
+   * @throws UnableToCompleteException if an error is logged
+   */
+  private void writeCreateMethodMapMethod(TreeLogger logger)
+      throws UnableToCompleteException {
+    ArrayList<JType> filteredTypes = new ArrayList<JType>();
+    JType[] types = getSerializableTypes();
+    int n = types.length;
+    for (int index = 0; index < n; ++index) {
+      JType type = types[index];
+      if (serializationOracle.maybeInstantiated(type)) {
+        filteredTypes.add(type);
+      }
+    }
+    int shardSize = DEFAULT_CREATEMETHODMAP_SHARD_SIZE;
+    String shardSizeProperty = System.getProperty(GWT_CREATEMETHODMAP_SHARD_SIZE);
+    if (shardSizeProperty != null) {
+      try {
+        shardSize = Integer.valueOf(shardSizeProperty);
+        if (shardSize < 0) {
+          logger.log(TreeLogger.ERROR, GWT_CREATEMETHODMAP_SHARD_SIZE
+              + " must be positive: " + shardSizeProperty);
+          throw new UnableToCompleteException();
         }
-
-        if (needComma) {
-          srcWriter.println(",");
-        } else {
-          needComma = true;
-        }
-
-        String typeString = serializationOracle.getSerializedTypeName(type)
-            + "/" + serializationOracle.getSerializationSignature(type);
-
-        srcWriter.print("\"" + typeString + "\":");
-
-        // Make a JSON array
-        srcWriter.println("[");
-        {
-          srcWriter.indent();
-          String serializerName = serializationOracle.getFieldSerializerName(type);
-
-          // First the initialization method
-          {
-            srcWriter.print("@");
-            if (needsCreateMethod(type)) {
-              srcWriter.print(getTypeSerializerClassName());
-              srcWriter.print("::");
-              srcWriter.print(getCreateMethodName(type));
-            } else {
-              srcWriter.print(serializerName);
-              srcWriter.print("::instantiate");
-            }
-            srcWriter.print("(L"
-                + SerializationStreamReader.class.getName().replace('.', '/')
-                + ";)");
-            srcWriter.println(",");
-          }
-
-          JClassType customSerializer = serializationOracle.hasCustomFieldSerializer(type);
-
-          // Now the deserialization method
-          {
-            // Assume param type is the concrete type of the serialized type.
-            JType paramType = type;
-            if (customSerializer != null) {
-              // But a custom serializer may specify a looser type.
-              JMethod deserializationMethod = CustomFieldSerializerValidator.getDeserializationMethod(
-                  customSerializer, (JClassType) type);
-              paramType = deserializationMethod.getParameters()[1].getType();
-            }
-            srcWriter.print("@" + serializerName);
-            srcWriter.print("::deserialize(L"
-                + SerializationStreamReader.class.getName().replace('.', '/')
-                + ";" + paramType.getJNISignature() + ")");
-            srcWriter.println(",");
-          }
-
-          // Now the serialization method
-          {
-            // Assume param type is the concrete type of the serialized type.
-            JType paramType = type;
-            if (customSerializer != null) {
-              // But a custom serializer may specify a looser type.
-              JMethod serializationMethod = CustomFieldSerializerValidator.getSerializationMethod(
-                  customSerializer, (JClassType) type);
-              paramType = serializationMethod.getParameters()[1].getType();
-            }
-            srcWriter.print("@" + serializerName);
-            srcWriter.print("::serialize(L"
-                + SerializationStreamWriter.class.getName().replace('.', '/')
-                + ";" + paramType.getJNISignature() + ")");
-            srcWriter.println();
-          }
-          srcWriter.outdent();
-        }
-        srcWriter.print("]");
+      } catch (NumberFormatException e) {
+        logger.log(TreeLogger.ERROR, "Property "
+            + GWT_CREATEMETHODMAP_SHARD_SIZE + " not a number: "
+            + shardSizeProperty, e);
+        throw new UnableToCompleteException();
       }
-      srcWriter.println();
-      srcWriter.println("};");
-      srcWriter.outdent();
     }
-    srcWriter.println("}-*/;");
+    logger.log(TreeLogger.TRACE, "Using a shard size of " + shardSize
+        + " for TypeSerializerCreator createMethodMap");
+    if (shardSize > 0 && filteredTypes.size() > shardSize) {
+      writeShardedCreateMethodMapMethod(filteredTypes, shardSize);
+    } else {
+      writeSingleCreateMethodMapMethod(filteredTypes);
+    }
     srcWriter.println();
   }
 
@@ -468,9 +443,136 @@
     srcWriter.println();
   }
 
+  /**
+   * Create a createMethodMap method which is sharded into smaller methods. This
+   * avoids a crash in old Mozilla dealing with very large JS functions being
+   * evaluated.
+   * 
+   * @param types list of types to include
+   * @param shardSize batch size for sharding
+   */
+  private void writeShardedCreateMethodMapMethod(List<JType> types,
+      int shardSize) {
+    srcWriter.println("private static JavaScriptObject createMethodMap() {");
+    int n = types.size();
+    srcWriter.indent();
+    srcWriter.println("JavaScriptObject map = JavaScriptObject.createObject();");
+    for (int i = 0; i < n; i += shardSize) {
+      srcWriter.println("createMethodMap_" + i + "(map);");
+    }
+    srcWriter.println("return map;");
+    srcWriter.outdent();
+    srcWriter.println("}");
+    srcWriter.println();
+    for (int outerIndex = 0; outerIndex < n; outerIndex += shardSize) {
+      srcWriter.println("@SuppressWarnings(\"restriction\")");
+      srcWriter.println("private static native void createMethodMap_"
+          + outerIndex + "(JavaScriptObject map) /*-" + '{');
+      srcWriter.indent();
+      int last = outerIndex + shardSize;
+      if (last > n) {
+        last = n;
+      }
+      for (int i = outerIndex; i < last; ++i) {
+        JType type = types.get(outerIndex);
+        String typeString = getTypeString(type);
+        srcWriter.print("map[\"" + typeString + "\"]=[");
+        writeTypeMethods(type);
+        srcWriter.println("];");
+      }
+      srcWriter.outdent();
+      srcWriter.println("}-*/;");
+      srcWriter.println();
+    }
+  }
+
+  private void writeSingleCreateMethodMapMethod(List<JType> types) {
+    srcWriter.println("@SuppressWarnings(\"restriction\")");
+    srcWriter.println("private static native JavaScriptObject createMethodMap() /*-" + '{');
+    srcWriter.indent();
+    srcWriter.println("return {");
+    int n = types.size();
+    for (int i = 0; i < n; ++i) {
+      if (i > 0) {
+        srcWriter.println(",");
+      }
+      JType type = types.get(i);
+      String typeString = getTypeString(type);
+      srcWriter.print("\"" + typeString + "\":[");
+      writeTypeMethods(type);
+      srcWriter.print("]");
+    }
+    srcWriter.println("};");
+    srcWriter.outdent();
+    srcWriter.println("}-*/;");
+  }
+
   private void writeStaticFields() {
     srcWriter.println("private static final JavaScriptObject methodMap = createMethodMap();");
     srcWriter.println("private static final JavaScriptObject signatureMap = createSignatureMap();");
     srcWriter.println();
   }
-}
\ No newline at end of file
+
+  /**
+   * Write an entry in the createMethodMap method for one type.
+   * 
+   * @param type type to generate entry for
+   */
+  private void writeTypeMethods(JType type) {
+    srcWriter.indent();
+    String serializerName = serializationOracle.getFieldSerializerName(type);
+
+    // First the initialization method
+    {
+      srcWriter.print("@");
+      if (needsCreateMethod(type)) {
+        srcWriter.print(getTypeSerializerClassName());
+        srcWriter.print("::");
+        srcWriter.print(getCreateMethodName(type));
+      } else {
+        srcWriter.print(serializerName);
+        srcWriter.print("::instantiate");
+      }
+      srcWriter.print("(L"
+          + SerializationStreamReader.class.getName().replace('.', '/') + ";)");
+      srcWriter.println(",");
+    }
+
+    JClassType customSerializer = serializationOracle.hasCustomFieldSerializer(type);
+
+    // Now the deserialization method
+    {
+      // Assume param type is the concrete type of the serialized type.
+      JType paramType = type;
+      if (customSerializer != null) {
+        // But a custom serializer may specify a looser type.
+        JMethod deserializationMethod = CustomFieldSerializerValidator.getDeserializationMethod(
+            customSerializer, (JClassType) type);
+        paramType = deserializationMethod.getParameters()[1].getType();
+      }
+      srcWriter.print("@" + serializerName);
+      srcWriter.print("::deserialize(L"
+          + SerializationStreamReader.class.getName().replace('.', '/') + ";"
+          + paramType.getJNISignature() + ")");
+      srcWriter.println(",");
+    }
+
+    // Now the serialization method
+    {
+      // Assume param type is the concrete type of the serialized type.
+      JType paramType = type;
+      if (customSerializer != null) {
+        // But a custom serializer may specify a looser type.
+        JMethod serializationMethod = CustomFieldSerializerValidator.getSerializationMethod(
+            customSerializer, (JClassType) type);
+        paramType = serializationMethod.getParameters()[1].getType();
+      }
+      srcWriter.print("@" + serializerName);
+      srcWriter.print("::serialize(L"
+          + SerializationStreamWriter.class.getName().replace('.', '/') + ";"
+          + paramType.getJNISignature() + ")");
+      srcWriter.println();
+    }
+    srcWriter.outdent();
+  }
+}

Reply via email to