From f31e156be33ba5503384a7e1a093327b993d9332 Mon Sep 17 00:00:00 2001 From: Mike Duigou Date: Mon, 5 May 2014 09:52:24 -0700 Subject: [PATCH] 8020860: cluster Hashtable/Vector field updates for better transactional memory behaviour Co-authored-by: Sandhya Viswanathan Reviewed-by: mduigou, martin, psandoz --- .../share/classes/java/util/Hashtable.java | 53 ++++++++------- jdk/src/share/classes/java/util/Vector.java | 68 ++++++++++++------- 2 files changed, 72 insertions(+), 49 deletions(-) diff --git a/jdk/src/share/classes/java/util/Hashtable.java b/jdk/src/share/classes/java/util/Hashtable.java index 4a89bd5816d..c2f42b10263 100644 --- a/jdk/src/share/classes/java/util/Hashtable.java +++ b/jdk/src/share/classes/java/util/Hashtable.java @@ -26,7 +26,6 @@ package java.util; import java.io.*; -import java.util.concurrent.ThreadLocalRandom; import java.util.function.BiConsumer; import java.util.function.Function; import java.util.function.BiFunction; @@ -92,8 +91,10 @@ import java.util.function.BiFunction; * ConcurrentModificationException}. Thus, in the face of concurrent * modification, the iterator fails quickly and cleanly, rather than risking * arbitrary, non-deterministic behavior at an undetermined time in the future. - * The Enumerations returned by Hashtable's keys and elements methods are - * not fail-fast. + * The Enumerations returned by Hashtable's {@link #keys keys} and + * {@link #elements elements} methods are not fail-fast; if the + * Hashtable is structurally modified at any time after the enumeration is + * created then the results of enumerating are undefined. * *

Note that the fail-fast behavior of an iterator cannot be guaranteed * as it is, generally speaking, impossible to make any hard guarantees in the @@ -115,6 +116,9 @@ import java.util.function.BiFunction; * to use {@link java.util.concurrent.ConcurrentHashMap} in place of * {@code Hashtable}. * + * @param the type of keys maintained by this map + * @param the type of mapped values + * * @author Arthur van Hoff * @author Josh Bloch * @author Neal Gafter @@ -246,6 +250,9 @@ public class Hashtable /** * Returns an enumeration of the keys in this hashtable. + * Use the Enumeration methods on the returned object to fetch the keys + * sequentially. If the hashtable is structurally modified while enumerating + * over the keys then the results of enumerating are undefined. * * @return an enumeration of the keys in this hashtable. * @see Enumeration @@ -260,7 +267,8 @@ public class Hashtable /** * Returns an enumeration of the values in this hashtable. * Use the Enumeration methods on the returned object to fetch the elements - * sequentially. + * sequentially. If the hashtable is structurally modified while enumerating + * over the values then the results of enumerating are undefined. * * @return an enumeration of the values in this hashtable. * @see java.util.Enumeration @@ -417,8 +425,6 @@ public class Hashtable } private void addEntry(int hash, K key, V value, int index) { - modCount++; - Entry tab[] = table; if (count >= threshold) { // Rehash the table if the threshold is exceeded @@ -434,6 +440,7 @@ public class Hashtable Entry e = (Entry) tab[index]; tab[index] = new Entry<>(hash, key, value, e); count++; + modCount++; } /** @@ -494,12 +501,12 @@ public class Hashtable Entry e = (Entry)tab[index]; for(Entry prev = null ; e != null ; prev = e, e = e.next) { if ((e.hash == hash) && e.key.equals(key)) { - modCount++; if (prev != null) { prev.next = e.next; } else { tab[index] = e.next; } + modCount++; count--; V oldValue = e.value; e.value = null; @@ -528,9 +535,9 @@ public class Hashtable */ public synchronized void clear() { Entry tab[] = table; - modCount++; for (int index = tab.length; --index >= 0; ) tab[index] = null; + modCount++; count = 0; } @@ -719,14 +726,14 @@ public class Hashtable Entry e = (Entry)tab[index]; for(Entry prev = null; e != null; prev = e, e = e.next) { if (e.hash==hash && e.equals(entry)) { - modCount++; if (prev != null) prev.next = e.next; else tab[index] = e.next; + e.value = null; // clear for gc. + modCount++; count--; - e.value = null; return true; } } @@ -939,14 +946,14 @@ public class Hashtable Entry e = (Entry)tab[index]; for (Entry prev = null; e != null; prev = e, e = e.next) { if ((e.hash == hash) && e.key.equals(key) && e.value.equals(value)) { - modCount++; if (prev != null) { prev.next = e.next; } else { tab[index] = e.next; } + e.value = null; // clear for gc + modCount++; count--; - e.value = null; return true; } } @@ -1030,12 +1037,12 @@ public class Hashtable if (e.hash == hash && e.key.equals(key)) { V newValue = remappingFunction.apply(key, e.value); if (newValue == null) { - modCount++; if (prev != null) { prev.next = e.next; } else { tab[index] = e.next; } + modCount++; count--; } else { e.value = newValue; @@ -1059,12 +1066,12 @@ public class Hashtable if (e.hash == hash && Objects.equals(e.key, key)) { V newValue = remappingFunction.apply(key, e.value); if (newValue == null) { - modCount++; if (prev != null) { prev.next = e.next; } else { tab[index] = e.next; } + modCount++; count--; } else { e.value = newValue; @@ -1094,12 +1101,12 @@ public class Hashtable if (e.hash == hash && e.key.equals(key)) { V newValue = remappingFunction.apply(e.value, value); if (newValue == null) { - modCount++; if (prev != null) { prev.next = e.next; } else { tab[index] = e.next; } + modCount++; count--; } else { e.value = newValue; @@ -1298,24 +1305,24 @@ public class Hashtable * by passing an Enumeration. */ private class Enumerator implements Enumeration, Iterator { - Entry[] table = Hashtable.this.table; + final Entry[] table = Hashtable.this.table; int index = table.length; Entry entry; Entry lastReturned; - int type; + final int type; /** * Indicates whether this Enumerator is serving as an Iterator * or an Enumeration. (true -> Iterator). */ - boolean iterator; + final boolean iterator; /** * The modCount value that the iterator believes that the backing * Hashtable should have. If this expectation is violated, the iterator * has detected concurrent modification. */ - protected int expectedModCount = modCount; + protected int expectedModCount = Hashtable.this.modCount; Enumerator(int type, boolean iterator) { this.type = type; @@ -1360,7 +1367,7 @@ public class Hashtable } public T next() { - if (modCount != expectedModCount) + if (Hashtable.this.modCount != expectedModCount) throw new ConcurrentModificationException(); return nextElement(); } @@ -1381,14 +1388,14 @@ public class Hashtable Entry e = (Entry)tab[index]; for(Entry prev = null; e != null; prev = e, e = e.next) { if (e == lastReturned) { - modCount++; - expectedModCount++; if (prev == null) tab[index] = e.next; else prev.next = e.next; - count--; + expectedModCount++; lastReturned = null; + Hashtable.this.modCount++; + Hashtable.this.count--; return; } } diff --git a/jdk/src/share/classes/java/util/Vector.java b/jdk/src/share/classes/java/util/Vector.java index 980f48ce075..01d505e1c8f 100644 --- a/jdk/src/share/classes/java/util/Vector.java +++ b/jdk/src/share/classes/java/util/Vector.java @@ -56,7 +56,9 @@ import java.util.function.UnaryOperator; * concurrent modification, the iterator fails quickly and cleanly, rather * than risking arbitrary, non-deterministic behavior at an undetermined * time in the future. The {@link Enumeration Enumerations} returned by - * the {@link #elements() elements} method are not fail-fast. + * the {@link #elements() elements} method are not fail-fast; if the + * Vector is structurally modified at any time after the enumeration is + * created then the results of enumerating are undefined. * *

Note that the fail-fast behavior of an iterator cannot be guaranteed * as it is, generally speaking, impossible to make any hard guarantees in the @@ -74,6 +76,8 @@ import java.util.function.UnaryOperator; * implementation is not needed, it is recommended to use {@link * ArrayList} in place of {@code Vector}. * + * @param Type of component elements + * * @author Lee Boynton * @author Jonathan Payne * @see Collection @@ -330,7 +334,9 @@ public class Vector * Returns an enumeration of the components of this vector. The * returned {@code Enumeration} object will generate all items in * this vector. The first item generated is the item at index {@code 0}, - * then the item at index {@code 1}, and so on. + * then the item at index {@code 1}, and so on. If the vector is + * structurally modified while enumerating over the elements then the + * results of enumerating are undefined. * * @return an enumeration of the components of this vector * @see Iterator @@ -553,7 +559,6 @@ public class Vector * ({@code index < 0 || index >= size()}) */ public synchronized void removeElementAt(int index) { - modCount++; if (index >= elementCount) { throw new ArrayIndexOutOfBoundsException(index + " >= " + elementCount); @@ -565,6 +570,7 @@ public class Vector if (j > 0) { System.arraycopy(elementData, index + 1, elementData, index, j); } + modCount++; elementCount--; elementData[elementCount] = null; /* to let gc do its work */ } @@ -593,7 +599,6 @@ public class Vector * ({@code index < 0 || index > size()}) */ public synchronized void insertElementAt(E obj, int index) { - modCount++; if (index > elementCount) { throw new ArrayIndexOutOfBoundsException(index + " > " + elementCount); @@ -601,6 +606,7 @@ public class Vector ensureCapacityHelper(elementCount + 1); System.arraycopy(elementData, index, elementData, index + 1, elementCount - index); elementData[index] = obj; + modCount++; elementCount++; } @@ -616,8 +622,8 @@ public class Vector * @param obj the component to be added */ public synchronized void addElement(E obj) { - modCount++; ensureCapacityHelper(elementCount + 1); + modCount++; elementData[elementCount++] = obj; } @@ -653,11 +659,11 @@ public class Vector * method (which is part of the {@link List} interface). */ public synchronized void removeAllElements() { - modCount++; // Let gc do its work for (int i = 0; i < elementCount; i++) elementData[i] = null; + modCount++; elementCount = 0; } @@ -705,12 +711,15 @@ public class Vector * of the Vector only if the caller knows that the Vector * does not contain any null elements.) * + * @param type of array elements. The same type as {@code } or a + * supertype of {@code }. * @param a the array into which the elements of the Vector are to * be stored, if it is big enough; otherwise, a new array of the * same runtime type is allocated for this purpose. * @return an array containing the elements of the Vector - * @throws ArrayStoreException if the runtime type of a is not a supertype - * of the runtime type of every element in this Vector + * @throws ArrayStoreException if the runtime type of a, {@code }, is not + * a supertype of the runtime type, {@code }, of every element in this + * Vector * @throws NullPointerException if the given array is null * @since 1.2 */ @@ -778,8 +787,8 @@ public class Vector * @since 1.2 */ public synchronized boolean add(E e) { - modCount++; ensureCapacityHelper(elementCount + 1); + modCount++; elementData[elementCount++] = e; return true; } @@ -879,14 +888,18 @@ public class Vector * @throws NullPointerException if the specified collection is null * @since 1.2 */ - public synchronized boolean addAll(Collection c) { - modCount++; + public boolean addAll(Collection c) { Object[] a = c.toArray(); int numNew = a.length; - ensureCapacityHelper(elementCount + numNew); - System.arraycopy(a, 0, elementData, elementCount, numNew); - elementCount += numNew; - return numNew != 0; + if (numNew > 0) { + synchronized (this) { + ensureCapacityHelper(elementCount + numNew); + System.arraycopy(a, 0, elementData, elementCount, numNew); + modCount++; + elementCount += numNew; + } + } + return numNew > 0; } /** @@ -951,22 +964,25 @@ public class Vector * @since 1.2 */ public synchronized boolean addAll(int index, Collection c) { - modCount++; if (index < 0 || index > elementCount) throw new ArrayIndexOutOfBoundsException(index); Object[] a = c.toArray(); int numNew = a.length; - ensureCapacityHelper(elementCount + numNew); - int numMoved = elementCount - index; - if (numMoved > 0) - System.arraycopy(elementData, index, elementData, index + numNew, - numMoved); + if (numNew > 0) { + ensureCapacityHelper(elementCount + numNew); - System.arraycopy(a, 0, elementData, index, numNew); - elementCount += numNew; - return numNew != 0; + int numMoved = elementCount - index; + if (numMoved > 0) + System.arraycopy(elementData, index, elementData, + index + numNew, numMoved); + + System.arraycopy(a, 0, elementData, index, numNew); + elementCount += numNew; + modCount++; + } + return numNew > 0; } /** @@ -1047,12 +1063,12 @@ public class Vector * (If {@code toIndex==fromIndex}, this operation has no effect.) */ protected synchronized void removeRange(int fromIndex, int toIndex) { - modCount++; int numMoved = elementCount - toIndex; System.arraycopy(elementData, toIndex, elementData, fromIndex, numMoved); // Let gc do its work + modCount++; int newElementCount = elementCount - (toIndex-fromIndex); while (elementCount != newElementCount) elementData[--elementCount] = null; @@ -1420,7 +1436,7 @@ public class Vector } public long estimateSize() { - return (long) (getFence() - index); + return getFence() - index; } public int characteristics() {