sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject svn commit: r1507979 - in /sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis: internal/jaxb/PrimitiveTypeProperties.java util/collection/WeakValueHashMap.java
Date Mon, 29 Jul 2013 09:10:01 GMT
Author: desruisseaux
Date: Mon Jul 29 09:10:01 2013
New Revision: 1507979

URL: http://svn.apache.org/r1507979
Log:
Added comments explaining rational for the implementation choice, and drawbacks.

Modified:
    sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/jaxb/PrimitiveTypeProperties.java
    sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/util/collection/WeakValueHashMap.java

Modified: sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/jaxb/PrimitiveTypeProperties.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/jaxb/PrimitiveTypeProperties.java?rev=1507979&r1=1507978&r2=1507979&view=diff
==============================================================================
--- sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/jaxb/PrimitiveTypeProperties.java
[UTF-8] (original)
+++ sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/internal/jaxb/PrimitiveTypeProperties.java
[UTF-8] Mon Jul 29 09:10:01 2013
@@ -16,16 +16,17 @@
  */
 package org.apache.sis.internal.jaxb;
 
+import java.util.Map;
+import java.util.IdentityHashMap;
 import java.lang.reflect.Modifier;
 import org.apache.sis.xml.NilReason;
-import org.apache.sis.util.collection.WeakValueHashMap;
 
 
 /**
  * A workaround for attaching properties ({@code nilreason}, {@code href}, <i>etc.</i>)
to primitive type wrappers.
  * The normal approach in SIS is to implement the {@link org.apache.sis.xml.NilObject} interface.
However we can not
  * do so when the object is a final Java class like {@link Boolean}, {@link Integer}, {@link
Double} or {@link String}.
- * This class provides a workaround using specific instances of some primitive types.
+ * This class provides a workaround using specific instances of those wrappers.
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @since   0.4
@@ -36,13 +37,29 @@ import org.apache.sis.util.collection.We
  */
 public final class PrimitiveTypeProperties {
     /**
-     * The map where to store specific instances. We really need an identity hash map;
-     * using the {@code Object.equals(Object)} method is not allowed here.
+     * The map where to store specific instances. Keys are instances of the primitive wrappers
considered as nil.
+     * Values are the {@code NilReason} why the primitive is missing, or any other property
we may want to attach.
      *
-     * <p>Keys are the primitive type instances. Values are the {@code NilReason} why
this value is missing,
-     * or any other property we may want to attach.</p>
+     * {@section Identity comparisons}
+     * We really need an identity hash map; using the {@code Object.equals(Object)} method
is not allowed here.
+     * This is because "nil values" are real values. For example if the type is {@link Integer},
then the nil value
+     * is an {@code Integer} instance having the value 0. We don't want to consider every
0 integer value as nil,
+     * but only the specific {@code Integer} instance used as sentinel value for nil.
+     *
+     * {@section Weak references}
+     * We can not use weak value references, because we don't want the {@link NilReason}
(the map value) to be lost
+     * while the sentinel value (the map key) is still in use. We could use weak references
for the keys, but JDK 7
+     * does not provides any map implementation which is both an {@code IdentityHashMap}
and a {@code WeakHashMap}.
+     *
+     * For now we do not use weak references. This means that if a user creates a custom
{@code NilReason} by a call
+     * to {@link NilReason#valueOf(String)}, and if he uses that nil reason for a primitive
type, then that custom
+     * {@code NilReason} instance and its sentinel values will never be garbage-collected.
+     * We presume that such cases will be rare enough for not being an issue in practice.
+     *
+     * {@section Synchronization}
+     * All accesses to this map shall be synchronized on the map object.
      */
-    private static final WeakValueHashMap<Object,Object> SENTINAL_VALUES = new WeakValueHashMap<>(Object.class,
true);
+    private static final Map<Object,Object> SENTINEL_VALUES = new IdentityHashMap<>();
 
     /**
      * Do not allow instantiation of this class.
@@ -69,10 +86,12 @@ public final class PrimitiveTypeProperti
      */
     public static void associate(final Object primitive, final Object property) {
         assert isValidKey(primitive) : primitive;
-        final Object old = SENTINAL_VALUES.put(primitive, property);
-        if (old != null) { // Should never happen - this is rather debugging check.
-            SENTINAL_VALUES.put(primitive, old);
-            throw new AssertionError(primitive);
+        synchronized (SENTINEL_VALUES) {
+            final Object old = SENTINEL_VALUES.put(primitive, property);
+            if (old != null) { // Should never happen - this is rather debugging check.
+                SENTINEL_VALUES.put(primitive, old);
+                throw new AssertionError(primitive);
+            }
         }
     }
 
@@ -84,6 +103,8 @@ public final class PrimitiveTypeProperti
      */
     public static Object property(final Object primitive) {
         assert isValidKey(primitive) : primitive;
-        return SENTINAL_VALUES.get(primitive);
+        synchronized (SENTINEL_VALUES) {
+            return SENTINEL_VALUES.get(primitive);
+        }
     }
 }

Modified: sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/util/collection/WeakValueHashMap.java
URL: http://svn.apache.org/viewvc/sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/util/collection/WeakValueHashMap.java?rev=1507979&r1=1507978&r2=1507979&view=diff
==============================================================================
--- sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/util/collection/WeakValueHashMap.java
[UTF-8] (original)
+++ sis/branches/JDK7/core/sis-utility/src/main/java/org/apache/sis/util/collection/WeakValueHashMap.java
[UTF-8] Mon Jul 29 09:10:01 2013
@@ -86,12 +86,21 @@ import java.util.Objects;
 @ThreadSafe
 public class WeakValueHashMap<K,V> extends AbstractMap<K,V> {
     /**
-     * The comparison mode for key objects.
+     * Comparison mode for key objects. The standard mode is {@code EQUALS}, which means
that keys are compared
+     * using their {@link Object#equals(Object)} method. But {@code WeakValueHashMap} will
automatically select
+     * {@code DEEP_EQUALS} if there is a chance that some keys are arrays. In the later case,
comparisons will
+     * be done by the more costly {@link Objects#deepEquals(Object, Object)} method instead.
+     *
+     * <p>The {@code IDENTITY} mode is rarely used, and is selected only if the user
explicitely asks for this mode
+     * at construction time. This mode is provided because reference-equality semantic is
sometime required, and
+     * hard to simulate if not supported natively by the hash map. See {@link java.util.IdentityHashMap}
javadoc
+     * for some examples of cases where reference-equality semantic is useful.</p>
      *
+     * @see #comparisonMode
      * @see #keyEquals(Object, Object)
      * @see #keyHashCode(Object)
      */
-    private static final int IDENTITY = 0, EQUALS = 1, DEEP_EQUALS = 2;
+    private static final byte IDENTITY = 0, EQUALS = 1, DEEP_EQUALS = 2;
 
     /**
      * An entry in the {@link WeakValueHashMap}. This is a weak reference
@@ -195,10 +204,15 @@ public class WeakValueHashMap<K,V> exten
     private final Class<K> keyType;
 
     /**
-     * {@link #DEEP_EQUALS} if the keys in this map may be arrays. If the keys can not be
arrays,
-     * then we can avoid the calls to the costly {@link Utilities} methods.
+     * Whether keys shall be compared by reference-equality ({@link #IDENTITY}), by shallow
object-equality
+     * ({@link #EQUALS}) or by deep object-equality ({@link #DEEP_EQUALS}). The {@code DEEP_EQUALS}
mode is
+     * selected only if the keys in this map may be arrays. If the keys can not be arrays,
then we select the
+     * {@code EQUALS} mode for avoiding calls to the costly {@link Objects#deepEquals(Object,
Object)} method.
+     *
+     * @see #keyEquals(Object, Object)
+     * @see #keyHashCode(Object)
      */
-    private final int comparisonMode;
+    private final byte comparisonMode;
 
     /**
      * The set of entries, created only when first needed.
@@ -227,15 +241,18 @@ public class WeakValueHashMap<K,V> exten
      * If {@code identity} is {@code true}, then two keys {@code k1} and {@code k2} are considered
equal if and
      * only if {@code (k1 == k2)} instead than if {@code k1.equals(k2)}.
      *
-     * @param keyType    The type of keys in the map.
-     * @param byIdentity {@code true} if the map shall use reference-equality in place of
object-equality
-     *                   when comparing keys, or {@code false} for the standard behavior.
+     * <p>Reference-equality semantic is rarely used. See the {@link java.util.IdentityHashMap}
class javadoc
+     * for a discussion about drawbacks and use cases when reference-equality semantic is
useful.</p>
+     *
+     * @param keyType  The type of keys in the map.
+     * @param identity {@code true} if the map shall use reference-equality in place of object-equality
+     *                 when comparing keys, or {@code false} for the standard behavior.
      *
      * @since 0.4
      */
-    public WeakValueHashMap(final Class<K> keyType, final boolean byIdentity) {
+    public WeakValueHashMap(final Class<K> keyType, final boolean identity) {
         this.keyType   = keyType;
-        comparisonMode = byIdentity ? IDENTITY :
+        comparisonMode = identity ? IDENTITY :
                 (keyType.isArray() || keyType.equals(Object.class)) ? DEEP_EQUALS : EQUALS;
         lastTimeNormalCapacity = System.nanoTime();
         /*



Mime
View raw message