You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by je...@apache.org on 2009/12/09 10:52:37 UTC
svn commit: r888752 - in /harmony/enhanced/classlib/trunk/modules/luni/src:
main/java/java/util/EnumSet.java main/java/java/util/HugeEnumSet.java
main/java/java/util/MiniEnumSet.java
test/api/common/org/apache/harmony/luni/tests/java/util/EnumSetTest.java
Author: jessewilson
Date: Wed Dec 9 09:52:37 2009
New Revision: 888752
URL: http://svn.apache.org/viewvc?rev=888752&view=rev
Log:
Give much needed attention to our EnumSet implementations.
Fix HugeEnumSet iterator.remove() bug
Our iterator used a single index variable to track both the most recently returned element and the next element. When these disagreed, remove was broken. Added a test case for this.
Fix HugeEnumSet concurrency bug
The old code used of fields to track the intermediate values bitsIndex, elementIndex, and oldIndex. This meant that concurrent reads were not thread safe, even if the EnumSet was never modified after construction.
Fix retainAll bug
Calling retainAll() on an empty enum set would return true when the passed in argument was also an enum set.
Replace "collection.size() == 0" with "collection.isEmpty()"
This is inefficient for ConcurrentHashMap, which must do a full iteration.
Remove no-op arrays.fill() in HugeEnumSet constructor
Newly allocated arrays are initialized with 0s, this was a no-op.
Throw on clone failure
Previously we swept an impossible clone failure under the rug by returning null. This can't happen; we should throw AssertionErrors if it does.
Improve type safety
Remove use of raw types. Move warning suppressions to single lines and document why the suppressions are safe.
Increase internal implementation consistency
The new structure of add/addAll, remove/removeAll, retainAll is consistent across all: for each method, calculate the newBits. Compare old and new bits and if they disagree, update size and return true.
Improve readability
Flip "if (null == foo)" to "if (foo == null)" etc.
Investigate performance benefits of mod+divide vs. mask+shift
On both DRLVM and the RI's VM, mod & divide by a constant factor performed no worse than mask & shift. I'll investigate how Dalvik behaves on a device; I may later change this code to use mask+shift.
Modified:
harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/EnumSet.java
harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/HugeEnumSet.java
harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/MiniEnumSet.java
harmony/enhanced/classlib/trunk/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/EnumSetTest.java
Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/EnumSet.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/EnumSet.java?rev=888752&r1=888751&r2=888752&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/EnumSet.java (original)
+++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/EnumSet.java Wed Dec 9 09:52:37 2009
@@ -103,7 +103,7 @@
if (c instanceof EnumSet) {
return copyOf((EnumSet<E>) c);
}
- if (0 == c.size()) {
+ if (c.isEmpty()) {
throw new IllegalArgumentException();
}
Iterator<E> iterator = c.iterator();
@@ -304,15 +304,13 @@
@Override
public EnumSet<E> clone() {
try {
- Object set = super.clone();
- return (EnumSet<E>) set;
+ return (EnumSet<E>) super.clone();
} catch (CloneNotSupportedException e) {
- return null;
+ throw new AssertionError(e);
}
}
- @SuppressWarnings("unchecked")
- boolean isValidType(Class cls) {
+ boolean isValidType(Class<?> cls) {
return cls == elementClass || cls.getSuperclass() == elementClass;
}
Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/HugeEnumSet.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/HugeEnumSet.java?rev=888752&r1=888751&r2=888752&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/HugeEnumSet.java (original)
+++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/HugeEnumSet.java Wed Dec 9 09:52:37 2009
@@ -18,90 +18,95 @@
/**
- * This is a concrete subclass of EnumSet designed specifically for enum type
- * with more than 64 elements.
- *
+ * A concrete EnumSet for enums with more than 64 elements.
*/
@SuppressWarnings("serial")
final class HugeEnumSet<E extends Enum<E>> extends EnumSet<E> {
+ private static final int BIT_IN_LONG = 64;
+
final private E[] enums;
private long[] bits;
private int size;
- private static final int BIT_IN_LONG = 64;
-
HugeEnumSet(Class<E> elementType) {
super(elementType);
enums = elementType.getEnumConstants();
bits = new long[(enums.length + BIT_IN_LONG - 1) / BIT_IN_LONG];
- Arrays.fill(bits, 0);
}
private class HugeEnumSetIterator implements Iterator<E> {
- private long[] unProcessedBits;
+ /**
+ * The bits yet to be returned for the long in bits[index]. As values from the current index
+ * are returned, their bits are zeroed out. When this reaches zero, the index must be
+ * incremented.
+ */
+ private long currentBits = bits[0];
- private int bitsPosition = 0;
+ /**
+ * The index into HugeEnumSet.bits of the next value to return.
+ */
+ private int index;
- /*
- * Mask for current element.
+ /**
+ * The single bit of the next value to return.
*/
- private long currentElementMask = 0;
+ private long mask;
- boolean canProcess = true;
+ /**
+ * The candidate for removal. If null, no value may be removed.
+ */
+ private E last;
private HugeEnumSetIterator() {
- unProcessedBits = new long[bits.length];
- System.arraycopy(bits, 0, unProcessedBits, 0, bits.length);
- bitsPosition = unProcessedBits.length;
- findNextNoneZeroPosition(0);
- if (bitsPosition == unProcessedBits.length) {
- canProcess = false;
- }
+ computeNextElement();
}
- private void findNextNoneZeroPosition(int start) {
- for (int i = start; i < unProcessedBits.length; i++) {
- if (0 != bits[i]) {
- bitsPosition = i;
- break;
+ /**
+ * Assigns mask and index to the next available value, cycling currentBits as necessary.
+ */
+ void computeNextElement() {
+ while (true) {
+ if (currentBits != 0) {
+ mask = currentBits & -currentBits; // the lowest 1 bit in currentBits
+ return;
+ } else if (++index < bits.length) {
+ currentBits = bits[index];
+ } else {
+ mask = 0;
+ return;
}
}
}
public boolean hasNext() {
- return canProcess;
+ return mask != 0;
}
public E next() {
- if (!canProcess) {
+ if (mask == 0) {
throw new NoSuchElementException();
}
- currentElementMask = unProcessedBits[bitsPosition]
- & (-unProcessedBits[bitsPosition]);
- unProcessedBits[bitsPosition] -= currentElementMask;
- int index = Long.numberOfTrailingZeros(currentElementMask)
- + bitsPosition * BIT_IN_LONG;
- if (0 == unProcessedBits[bitsPosition]) {
- int oldBitsPosition = bitsPosition;
- findNextNoneZeroPosition(bitsPosition + 1);
- if (bitsPosition == oldBitsPosition) {
- canProcess = false;
- }
- }
- return enums[index];
+
+ int ordinal = Long.numberOfTrailingZeros(mask) + index * BIT_IN_LONG;
+ last = enums[ordinal];
+
+ currentBits &= ~mask;
+ computeNextElement();
+
+ return last;
}
public void remove() {
- if (currentElementMask == 0) {
+ if (last == null) {
throw new IllegalStateException();
}
- bits[bitsPosition] &= ~currentElementMask;
- size--;
- currentElementMask = 0;
+
+ HugeEnumSet.this.remove(last);
+ last = null;
}
}
@@ -110,38 +115,44 @@
if (!isValidType(element.getDeclaringClass())) {
throw new ClassCastException();
}
- calculateElementIndex(element);
- bits[bitsIndex] |= (1l << elementInBits);
- if (oldBits == bits[bitsIndex]) {
- return false;
+ int ordinal = element.ordinal();
+ int index = ordinal / BIT_IN_LONG;
+ int inBits = ordinal % BIT_IN_LONG;
+ long oldBits = bits[index];
+ long newBits = oldBits | (1L << inBits);
+ if (oldBits != newBits) {
+ bits[index] = newBits;
+ size++;
+ return true;
}
- size++;
- return true;
+ return false;
}
@Override
public boolean addAll(Collection<? extends E> collection) {
- if (0 == collection.size() || this == collection) {
+ if (collection.isEmpty() || collection == this) {
return false;
}
+
if (collection instanceof EnumSet) {
- EnumSet set = (EnumSet) collection;
+ EnumSet<?> set = (EnumSet<?>) collection;
if (!isValidType(set.elementClass)) {
throw new ClassCastException();
}
- HugeEnumSet hugeSet = (HugeEnumSet) set;
- boolean addSuccessful = false;
+
+ HugeEnumSet<E> hugeSet = (HugeEnumSet<E>) set;
+ boolean changed = false;
for (int i = 0; i < bits.length; i++) {
- oldBits = bits[i];
- bits[i] |= hugeSet.bits[i];
- if (oldBits != bits[i]) {
- addSuccessful = true;
- size = size - Long.bitCount(oldBits)
- + Long.bitCount(bits[i]);
+ long oldBits = bits[i];
+ long newBits = oldBits | hugeSet.bits[i];
+ if (oldBits != newBits) {
+ bits[i] = newBits;
+ size += Long.bitCount(newBits) - Long.bitCount(oldBits);
+ changed = true;
}
}
- return addSuccessful;
+ return changed;
}
return super.addAll(collection);
}
@@ -173,43 +184,39 @@
}
}
- @SuppressWarnings("unchecked")
@Override
public boolean contains(Object object) {
- if (null == object) {
+ if (object == null || !isValidType(object.getClass())) {
return false;
}
- if (!isValidType(object.getClass())) {
- return false;
- }
- calculateElementIndex((E)object);
- return (bits[bitsIndex] & (1l << elementInBits)) != 0;
+
+ @SuppressWarnings("unchecked") // guarded by isValidType()
+ int ordinal = ((E) object).ordinal();
+ int index = ordinal / BIT_IN_LONG;
+ int inBits = ordinal % BIT_IN_LONG;
+ return (bits[index] & (1L << inBits)) != 0;
}
@Override
- @SuppressWarnings("unchecked")
public HugeEnumSet<E> clone() {
- Object set = super.clone();
- if (null != set) {
- ((HugeEnumSet<E>) set).bits = bits.clone();
- return (HugeEnumSet<E>) set;
- }
- return null;
+ HugeEnumSet<E> set = (HugeEnumSet<E>) super.clone();
+ set.bits = bits.clone();
+ return set;
}
@Override
public boolean containsAll(Collection<?> collection) {
- if (collection.size() == 0) {
+ if (collection.isEmpty()) {
return true;
}
if (collection instanceof HugeEnumSet) {
- HugeEnumSet set = (HugeEnumSet) collection;
- if(isValidType(set.elementClass )) {
- for(int i = 0; i < bits.length; i++) {
- if((bits[i] & set.bits[i]) != set.bits[i]){
+ HugeEnumSet<?> set = (HugeEnumSet<?>) collection;
+ if (isValidType(set.elementClass)) {
+ for (int i = 0; i < bits.length; i++) {
+ long setBits = set.bits[i];
+ if ((bits[i] & setBits) != setBits) {
return false;
}
-
}
return true;
}
@@ -219,13 +226,13 @@
@Override
public boolean equals(Object object) {
- if (null == object) {
+ if (object == null) {
return false;
}
if (!isValidType(object.getClass())) {
return super.equals(object);
}
- return Arrays.equals(bits, ((HugeEnumSet) object).bits);
+ return Arrays.equals(bits, ((HugeEnumSet<?>) object).bits);
}
@Override
@@ -235,38 +242,48 @@
@Override
public boolean remove(Object object) {
- if (!contains(object)) {
+ if (object == null || !isValidType(object.getClass())) {
return false;
}
- bits[bitsIndex] -= (1l << elementInBits);
- size--;
- return true;
+
+ @SuppressWarnings("unchecked") // guarded by isValidType()
+ int ordinal = ((E) object).ordinal();
+ int index = ordinal / BIT_IN_LONG;
+ int inBits = ordinal % BIT_IN_LONG;
+ long oldBits = bits[index];
+ long newBits = oldBits & ~(1L << inBits);
+ if (oldBits != newBits) {
+ bits[index] = newBits;
+ size--;
+ return true;
+ }
+ return false;
}
@Override
public boolean removeAll(Collection<?> collection) {
- if (0 == collection.size()) {
+ if (collection.isEmpty()) {
return false;
}
if (collection instanceof EnumSet) {
- EnumSet<E> set = (EnumSet<E>) collection;
+ EnumSet<?> set = (EnumSet<?>) collection;
if (!isValidType(set.elementClass)) {
return false;
}
- boolean removeSuccessful = false;
- long mask = 0;
+
+ HugeEnumSet<E> hugeSet = (HugeEnumSet<E>) set;
+ boolean changed = false;
for (int i = 0; i < bits.length; i++) {
- oldBits = bits[i];
- mask = bits[i] & ((HugeEnumSet<E>) set).bits[i];
- if (mask != 0) {
- bits[i] -= mask;
- size = (size - Long.bitCount(oldBits) + Long
- .bitCount(bits[i]));
- removeSuccessful = true;
+ long oldBits = bits[i];
+ long newBits = oldBits & ~hugeSet.bits[i];
+ if (oldBits != newBits) {
+ bits[i] = newBits;
+ size += Long.bitCount(newBits) - Long.bitCount(oldBits);
+ changed = true;
}
}
- return removeSuccessful;
+ return changed;
}
return super.removeAll(collection);
}
@@ -274,72 +291,65 @@
@Override
public boolean retainAll(Collection<?> collection) {
if (collection instanceof EnumSet) {
- EnumSet<E> set = (EnumSet<E>) collection;
+ EnumSet<?> set = (EnumSet<?>) collection;
if (!isValidType(set.elementClass)) {
- clear();
- return true;
+ if (size > 0) {
+ clear();
+ return true;
+ } else {
+ return false;
+ }
}
- boolean retainSuccessful = false;
- oldBits = 0;
+ HugeEnumSet<E> hugeSet = (HugeEnumSet<E>) set;
+ boolean changed = false;
for (int i = 0; i < bits.length; i++) {
- oldBits = bits[i];
- bits[i] &= ((HugeEnumSet<E>) set).bits[i];
- if (oldBits != bits[i]) {
- size = size - Long.bitCount(oldBits)
- + Long.bitCount(bits[i]);
- retainSuccessful = true;
+ long oldBits = bits[i];
+ long newBits = oldBits & hugeSet.bits[i];
+ if (oldBits != newBits) {
+ bits[i] = newBits;
+ size += Long.bitCount(newBits) - Long.bitCount(oldBits);
+ changed = true;
}
}
- return retainSuccessful;
+ return changed;
}
return super.retainAll(collection);
}
@Override
void setRange(E start, E end) {
- calculateElementIndex(start);
- int startBitsIndex = bitsIndex;
- int startElementInBits = elementInBits;
- calculateElementIndex(end);
- int endBitsIndex = bitsIndex;
- int endElementInBits = elementInBits;
- long range = 0;
- if (startBitsIndex == endBitsIndex) {
- range = (-1l >>> (BIT_IN_LONG -(endElementInBits - startElementInBits + 1))) << startElementInBits;
- size -= Long.bitCount(bits[bitsIndex]);
- bits[bitsIndex] |= range;
- size += Long.bitCount(bits[bitsIndex]);
+ int startOrdinal = start.ordinal();
+ int startIndex = startOrdinal / BIT_IN_LONG;
+ int startInBits = startOrdinal % BIT_IN_LONG;
+
+ int endOrdinal = end.ordinal();
+ int endIndex = endOrdinal / BIT_IN_LONG;
+ int endInBits = endOrdinal % BIT_IN_LONG;
+
+ if (startIndex == endIndex) {
+ long range = (-1L >>> (BIT_IN_LONG -(endInBits - startInBits + 1))) << startInBits;
+ size -= Long.bitCount(bits[startIndex]);
+ bits[startIndex] |= range;
+ size += Long.bitCount(bits[startIndex]);
+
} else {
- range = (-1l >>> startElementInBits) << startElementInBits;
- size -= Long.bitCount(bits[startBitsIndex]);
- bits[startBitsIndex] |= range;
- size += Long.bitCount(bits[startBitsIndex]);
-
- // endElementInBits + 1 is the number of consecutive ones.
- // 63 - endElementInBits is the following zeros of the right most one.
- range = -1l >>> (BIT_IN_LONG - (endElementInBits + 1));
- size -= Long.bitCount(bits[endBitsIndex]);
- bits[endBitsIndex] |= range;
- size += Long.bitCount(bits[endBitsIndex]);
- for(int i = (startBitsIndex + 1); i <= (endBitsIndex - 1); i++) {
+ long range = (-1L >>> startInBits) << startInBits;
+ size -= Long.bitCount(bits[startIndex]);
+ bits[startIndex] |= range;
+ size += Long.bitCount(bits[startIndex]);
+
+ // endInBits + 1 is the number of consecutive ones.
+ // 63 - endInBits is the following zeros of the right most one.
+ range = -1L >>> (BIT_IN_LONG - (endInBits + 1));
+ size -= Long.bitCount(bits[endIndex]);
+ bits[endIndex] |= range;
+ size += Long.bitCount(bits[endIndex]);
+ for (int i = (startIndex + 1); i <= (endIndex - 1); i++) {
size -= Long.bitCount(bits[i]);
- bits[i] = -1l;
+ bits[i] = -1L;
size += Long.bitCount(bits[i]);
}
}
}
-
- private void calculateElementIndex(E element) {
- int elementOrdinal = element.ordinal();
- bitsIndex = elementOrdinal / BIT_IN_LONG;
- elementInBits = elementOrdinal % BIT_IN_LONG;
- oldBits = bits[bitsIndex];
- }
-
- private int bitsIndex;
-
- private int elementInBits;
-
- private long oldBits;
}
Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/MiniEnumSet.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/MiniEnumSet.java?rev=888752&r1=888751&r2=888752&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/MiniEnumSet.java (original)
+++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/util/MiniEnumSet.java Wed Dec 9 09:52:37 2009
@@ -18,9 +18,7 @@
/**
- * This is a concrete subclass of EnumSet designed specifically for enum type
- * with less than or equal to 64 elements.
- *
+ * A concrete EnumSet for enums with 64 or fewer elements.
*/
@SuppressWarnings("serial")
final class MiniEnumSet<E extends Enum<E>> extends EnumSet<E> {
@@ -39,45 +37,47 @@
private class MiniEnumSetIterator implements Iterator<E> {
- private long unProcessedBits;
-
- /*
- * Mask for current element.
+ /**
+ * The bits yet to be returned for bits. As values from the current index are returned,
+ * their bits are zeroed out.
*/
- private long currentElementMask;
+ private long currentBits = bits;
- private boolean canProcess = true;
+ /**
+ * The single bit of the next value to return.
+ */
+ private long mask = currentBits & -currentBits; // the lowest 1 bit in currentBits
- private MiniEnumSetIterator() {
- unProcessedBits = bits;
- if (0 == unProcessedBits) {
- canProcess = false;
- }
- }
+ /**
+ * The candidate for removal. If null, no value may be removed.
+ */
+ private E last;
public boolean hasNext() {
- return canProcess;
+ return mask != 0;
}
public E next() {
- if (!canProcess) {
+ if (mask == 0) {
throw new NoSuchElementException();
}
- currentElementMask = unProcessedBits & (-unProcessedBits);
- unProcessedBits -= currentElementMask;
- if (0 == unProcessedBits) {
- canProcess = false;
- }
- return enums[Long.numberOfTrailingZeros(currentElementMask)];
+
+ int ordinal = Long.numberOfTrailingZeros(mask);
+ last = enums[ordinal];
+
+ currentBits &= ~mask;
+ mask = currentBits & -currentBits; // the lowest 1 bit in currentBits
+
+ return last;
}
public void remove() {
- if ( currentElementMask == 0 ) {
+ if (last == null) {
throw new IllegalStateException();
}
- bits &= ~currentElementMask;
- size = Long.bitCount(bits);
- currentElementMask = 0;
+
+ MiniEnumSet.this.remove(last);
+ last = null;
}
}
@@ -102,77 +102,83 @@
if (!isValidType(element.getDeclaringClass())) {
throw new ClassCastException();
}
- long mask = 1l << element.ordinal();
- if ((bits & mask) == mask) {
- return false;
- }
- bits |= mask;
- size++;
- return true;
+ long oldBits = bits;
+ long newBits = oldBits | (1L << element.ordinal());
+ if (oldBits != newBits) {
+ bits = newBits;
+ size++;
+ return true;
+ }
+ return false;
}
@Override
public boolean addAll(Collection<? extends E> collection) {
- if (0 == collection.size()) {
+ if (collection.isEmpty()) {
return false;
}
if (collection instanceof EnumSet) {
- EnumSet<?> set = (EnumSet)collection;
+ EnumSet<?> set = (EnumSet<?>) collection;
if (!isValidType(set.elementClass)) {
throw new ClassCastException();
}
+
MiniEnumSet<?> miniSet = (MiniEnumSet<?>) set;
long oldBits = bits;
- bits |= miniSet.bits;
- size = Long.bitCount(bits);
- return (oldBits != bits);
+ long newBits = oldBits | miniSet.bits;
+ bits = newBits;
+ size = Long.bitCount(newBits);
+ return (oldBits != newBits);
}
return super.addAll(collection);
}
@Override
public boolean contains(Object object) {
- if (null == object) {
- return false;
- }
- if (!isValidType(object.getClass())) {
+ if (object == null || !isValidType(object.getClass())) {
return false;
}
- Enum<?> element = (Enum<?>) object;
+
+ @SuppressWarnings("unchecked") // guarded by isValidType()
+ Enum<E> element = (Enum<E>) object;
int ordinal = element.ordinal();
- return (bits & (1l << ordinal)) != 0;
+ return (bits & (1L << ordinal)) != 0;
}
@Override
public boolean containsAll(Collection<?> collection) {
- if (collection.size() == 0) {
+ if (collection.isEmpty()) {
return true;
}
if (collection instanceof MiniEnumSet) {
MiniEnumSet<?> set = (MiniEnumSet<?>) collection;
- return isValidType(set.elementClass ) && ((bits & set.bits) == set.bits);
+ long setBits = set.bits;
+ return isValidType(set.elementClass) && ((bits & setBits) == setBits);
}
return !(collection instanceof EnumSet) && super.containsAll(collection);
}
@Override
public boolean removeAll(Collection<?> collection) {
- if (0 == collection.size()) {
+ if (collection.isEmpty()) {
return false;
}
if (collection instanceof EnumSet) {
- EnumSet<E> set = (EnumSet<E>) collection;
- boolean removeSuccessful = false;
- if (isValidType(set.elementClass)) {
- long mask = bits & ((MiniEnumSet<E>) set).bits;
- if (mask != 0) {
- bits -= mask;
- size = Long.bitCount(bits);
- removeSuccessful = true;
- }
+ EnumSet<?> set = (EnumSet<?>) collection;
+ if (!isValidType(set.elementClass)) {
+ return false;
+ }
+
+ MiniEnumSet<E> miniSet = (MiniEnumSet<E>) set;
+ long oldBits = bits;
+ long newBits = oldBits & ~miniSet.bits;
+ if (oldBits != newBits) {
+ bits = newBits;
+ size = Long.bitCount(newBits);
+ return true;
}
- return removeSuccessful;
+ return false;
}
return super.removeAll(collection);
}
@@ -180,33 +186,46 @@
@Override
public boolean retainAll(Collection<?> collection) {
if (collection instanceof EnumSet) {
- EnumSet<E> set = (EnumSet<E>) collection;
+ EnumSet<?> set = (EnumSet<?>) collection;
if (!isValidType(set.elementClass)) {
- clear();
- return true;
+ if (size > 0) {
+ clear();
+ return true;
+ } else {
+ return false;
+ }
}
- boolean retainSuccessful = false;
+
+ MiniEnumSet<E> miniSet = (MiniEnumSet<E>) set;
long oldBits = bits;
- bits &= ((MiniEnumSet<E>)set).bits;
- if (oldBits != bits) {
- size = Long.bitCount(bits);
- retainSuccessful = true;
+ long newBits = oldBits & miniSet.bits;
+ if (oldBits != newBits) {
+ bits = newBits;
+ size = Long.bitCount(newBits);
+ return true;
}
- return retainSuccessful;
+ return false;
}
return super.retainAll(collection);
}
@Override
public boolean remove(Object object) {
- if (!contains(object)) {
+ if (object == null || !isValidType(object.getClass())) {
return false;
}
- Enum<?> element = (Enum<?>) object;
+
+ @SuppressWarnings("unchecked") // guarded by isValidType()
+ Enum<E> element = (Enum<E>) object;
int ordinal = element.ordinal();
- bits -= (1l << ordinal);
- size--;
- return true;
+ long oldBits = bits;
+ long newBits = oldBits & ~(1L << ordinal);
+ if (oldBits != newBits) {
+ bits = newBits;
+ size--;
+ return true;
+ }
+ return false;
}
@Override
@@ -214,18 +233,18 @@
if (!(object instanceof EnumSet)) {
return super.equals(object);
}
- EnumSet<?> set =(EnumSet<?>)object;
- if( !isValidType(set.elementClass) ) {
- return size == 0 && set.size() == 0;
+ EnumSet<?> set =(EnumSet<?>) object;
+ if (!isValidType(set.elementClass)) {
+ return size == 0 && set.isEmpty();
}
- return bits == ((MiniEnumSet<?>)set).bits;
+ return bits == ((MiniEnumSet<?>) set).bits;
}
@Override
void complement() {
- if (0 != enums.length) {
+ if (enums.length != 0) {
bits = ~bits;
- bits &= (-1l >>> (MAX_ELEMENTS - enums.length));
+ bits &= (-1L >>> (MAX_ELEMENTS - enums.length));
size = enums.length - size;
}
}
@@ -233,7 +252,7 @@
@Override
void setRange(E start, E end) {
int length = end.ordinal() - start.ordinal() + 1;
- long range = (-1l >>> (MAX_ELEMENTS - length)) << start.ordinal();
+ long range = (-1L >>> (MAX_ELEMENTS - length)) << start.ordinal();
bits |= range;
size = Long.bitCount(bits);
}
Modified: harmony/enhanced/classlib/trunk/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/EnumSetTest.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/EnumSetTest.java?rev=888752&r1=888751&r2=888752&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/EnumSetTest.java (original)
+++ harmony/enhanced/classlib/trunk/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/EnumSetTest.java Wed Dec 9 09:52:37 2009
@@ -137,6 +137,25 @@
}
}
+ public void testRemoveIteratorRemoveFromHugeEnumSet() {
+ EnumSet<HugeEnumCount> set = EnumSet.noneOf(HugeEnumCount.class);
+ set.add(HugeEnumCount.NO64);
+ set.add(HugeEnumCount.NO65);
+ set.add(HugeEnumCount.NO128);
+ Iterator<HugeEnumCount> iterator = set.iterator();
+ assertTrue(iterator.hasNext());
+ assertEquals(HugeEnumCount.NO64, iterator.next());
+ assertTrue(iterator.hasNext());
+ iterator.remove();
+ assertEquals(HugeEnumCount.NO65, iterator.next());
+ assertTrue(iterator.hasNext());
+ assertEquals(HugeEnumCount.NO128, iterator.next());
+ assertFalse(iterator.hasNext());
+ assertEquals(EnumSet.of(HugeEnumCount.NO65, HugeEnumCount.NO128), set);
+ iterator.remove();
+ assertEquals(EnumSet.of(HugeEnumCount.NO65), set);
+ }
+
/**
* @tests java.util.EnumSet#allOf(java.lang.Class)
*/
@@ -1220,21 +1239,21 @@
Set<EnumWithInnerClass> setWithInnerClass = EnumSet
.allOf(EnumWithInnerClass.class);
result = set.retainAll(setWithInnerClass);
- assertTrue("Should return true", result); //$NON-NLS-1$
+ assertFalse("Should return false", result); //$NON-NLS-1$
assertEquals("Size of set should be 0", 0, set.size()); //$NON-NLS-1$
setWithInnerClass = EnumSet.noneOf(EnumWithInnerClass.class);
result = set.retainAll(setWithInnerClass);
- assertTrue("Should return true", result); //$NON-NLS-1$
+ assertFalse("Should return false", result); //$NON-NLS-1$
Set<EmptyEnum> emptySet = EnumSet.allOf(EmptyEnum.class);
result = set.retainAll(emptySet);
- assertTrue("Should return true", result); //$NON-NLS-1$
+ assertFalse("Should return false", result); //$NON-NLS-1$
Set<EnumWithAllInnerClass> setWithAllInnerClass = EnumSet
.allOf(EnumWithAllInnerClass.class);
result = set.retainAll(setWithAllInnerClass);
- assertTrue("Should return true", result); //$NON-NLS-1$
+ assertFalse("Should return false", result); //$NON-NLS-1$
set.add(EnumFoo.a);
result = set.retainAll(setWithInnerClass);
@@ -1314,17 +1333,17 @@
Set<HugeEnumWithInnerClass> hugeSetWithInnerClass = EnumSet
.allOf(HugeEnumWithInnerClass.class);
result = hugeSet.retainAll(hugeSetWithInnerClass);
- assertTrue(result);
+ assertFalse(result);
assertEquals(0, hugeSet.size());
hugeSetWithInnerClass = EnumSet.noneOf(HugeEnumWithInnerClass.class);
result = hugeSet.retainAll(hugeSetWithInnerClass);
- assertTrue(result);
+ assertFalse(result);
Set<HugeEnumWithInnerClass> hugeSetWithAllInnerClass = EnumSet
.allOf(HugeEnumWithInnerClass.class);
result = hugeSet.retainAll(hugeSetWithAllInnerClass);
- assertTrue(result);
+ assertFalse(result);
hugeSet.add(HugeEnum.a);
result = hugeSet.retainAll(hugeSetWithInnerClass);