Hi all,
I have attached a patch for TreeMap Serialization. The patch has been
reviewed by John Tamplin. Most of the code is similar to the way we do
serialization/deserialization for HashMap. The deserialization is done
element by element and the tree is being built incrementally. John pointed
out that this approach is fine for now, since the TreeMap implementation
currently does not use the SortedMap constructor.
There is just one drawback to this code going in 1_5_3. To get Stob to
generate serializers for the key and value types, based on Lex's suggestion,
I created two private variables that are not being used anywhere. Since my
tests run correctly, either the compiler does not optimize away these two
variables or Stob is looking at the code before these are optimized away? In
both situations, there seem to be optimization opportunities. Moreover, in
the former case, anyone using TreeMap would suffer an increase in codesize
regardless of whether they use it for serialization or not. Am I missing
something here?
Thoughts about whether this code should go in 1_5_3 or not?
Regards,
Amit
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
Index: user/test/com/google/gwt/user/server/rpc/CollectionsTestServiceImpl.java
===================================================================
--- user/test/com/google/gwt/user/server/rpc/CollectionsTestServiceImpl.java
(revision 3731)
+++ user/test/com/google/gwt/user/server/rpc/CollectionsTestServiceImpl.java
(working copy)
@@ -24,6 +24,7 @@
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeHashSet;
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeLinkedHashMap;
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeLinkedHashSet;
+import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeTreeMap;
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeVector;
import java.sql.Time;
@@ -36,6 +37,7 @@
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
+import java.util.TreeMap;
import java.util.Vector;
/**
@@ -328,6 +330,18 @@
return actual;
}
+ public TreeMap<String, MarkerTypeTreeMap> echo(
+ TreeMap<String, MarkerTypeTreeMap> actual, boolean option)
+ throws CollectionsTestServiceException {
+ TreeMap<String, MarkerTypeTreeMap> expected =
TestSetFactory.createTreeMap(option);
+ if (!TestSetValidator.isValid(expected, actual)) {
+ throw new CollectionsTestServiceException("expected: "
+ + expected.toString() + " actual: " + actual.toString());
+ }
+
+ return actual;
+ }
+
public Vector<MarkerTypeVector> echo(Vector<MarkerTypeVector> actual)
throws CollectionsTestServiceException {
Vector<MarkerTypeVector> expected = TestSetFactory.createVector();
@@ -348,4 +362,5 @@
return value;
}
+
}
Index: user/test/com/google/gwt/user/client/rpc/TestSetValidator.java
===================================================================
--- user/test/com/google/gwt/user/client/rpc/TestSetValidator.java
(revision 3731)
+++ user/test/com/google/gwt/user/client/rpc/TestSetValidator.java
(working copy)
@@ -15,6 +15,7 @@
*/
package com.google.gwt.user.client.rpc;
+import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeTreeMap;
import
com.google.gwt.user.client.rpc.TestSetFactory.SerializableDoublyLinkedNode;
import com.google.gwt.user.client.rpc.TestSetFactory.SerializablePrivateNoArg;
import com.google.gwt.user.client.rpc.TestSetFactory.SerializableWithTwoArrays;
@@ -26,7 +27,9 @@
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
+import java.util.Map;
import java.util.Set;
+import java.util.TreeMap;
import java.util.Vector;
import java.util.Map.Entry;
@@ -318,6 +321,36 @@
return node.one == node.two;
}
+ // also checks whether the sorting of entries is maintained or not.
+ public static boolean isValid(TreeMap<String, MarkerTypeTreeMap> expected,
+ TreeMap<String, MarkerTypeTreeMap> map) {
+ if (map == null) {
+ return false;
+ }
+ if (!equalsWithNullCheck(map.comparator(), expected.comparator())) {
+ return false;
+ }
+ int size = 0;
+ if ((size = expected.size()) != map.size()) {
+ return false;
+ }
+ // entrySet returns entries in the sorted order
+ List<Map.Entry<String, MarkerTypeTreeMap>> actualList = new
ArrayList<Map.Entry<String, MarkerTypeTreeMap>>(
+ map.entrySet());
+ List<Map.Entry<String, MarkerTypeTreeMap>> expectedList = new
ArrayList<Map.Entry<String, MarkerTypeTreeMap>>(
+ expected.entrySet());
+ for (int index = 0; index < size; index++) {
+ Entry<String, MarkerTypeTreeMap> expectedEntry = expectedList.get(index);
+ Entry<String, MarkerTypeTreeMap> actualEntry = actualList.get(index);
+ if (!equalsWithNullCheck(expectedEntry.getKey(), actualEntry.getKey())
+ || !equalsWithNullCheck(expectedEntry.getValue(),
+ actualEntry.getValue())) {
+ return false;
+ }
+ }
+ return true;
+ }
+
public static boolean isValid(Vector expected, Vector actual) {
if (actual == null) {
return false;
@@ -454,4 +487,8 @@
}
}
+ private static boolean equalsWithNullCheck(Object a, Object b) {
+ return a == b || (a != null && a.equals(b));
+ }
+
}
Index: user/test/com/google/gwt/user/client/rpc/CollectionsTest.java
===================================================================
--- user/test/com/google/gwt/user/client/rpc/CollectionsTest.java
(revision 3731)
+++ user/test/com/google/gwt/user/client/rpc/CollectionsTest.java
(working copy)
@@ -22,6 +22,7 @@
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeHashMap;
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeHashSet;
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeLinkedHashMap;
+import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeTreeMap;
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeVector;
import java.sql.Time;
@@ -32,6 +33,7 @@
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
+import java.util.TreeMap;
import java.util.Vector;
/**
@@ -591,6 +593,27 @@
});
}
+ public void testTreeMap() {
+ delayTestFinish(TEST_DELAY);
+
+ CollectionsTestServiceAsync service = getServiceAsync();
+ for (boolean option : new boolean[] {true, false}) {
+ final TreeMap<String, MarkerTypeTreeMap> expected =
TestSetFactory.createTreeMap(option);
+ service.echo(expected, option,
+ new AsyncCallback<TreeMap<String, MarkerTypeTreeMap>>() {
+ public void onFailure(Throwable caught) {
+ TestSetValidator.rethrowException(caught);
+ }
+
+ public void onSuccess(TreeMap<String, MarkerTypeTreeMap> result) {
+ assertNotNull(result);
+ assertTrue(TestSetValidator.isValid(expected, result));
+ finishTest();
+ }
+ });
+ }
+ }
+
public void testVector() {
delayTestFinish(TEST_DELAY);
Index: user/test/com/google/gwt/user/client/rpc/TestSetFactory.java
===================================================================
--- user/test/com/google/gwt/user/client/rpc/TestSetFactory.java
(revision 3731)
+++ user/test/com/google/gwt/user/client/rpc/TestSetFactory.java
(working copy)
@@ -15,16 +15,19 @@
*/
package com.google.gwt.user.client.rpc;
+import java.io.Serializable;
import java.sql.Time;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Comparator;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
+import java.util.TreeMap;
import java.util.Vector;
/**
@@ -157,6 +160,21 @@
* A single-use marker type to independently check type parameter exposure in
* various collections.
*/
+ public static final class MarkerTypeTreeMap extends MarkerBase {
+
+ public MarkerTypeTreeMap(String value) {
+ super(value);
+ }
+
+ MarkerTypeTreeMap() {
+ super(null);
+ }
+ }
+
+ /**
+ * A single-use marker type to independently check type parameter exposure in
+ * various collections.
+ */
public static final class MarkerTypeVector extends MarkerBase {
public MarkerTypeVector(String value) {
@@ -236,6 +254,34 @@
public static class UnserializableNode {
}
+ static class ReverseSorter implements Comparator<String>, Serializable {
+
+ // for gwt-serialization
+ ReverseSorter() {
+ }
+
+ public int compare(String a, String b) {
+ // Explicit null check to match JRE specs
+ if (a == null || b == null) {
+ throw new NullPointerException();
+ }
+ return b.compareTo(a);
+ }
+
+ @Override
+ public int hashCode() {
+ return 0;
+ }
+
+ @Override
+ public boolean equals(Object ob) {
+ if (!(ob instanceof ReverseSorter)) {
+ return false;
+ }
+ return true;
+ }
+ }
+
public static ArrayList<MarkerTypeArrayList> createArrayList() {
ArrayList<MarkerTypeArrayList> list = new ArrayList<MarkerTypeArrayList>();
list.add(new MarkerTypeArrayList("foo"));
@@ -425,6 +471,22 @@
"valueOf", "constructor", "__proto__"};
}
+ public static TreeMap<String, MarkerTypeTreeMap> createTreeMap(
+ boolean defaultComparator) {
+ TreeMap<String, MarkerTypeTreeMap> map;
+ if (defaultComparator) {
+ map = new TreeMap<String, MarkerTypeTreeMap>();
+ } else {
+ map = new TreeMap<String, MarkerTypeTreeMap>(new ReverseSorter());
+ }
+ map.put("foo", new MarkerTypeTreeMap("foo"));
+ map.put("bar", new MarkerTypeTreeMap("bar"));
+ map.put("baz", new MarkerTypeTreeMap("baz"));
+ map.put("bal", new MarkerTypeTreeMap("bal"));
+ map.put("w00t", new MarkerTypeTreeMap("w00t"));
+ return map;
+ }
+
public static Vector<MarkerTypeVector> createVector() {
Vector<MarkerTypeVector> vector = new Vector<MarkerTypeVector>();
vector.add(new MarkerTypeVector("foo"));
Index: user/test/com/google/gwt/user/client/rpc/CollectionsTestService.java
===================================================================
--- user/test/com/google/gwt/user/client/rpc/CollectionsTestService.java
(revision 3731)
+++ user/test/com/google/gwt/user/client/rpc/CollectionsTestService.java
(working copy)
@@ -21,6 +21,7 @@
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeHashSet;
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeLinkedHashMap;
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeLinkedHashSet;
+import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeTreeMap;
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeVector;
import java.sql.Time;
@@ -32,6 +33,7 @@
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
+import java.util.TreeMap;
import java.util.Vector;
/**
@@ -113,6 +115,10 @@
Timestamp[] echo(Timestamp[] value) throws CollectionsTestServiceException;
+ TreeMap<String, MarkerTypeTreeMap> echo(
+ TreeMap<String, MarkerTypeTreeMap> value, boolean option)
+ throws CollectionsTestServiceException;
+
Vector<MarkerTypeVector> echo(Vector<MarkerTypeVector> value)
throws CollectionsTestServiceException;
Index: user/test/com/google/gwt/user/client/rpc/CollectionsTestServiceAsync.java
===================================================================
--- user/test/com/google/gwt/user/client/rpc/CollectionsTestServiceAsync.java
(revision 3731)
+++ user/test/com/google/gwt/user/client/rpc/CollectionsTestServiceAsync.java
(working copy)
@@ -21,6 +21,7 @@
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeHashSet;
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeLinkedHashMap;
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeLinkedHashSet;
+import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeTreeMap;
import com.google.gwt.user.client.rpc.TestSetFactory.MarkerTypeVector;
import java.sql.Time;
@@ -32,6 +33,7 @@
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
+import java.util.TreeMap;
import java.util.Vector;
/**
@@ -94,6 +96,9 @@
void echo(String[][] value, AsyncCallback<String[][]> callback);
void echo(Time[] value, AsyncCallback<Time[]> callback);
+
+ void echo(TreeMap<String, MarkerTypeTreeMap> value, boolean option,
+ AsyncCallback<TreeMap<String, MarkerTypeTreeMap>> callback);
void echo(Timestamp[] value, AsyncCallback<Timestamp[]> callback);
Index:
user/src/com/google/gwt/user/client/rpc/core/java/util/TreeMap_CustomFieldSerializer.java
===================================================================
---
user/src/com/google/gwt/user/client/rpc/core/java/util/TreeMap_CustomFieldSerializer.java
(revision 0)
+++
user/src/com/google/gwt/user/client/rpc/core/java/util/TreeMap_CustomFieldSerializer.java
(revision 0)
@@ -0,0 +1,47 @@
+/*
+ * Copyright 2008 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy
of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+package com.google.gwt.user.client.rpc.core.java.util;
+
+import com.google.gwt.user.client.rpc.SerializationException;
+import com.google.gwt.user.client.rpc.SerializationStreamReader;
+import com.google.gwt.user.client.rpc.SerializationStreamWriter;
+
+import java.util.Comparator;
+import java.util.TreeMap;
+
+/**
+ * Custom field serializer for [EMAIL PROTECTED] java.util.TreeMap}.
+ */
[EMAIL PROTECTED]("unchecked")
+public class TreeMap_CustomFieldSerializer {
+
+ /* for now, build it entry by entry. Can optimize later via bulk loading */
+ public static void deserialize(SerializationStreamReader streamReader,
+ TreeMap instance) throws SerializationException {
+ Map_CustomFieldSerializerBase.deserialize(streamReader, instance);
+ }
+
+ public static TreeMap instantiate(SerializationStreamReader streamReader)
+ throws SerializationException {
+ return new TreeMap((Comparator) streamReader.readObject());
+ }
+
+ public static void serialize(SerializationStreamWriter streamWriter,
+ TreeMap instance) throws SerializationException {
+ streamWriter.writeObject(instance.comparator());
+ Map_CustomFieldSerializerBase.serialize(streamWriter, instance);
+ }
+}
Index: user/super/com/google/gwt/emul/java/util/TreeMap.java
===================================================================
--- user/super/com/google/gwt/emul/java/util/TreeMap.java (revision 3731)
+++ user/super/com/google/gwt/emul/java/util/TreeMap.java (working copy)
@@ -15,6 +15,8 @@
*/
package java.util;
+import java.io.Serializable;
+
/**
* Implements a TreeMap using a red-black tree. This guarantees O(log n)
* performance on lookups, inserts, and deletes while maintaining linear
@@ -24,7 +26,8 @@
* @param <K> key type
* @param <V> value type
*/
-public class TreeMap<K, V> extends AbstractMap<K, V> implements SortedMap<K,
V> {
+public class TreeMap<K, V> extends AbstractMap<K, V> implements
+ SortedMap<K, V>, Serializable {
/*
* Implementation derived from public domain C implementation as of 5
* September 2007 at:
@@ -494,14 +497,14 @@
private enum SubMapType {
All,
-
+
Head {
@Override
public boolean toKeyValid() {
return true;
}
},
-
+
Range {
@Override
public boolean fromKeyValid() {
@@ -513,21 +516,21 @@
return true;
}
},
-
+
Tail {
@Override
public boolean fromKeyValid() {
return true;
}
};
-
+
/**
* @return true if this submap type uses a from-key.
*/
public boolean fromKeyValid() {
return false;
}
-
+
/**
* @return true if this submap type uses a to-key.
*/
@@ -580,8 +583,17 @@
// The comparator to use.
private Comparator<? super K> cmp;
+ /*
+ * These two fields are just hints to STOB so that it generates serializers
+ * for K and V
+ */
+ @SuppressWarnings("unused")
+ private K exposeKeyType;
+ @SuppressWarnings("unused")
+ private V exposeValueType;
+
// The root of the tree.
- private Node<K, V> root;
+ private transient Node<K, V> root;
// The number of nodes in the tree.
private int size = 0;
@@ -843,14 +855,13 @@
/**
* Insert a node into a subtree, collecting state about the insertion.
*
- * If the same key already exists, the value of the node is overwritten
- * with the value from the new node instead.
+ * If the same key already exists, the value of the node is overwritten with
+ * the value from the new node instead.
*
* @param tree subtree to insert into
* @param newNode new node to insert
- * @param state result of the insertion:
- * state.found true if the key already existed in the tree
- * state.value the old value if the key existed
+ * @param state result of the insertion: state.found true if the key already
+ * existed in the tree state.value the old value if the key existed
* @return the new subtree root
*/
private Node<K, V> insert(Node<K, V> tree, Node<K, V> newNode, State<V>
state) {
