8024009: Remove jdk.map.useRandomSeed system property
authorbchristi
Thu, 12 Sep 2013 14:22:53 -0700
changeset 19853 832b09e2714c
parent 19852 f8e5a6c5d379
child 19854 dda528ceb1de
8024009: Remove jdk.map.useRandomSeed system property Summary: Removed usage of hashSeed in Hashtable & WeakHashMap, and removed tests Reviewed-by: alanb, mduigou
jdk/src/share/classes/java/util/Hashtable.java
jdk/src/share/classes/java/util/WeakHashMap.java
jdk/test/java/util/Map/CheckRandomHashSeed.java
jdk/test/java/util/Map/Collisions.java
--- a/jdk/src/share/classes/java/util/Hashtable.java	Fri Sep 13 10:48:12 2013 +0200
+++ b/jdk/src/share/classes/java/util/Hashtable.java	Thu Sep 12 14:22:53 2013 -0700
@@ -168,68 +168,6 @@
     /** use serialVersionUID from JDK 1.0.2 for interoperability */
     private static final long serialVersionUID = 1421746759512286392L;
 
-    private static class Holder {
-            // Unsafe mechanics
-        /**
-         *
-         */
-        static final sun.misc.Unsafe UNSAFE;
-
-        /**
-         * Offset of "final" hashSeed field we must set in
-         * readObject() method.
-         */
-        static final long HASHSEED_OFFSET;
-
-        static final boolean USE_HASHSEED;
-
-        static {
-            String hashSeedProp = java.security.AccessController.doPrivileged(
-                    new sun.security.action.GetPropertyAction(
-                        "jdk.map.useRandomSeed"));
-            boolean localBool = (null != hashSeedProp)
-                    ? Boolean.parseBoolean(hashSeedProp) : false;
-            USE_HASHSEED = localBool;
-
-            if (USE_HASHSEED) {
-                try {
-                    UNSAFE = sun.misc.Unsafe.getUnsafe();
-                    HASHSEED_OFFSET = UNSAFE.objectFieldOffset(
-                        Hashtable.class.getDeclaredField("hashSeed"));
-                } catch (NoSuchFieldException | SecurityException e) {
-                    throw new InternalError("Failed to record hashSeed offset", e);
-                }
-            } else {
-                UNSAFE = null;
-                HASHSEED_OFFSET = 0;
-            }
-        }
-    }
-
-    /**
-     * A randomizing value associated with this instance that is applied to
-     * hash code of keys to make hash collisions harder to find.
-     *
-     * Non-final so it can be set lazily, but be sure not to set more than once.
-     */
-    transient final int hashSeed;
-
-    /**
-     * Return an initial value for the hashSeed, or 0 if the random seed is not
-     * enabled.
-     */
-    final int initHashSeed() {
-        if (sun.misc.VM.isBooted() && Holder.USE_HASHSEED) {
-            int seed = ThreadLocalRandom.current().nextInt();
-            return (seed != 0) ? seed : 1;
-        }
-        return 0;
-    }
-
-    private int hash(Object k) {
-        return hashSeed ^ k.hashCode();
-    }
-
     /**
      * Constructs a new, empty hashtable with the specified initial
      * capacity and the specified load factor.
@@ -251,7 +189,6 @@
         this.loadFactor = loadFactor;
         table = new Entry<?,?>[initialCapacity];
         threshold = (int)Math.min(initialCapacity * loadFactor, MAX_ARRAY_SIZE + 1);
-        hashSeed = initHashSeed();
     }
 
     /**
@@ -395,7 +332,7 @@
      */
     public synchronized boolean containsKey(Object key) {
         Entry<?,?> tab[] = table;
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         for (Entry<?,?> e = tab[index] ; e != null ; e = e.next) {
             if ((e.hash == hash) && e.key.equals(key)) {
@@ -423,7 +360,7 @@
     @SuppressWarnings("unchecked")
     public synchronized V get(Object key) {
         Entry<?,?> tab[] = table;
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         for (Entry<?,?> e = tab[index] ; e != null ; e = e.next) {
             if ((e.hash == hash) && e.key.equals(key)) {
@@ -488,7 +425,7 @@
             rehash();
 
             tab = table;
-            hash = hash(key);
+            hash = key.hashCode();
             index = (hash & 0x7FFFFFFF) % tab.length;
         }
 
@@ -524,7 +461,7 @@
 
         // Makes sure the key is not already in the hashtable.
         Entry<?,?> tab[] = table;
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         @SuppressWarnings("unchecked")
         Entry<K,V> entry = (Entry<K,V>)tab[index];
@@ -551,7 +488,7 @@
      */
     public synchronized V remove(Object key) {
         Entry<?,?> tab[] = table;
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         @SuppressWarnings("unchecked")
         Entry<K,V> e = (Entry<K,V>)tab[index];
@@ -760,7 +697,7 @@
             Map.Entry<?,?> entry = (Map.Entry<?,?>)o;
             Object key = entry.getKey();
             Entry<?,?>[] tab = table;
-            int hash = hash(key);
+            int hash = key.hashCode();
             int index = (hash & 0x7FFFFFFF) % tab.length;
 
             for (Entry<?,?> e = tab[index]; e != null; e = e.next)
@@ -775,7 +712,7 @@
             Map.Entry<?,?> entry = (Map.Entry<?,?>) o;
             Object key = entry.getKey();
             Entry<?,?>[] tab = table;
-            int hash = hash(key);
+            int hash = key.hashCode();
             int index = (hash & 0x7FFFFFFF) % tab.length;
 
             @SuppressWarnings("unchecked")
@@ -975,7 +912,7 @@
 
         // Makes sure the key is not already in the hashtable.
         Entry<?,?> tab[] = table;
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         @SuppressWarnings("unchecked")
         Entry<K,V> entry = (Entry<K,V>)tab[index];
@@ -998,7 +935,7 @@
         Objects.requireNonNull(value);
 
         Entry<?,?> tab[] = table;
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         @SuppressWarnings("unchecked")
         Entry<K,V> e = (Entry<K,V>)tab[index];
@@ -1021,7 +958,7 @@
     @Override
     public synchronized boolean replace(K key, V oldValue, V newValue) {
         Entry<?,?> tab[] = table;
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         @SuppressWarnings("unchecked")
         Entry<K,V> e = (Entry<K,V>)tab[index];
@@ -1041,7 +978,7 @@
     @Override
     public synchronized V replace(K key, V value) {
         Entry<?,?> tab[] = table;
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         @SuppressWarnings("unchecked")
         Entry<K,V> e = (Entry<K,V>)tab[index];
@@ -1060,7 +997,7 @@
         Objects.requireNonNull(mappingFunction);
 
         Entry<?,?> tab[] = table;
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         @SuppressWarnings("unchecked")
         Entry<K,V> e = (Entry<K,V>)tab[index];
@@ -1084,7 +1021,7 @@
         Objects.requireNonNull(remappingFunction);
 
         Entry<?,?> tab[] = table;
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         @SuppressWarnings("unchecked")
         Entry<K,V> e = (Entry<K,V>)tab[index];
@@ -1113,7 +1050,7 @@
         Objects.requireNonNull(remappingFunction);
 
         Entry<?,?> tab[] = table;
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         @SuppressWarnings("unchecked")
         Entry<K,V> e = (Entry<K,V>)tab[index];
@@ -1148,7 +1085,7 @@
         Objects.requireNonNull(remappingFunction);
 
         Entry<?,?> tab[] = table;
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         @SuppressWarnings("unchecked")
         Entry<K,V> e = (Entry<K,V>)tab[index];
@@ -1228,13 +1165,6 @@
         // Read in the length, threshold, and loadfactor
         s.defaultReadObject();
 
-        // set hashMask
-        if (Holder.USE_HASHSEED) {
-            int seed = ThreadLocalRandom.current().nextInt();
-            Holder.UNSAFE.putIntVolatile(this, Holder.HASHSEED_OFFSET,
-                                         (seed != 0) ? seed : 1);
-        }
-
         // Read the original length of the array and number of elements
         int origlength = s.readInt();
         int elements = s.readInt();
@@ -1282,7 +1212,7 @@
         }
         // Makes sure the key is not already in the hashtable.
         // This should not happen in deserialized version.
-        int hash = hash(key);
+        int hash = key.hashCode();
         int index = (hash & 0x7FFFFFFF) % tab.length;
         for (Entry<?,?> e = tab[index] ; e != null ; e = e.next) {
             if ((e.hash == hash) && e.key.equals(key)) {
@@ -1347,7 +1277,7 @@
         }
 
         public int hashCode() {
-            return (Objects.hashCode(key) ^ Objects.hashCode(value));
+            return hash ^ Objects.hashCode(value);
         }
 
         public String toString() {
--- a/jdk/src/share/classes/java/util/WeakHashMap.java	Fri Sep 13 10:48:12 2013 +0200
+++ b/jdk/src/share/classes/java/util/WeakHashMap.java	Thu Sep 12 14:22:53 2013 -0700
@@ -190,39 +190,6 @@
      */
     int modCount;
 
-    private static class Holder {
-        static final boolean USE_HASHSEED;
-
-        static {
-            String hashSeedProp = java.security.AccessController.doPrivileged(
-                    new sun.security.action.GetPropertyAction(
-                        "jdk.map.useRandomSeed"));
-            boolean localBool = (null != hashSeedProp)
-                    ? Boolean.parseBoolean(hashSeedProp) : false;
-            USE_HASHSEED = localBool;
-        }
-    }
-
-    /**
-     * A randomizing value associated with this instance that is applied to
-     * hash code of keys to make hash collisions harder to find.
-     *
-     * Non-final so it can be set lazily, but be sure not to set more than once.
-     */
-    transient int hashSeed;
-
-    /**
-     * Initialize the hashing mask value.
-     */
-    final void initHashSeed() {
-        if (sun.misc.VM.isBooted() && Holder.USE_HASHSEED) {
-            // Do not set hashSeed more than once!
-            // assert hashSeed == 0;
-            int seed = ThreadLocalRandom.current().nextInt();
-            hashSeed = (seed != 0) ? seed : 1;
-        }
-    }
-
     @SuppressWarnings("unchecked")
     private Entry<K,V>[] newTable(int n) {
         return (Entry<K,V>[]) new Entry<?,?>[n];
@@ -253,7 +220,6 @@
         table = newTable(capacity);
         this.loadFactor = loadFactor;
         threshold = (int)(capacity * loadFactor);
-        initHashSeed();
     }
 
     /**
@@ -329,7 +295,7 @@
      * in lower bits.
      */
     final int hash(Object k) {
-        int h = hashSeed ^ k.hashCode();
+        int h = k.hashCode();
 
         // This function ensures that hashCodes that differ only by
         // constant multiples at each bit position have a bounded
@@ -783,8 +749,7 @@
         public int hashCode() {
             K k = getKey();
             V v = getValue();
-            return ((k==null ? 0 : k.hashCode()) ^
-                    (v==null ? 0 : v.hashCode()));
+            return Objects.hashCode(k) ^ Objects.hashCode(v);
         }
 
         public String toString() {
--- a/jdk/test/java/util/Map/CheckRandomHashSeed.java	Fri Sep 13 10:48:12 2013 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,91 +0,0 @@
-/*
- * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
- * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
- *
- * This code is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
- *
- * This code is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
- * or visit www.oracle.com if you need additional information or have any
- * questions.
- */
-
-/**
- * @test
- * @bug 8005698
- * @summary Check operation of jdk.map.useRandomSeed property
- * @run main CheckRandomHashSeed
- * @run main/othervm -Djdk.map.useRandomSeed=false CheckRandomHashSeed
- * @run main/othervm -Djdk.map.useRandomSeed=bogus CheckRandomHashSeed
- * @run main/othervm -Djdk.map.useRandomSeed=true CheckRandomHashSeed true
- * @author Brent Christian
- */
-import java.lang.reflect.Field;
-import java.util.Map;
-import java.util.HashMap;
-import java.util.LinkedHashMap;
-import java.util.Hashtable;
-import java.util.WeakHashMap;
-
-public class CheckRandomHashSeed {
-    private final static String PROP_NAME = "jdk.map.useRandomSeed";
-    static boolean expectRandom = false;
-
-    public static void main(String[] args) {
-        if (args.length > 0 && args[0].equals("true")) {
-            expectRandom = true;
-        }
-        String hashSeedProp = System.getProperty(PROP_NAME);
-        boolean propSet = (null != hashSeedProp)
-                ? Boolean.parseBoolean(hashSeedProp) : false;
-        if (expectRandom != propSet) {
-            throw new Error("Error in test setup: " + (expectRandom ? "" : "not " ) + "expecting random hashSeed, but " + PROP_NAME + " is " + (propSet ? "" : "not ") + "enabled");
-        }
-
-        testMap(new WeakHashMap());
-        testMap(new Hashtable());
-    }
-
-    private static void testMap(Map map) {
-        int hashSeed = getHashSeed(map);
-        boolean hashSeedIsZero = (hashSeed == 0);
-
-        if (expectRandom != hashSeedIsZero) {
-            System.out.println("Test passed for " + map.getClass().getSimpleName() + " - expectRandom: " + expectRandom + ", hashSeed: " + hashSeed);
-        } else {
-            throw new Error ("Test FAILED for " + map.getClass().getSimpleName() + " -  expectRandom: " + expectRandom + ", hashSeed: " + hashSeed);
-        }
-    }
-
-    private static int getHashSeed(Map map) {
-        try {
-            if (map instanceof HashMap || map instanceof LinkedHashMap) {
-                map.put("Key", "Value");
-                Field hashSeedField = HashMap.class.getDeclaredField("hashSeed");
-                hashSeedField.setAccessible(true);
-                int hashSeed = hashSeedField.getInt(map);
-                return hashSeed;
-            } else {
-                map.put("Key", "Value");
-                Field hashSeedField = map.getClass().getDeclaredField("hashSeed");
-                hashSeedField.setAccessible(true);
-                int hashSeed = hashSeedField.getInt(map);
-                return hashSeed;
-            }
-        } catch(Exception e) {
-            e.printStackTrace();
-            throw new Error(e);
-        }
-    }
-}
--- a/jdk/test/java/util/Map/Collisions.java	Fri Sep 13 10:48:12 2013 +0200
+++ b/jdk/test/java/util/Map/Collisions.java	Thu Sep 12 14:22:53 2013 -0700
@@ -25,8 +25,6 @@
  * @test
  * @bug 7126277
  * @run main Collisions -shortrun
- * @run main/othervm -Djdk.map.althashing.threshold=0 Collisions -shortrun
- * @run main/othervm -Djdk.map.useRandomSeed=true Collisions -shortrun
  * @summary Ensure Maps behave well with lots of hashCode() collisions.
  * @author Mike Duigou
  */