From 4482b52cfdf8420109882518a50069a9d4421e12 Mon Sep 17 00:00:00 2001 From: Julian Hyde Date: Sat, 3 Oct 2009 23:35:54 +0000 Subject: [PATCH] It's much more efficient to call ArrayMap(Map) than ArrayMap() followed by put or putAll. Fix Pair so that its hashCode is consistent with Map.Entry. git-svn-id: https://olap4j.svn.sourceforge.net/svnroot/olap4j/trunk@285 c6a108a4-781c-0410-a6c6-c2d559e19af0 --- .../driver/xmla/XmlaOlap4jConnection.java | 52 +++++++---- .../xmla/XmlaOlap4jDatabaseMetaData.java | 2 +- .../olap4j/driver/xmla/XmlaOlap4jMeasure.java | 4 +- .../olap4j/driver/xmla/XmlaOlap4jMember.java | 8 +- .../driver/xmla/XmlaOlap4jPositionMember.java | 3 +- src/org/olap4j/impl/ArrayMap.java | 63 ++++++++++++- src/org/olap4j/impl/Pair.java | 8 +- testsrc/org/olap4j/test/ArrayMapTest.java | 90 +++++++++++++++++++ 8 files changed, 200 insertions(+), 30 deletions(-) diff --git a/src/org/olap4j/driver/xmla/XmlaOlap4jConnection.java b/src/org/olap4j/driver/xmla/XmlaOlap4jConnection.java index bd231d8..df22b28 100644 --- a/src/org/olap4j/driver/xmla/XmlaOlap4jConnection.java +++ b/src/org/olap4j/driver/xmla/XmlaOlap4jConnection.java @@ -1224,6 +1224,14 @@ static class MemberHandler extends HandlerImpl { Property.StandardMemberProperty.CHILDREN_CARDINALITY.name(), Property.StandardMemberProperty.DEPTH.name())); + /** + * Cached value returned by the {@link Member.Type#values} method, which + * calls {@link Class#getEnumConstants()} and unfortunately clones an + * array every time. + */ + private static final Member.Type[] MEMBER_TYPE_VALUES = + Member.Type.values(); + public void handle( Element row, Context context, @@ -1276,7 +1284,7 @@ public void handle( row, Property.StandardMemberProperty.PARENT_UNIQUE_NAME.name()); Member.Type memberType = - Member.Type.values()[ + MEMBER_TYPE_VALUES[ integerElement( row, Property.StandardMemberProperty.MEMBER_TYPE.name())]; @@ -1289,17 +1297,15 @@ public void handle( row, Property.StandardMemberProperty.CHILDREN_CARDINALITY .name()); - // If this member is a measure, we want to return an object that - // implements the Measure interface to all API calls. But we also - // need to retrieve the properties that occur in MDSCHEMA_MEMBERS - // that are not available in MDSCHEMA_MEASURES, so we create a - // member for internal use. - XmlaOlap4jMember member = - new XmlaOlap4jMember( - context.getLevel(row), memberUniqueName, memberName, - memberCaption, "", parentUniqueName, memberType, - childrenCardinality, memberOrdinal); - addUserDefinedDimensionProperties(row, member); + + // Gather member property values into a temporary map, so we can + // create the member with all properties known. XmlaOlap4jMember + // uses an ArrayMap for property values and it is not efficient to + // add entries to the map one at a time. + final XmlaOlap4jLevel level = context.getLevel(row); + final Map map = + new HashMap(); + addUserDefinedDimensionProperties(row, level, map); // Usually members have the same depth as their level. (Ragged and // parent-child hierarchies are an exception.) Only store depth for @@ -1309,18 +1315,30 @@ public void handle( row, Property.StandardMemberProperty.DEPTH.name()); if (depth != null - && depth.intValue() != member.getLevel().getDepth()) + && depth.intValue() != level.getDepth()) { - member.setProperty( + map.put( Property.StandardMemberProperty.DEPTH, depth); } + + // If this member is a measure, we want to return an object that + // implements the Measure interface to all API calls. But we also + // need to retrieve the properties that occur in MDSCHEMA_MEMBERS + // that are not available in MDSCHEMA_MEASURES, so we create a + // member for internal use. + XmlaOlap4jMember member = + new XmlaOlap4jMember( + level, memberUniqueName, memberName, + memberCaption, "", parentUniqueName, memberType, + childrenCardinality, memberOrdinal, map); list.add(member); } private void addUserDefinedDimensionProperties( Element row, - XmlaOlap4jMember member) + XmlaOlap4jLevel level, + Map map) { NodeList nodes = row.getChildNodes(); for (int i = 0; i < nodes.getLength(); i++) { @@ -1328,12 +1346,12 @@ private void addUserDefinedDimensionProperties( if (EXCLUDED_PROPERTY_NAMES.contains(node.getLocalName())) { continue; } - for (Property property : member.getLevel().getProperties()) { + for (Property property : level.getProperties()) { if (property instanceof XmlaOlap4jProperty && property.getName().equalsIgnoreCase( node.getLocalName())) { - member.setProperty(property, node.getTextContent()); + map.put(property, node.getTextContent()); } } } diff --git a/src/org/olap4j/driver/xmla/XmlaOlap4jDatabaseMetaData.java b/src/org/olap4j/driver/xmla/XmlaOlap4jDatabaseMetaData.java index a484562..4180486 100644 --- a/src/org/olap4j/driver/xmla/XmlaOlap4jDatabaseMetaData.java +++ b/src/org/olap4j/driver/xmla/XmlaOlap4jDatabaseMetaData.java @@ -92,7 +92,7 @@ private ResultSet getMetadata( new XmlaOlap4jConnection.Context( olap4jConnection, null, null, null, null, null, null, null); List patternValueList = new ArrayList(); - Map predicateList = new ArrayMap(); + Map predicateList = new HashMap(); for (int i = 0; i < patternValues.length; i += 2) { String name = (String) patternValues[i]; assert metadataRequest.getColumn(name) != null diff --git a/src/org/olap4j/driver/xmla/XmlaOlap4jMeasure.java b/src/org/olap4j/driver/xmla/XmlaOlap4jMeasure.java index a382c94..64338bf 100644 --- a/src/org/olap4j/driver/xmla/XmlaOlap4jMeasure.java +++ b/src/org/olap4j/driver/xmla/XmlaOlap4jMeasure.java @@ -11,6 +11,8 @@ import org.olap4j.impl.Named; import org.olap4j.metadata.*; +import java.util.Collections; + /** * Implementation of {@link org.olap4j.metadata.Measure} * for XML/A providers. @@ -43,7 +45,7 @@ class XmlaOlap4jMeasure olap4jLevel, uniqueName, name, caption, description, parentMemberUniqueName, aggregator == Aggregator.CALCULATED ? Type.FORMULA : Type.MEASURE, - 0, ordinal); + 0, ordinal, Collections.emptyMap()); assert olap4jLevel.olap4jHierarchy.olap4jDimension.type == Dimension.Type.MEASURE; this.aggregator = aggregator; diff --git a/src/org/olap4j/driver/xmla/XmlaOlap4jMember.java b/src/org/olap4j/driver/xmla/XmlaOlap4jMember.java index f38b984..dc18356 100644 --- a/src/org/olap4j/driver/xmla/XmlaOlap4jMember.java +++ b/src/org/olap4j/driver/xmla/XmlaOlap4jMember.java @@ -45,8 +45,7 @@ class XmlaOlap4jMember private XmlaOlap4jMember parentMember; private final int childMemberCount; private final int ordinal; - private final ArrayMap propertyValueMap = - new ArrayMap(); + private final ArrayMap propertyValueMap; XmlaOlap4jMember( XmlaOlap4jLevel olap4jLevel, @@ -57,7 +56,8 @@ class XmlaOlap4jMember String parentMemberUniqueName, Type type, int childMemberCount, - int ordinal) + int ordinal, + Map propertyValueMap) { super(uniqueName, name, caption, description); this.ordinal = ordinal; @@ -67,6 +67,8 @@ class XmlaOlap4jMember this.parentMemberUniqueName = parentMemberUniqueName; this.type = type; this.childMemberCount = childMemberCount; + this.propertyValueMap = + new ArrayMap(propertyValueMap); } public int hashCode() { diff --git a/src/org/olap4j/driver/xmla/XmlaOlap4jPositionMember.java b/src/org/olap4j/driver/xmla/XmlaOlap4jPositionMember.java index 113ebec..2e7875a 100644 --- a/src/org/olap4j/driver/xmla/XmlaOlap4jPositionMember.java +++ b/src/org/olap4j/driver/xmla/XmlaOlap4jPositionMember.java @@ -50,8 +50,7 @@ class XmlaOlap4jPositionMember assert member != null; assert propertyValues != null; this.member = member; - this.propertyValues = new ArrayMap(); - this.propertyValues.putAll(propertyValues); + this.propertyValues = new ArrayMap(propertyValues); } public boolean equals(Object obj) { diff --git a/src/org/olap4j/impl/ArrayMap.java b/src/org/olap4j/impl/ArrayMap.java index af53701..55b4986 100644 --- a/src/org/olap4j/impl/ArrayMap.java +++ b/src/org/olap4j/impl/ArrayMap.java @@ -53,6 +53,51 @@ public ArrayMap(Map map) { } } + public boolean equals(Object o) { + if (o == this) { + return true; + } + if (!(o instanceof Map)) { + return false; + } + Map m = (Map) o; + if (m.size() != size()) { + return false; + } + try { + Iterator> i = entrySet().iterator(); + while (i.hasNext()) { + Entry e = i.next(); + K key = e.getKey(); + V value = e.getValue(); + if (value == null) { + if (!(m.get(key) == null && m.containsKey(key))) { + return false; + } + } else { + if (!value.equals(m.get(key))) { + return false; + } + } + } + } catch (ClassCastException unused) { + return false; + } catch (NullPointerException unused) { + return false; + } + + return true; + } + + public int hashCode() { + int h = 0; + Iterator> i = entrySet().iterator(); + while (i.hasNext()) { + h += i.next().hashCode(); + } + return h; + } + public int size() { return keyValues.length / 2; } @@ -130,8 +175,22 @@ private void removeInternal(int i) { } public void putAll(Map m) { - for (Entry entry : m.entrySet()) { - put(entry.getKey(), entry.getValue()); + if (keyValues.length == 0) { + // Efficient implementation of common case where map is initially + // empty; otherwise we have O(n^2) reallocs. + keyValues = new Object[m.size() * 2]; + int i = 0; + for (Entry entry : m.entrySet()) { + keyValues[i++] = entry.getKey(); + keyValues[i++] = entry.getValue(); + } + } else { + // This algorithm is O(n^2): not great if m is large. But it's + // difficult to preallocate the array if we don't know how many + // keys overlap between this and m. + for (Entry entry : m.entrySet()) { + put(entry.getKey(), entry.getValue()); + } } } diff --git a/src/org/olap4j/impl/Pair.java b/src/org/olap4j/impl/Pair.java index 5854c6b..9e60401 100644 --- a/src/org/olap4j/impl/Pair.java +++ b/src/org/olap4j/impl/Pair.java @@ -48,12 +48,12 @@ public boolean equals(Object obj) { } public int hashCode() { - int k = (left == null) ? 0 : left.hashCode(); - int k1 = (right == null) ? 0 : right.hashCode(); - return ((k << 4) | k) ^ k1; + // formula required by Map contract + final int k = left == null ? 0 : left.hashCode(); + final int k1 = right == null ? 0 : right.hashCode(); + return k ^ k1; } - public int compareTo(Pair that) { int c = compare((Comparable) this.left, (Comparable)that.left); if (c == 0) { diff --git a/testsrc/org/olap4j/test/ArrayMapTest.java b/testsrc/org/olap4j/test/ArrayMapTest.java index a611ab8..ee9ad6f 100644 --- a/testsrc/org/olap4j/test/ArrayMapTest.java +++ b/testsrc/org/olap4j/test/ArrayMapTest.java @@ -97,6 +97,96 @@ public void testArrayMap() { map.remove("George"); assertEquals(2, map.size()); } + + /** + * Oops, forgot that I had written the first test and wrote another. Mostly + * overlap with {@link #testArrayMap()}, but can't hurt. + */ + public void testArrayMap2() { + final ArrayMap map = new ArrayMap(); + final HashMap hashMap = new HashMap(); + assertEquals(0, map.size()); + assertEquals(map, hashMap); + assertEquals(map.hashCode(), hashMap.hashCode()); + + // put + map.put("foo", 0); + assertEquals(1, map.size()); + assertEquals(1, map.keySet().size()); + assertEquals(1, map.values().size()); + + // equivalence to hashmap + hashMap.put("foo", 0); + assertEquals(map, hashMap); + assertEquals(hashMap, map); + assertEquals(map.hashCode(), hashMap.hashCode()); + + // containsKey, get + assertTrue(map.containsKey("foo")); + assertFalse(map.containsKey("bar")); + assertEquals(Integer.valueOf(0), map.get("foo")); + assertNull(map.get("baz")); + + // putall + final Map hashMap2 = new HashMap(); + hashMap2.put("bar", 1); + hashMap2.put("foo", 2); + hashMap2.put("baz", 0); + map.putAll(hashMap2); + hashMap.putAll(hashMap2); + assertEquals(3, map.size()); + assertEquals(map, hashMap); + assertEquals(hashMap, map); + assertEquals(map.hashCode(), hashMap.hashCode()); + assertEquals(map.keySet(), hashMap.keySet()); + // values collections have same contents, not necessarily in same order + assertEquals( + new HashSet(map.values()), + new HashSet(hashMap.values())); + + // replace existing key + map.put("foo", -5); + hashMap.put("foo", -5); + assertEquals(3, map.size()); + assertEquals(Integer.valueOf(-5), map.get("foo")); + assertEquals(map, hashMap); + assertEquals(hashMap, map); + + // null key + assertFalse(map.containsKey(null)); + map.put(null, 75); + assertEquals(Integer.valueOf(75), map.get(null)); + assertTrue(map.containsKey(null)); + + // null value + map.put("zzzz", null); + assertTrue(map.containsKey("zzzz")); + assertNull(map.get("zzzz")); + + // compare to hashmap + hashMap.put(null, 75); + hashMap.put("zzzz", null); + assertEquals(map, hashMap); + assertEquals(hashMap, map); + + // isEmpty, clear + assertFalse(map.isEmpty()); + map.clear(); + assertTrue(map.isEmpty()); + assertEquals(0, map.size()); + + // putAll to populate empty map (uses different code path than putAll + // on non-empty map) + final ArrayMap map2 = + new ArrayMap(); + map2.putAll(hashMap); + assertEquals(map2, hashMap); + + // copy constructor + final ArrayMap map3 = + new ArrayMap(hashMap); + assertEquals(map3, hashMap); + } } // End ArrayMapTest.java