Skip to content

Commit

Permalink
Add back IntObjectMap.values(Class<V>)
Browse files Browse the repository at this point in the history 
Motivation:

Although the new IntObjectMap.values() that returns Collection is
useful, the removed values(Class<V>) that returns an array is also
useful. It's also good for backward compatibility.

Modifications:

- Add IntObjectMap.values(Class<V>) back
- Miscellaneous improvements
  - Cache the collection returned by IntObjectHashMap.values()
  - Inspector warnings
- Update the IntObjectHashMapTest to test both values()

Result:

- Backward compatibility
- Potential performance improvement of values()
  • Loading branch information
@trustin
trustin committed Nov 21, 2014
1 parent c9e5238 commit d87be08
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 51 deletions.
89 changes: 54 additions & 35 deletions common/src/main/java/io/netty/util/collection/IntObjectHashMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

package io.netty.util.collection;

import java.lang.reflect.Array;
import java.util.AbstractCollection;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -51,6 +52,7 @@ public class IntObjectHashMap<V> implements IntObjectMap<V>, Iterable<IntObjectM

private int[] keys;
private V[] values;
private Collection<V> valueCollection;
private int size;

public IntObjectHashMap() {
Expand Down Expand Up @@ -78,7 +80,7 @@ public IntObjectHashMap(int initialCapacity, float loadFactor) {

// Allocate the arrays.
keys = new int[capacity];
@SuppressWarnings({ "unchecked", })
@SuppressWarnings({ "unchecked", "SuspiciousArrayCast" })
V[] temp = (V[]) new Object[capacity];
values = temp;

Expand Down Expand Up @@ -190,9 +192,9 @@ public boolean containsKey(int key) {
@Override
public boolean containsValue(V value) {
V v = toInternal(value);
for (int i = 0; i < values.length; ++i) {
for (V value1 : values) {
// The map supports null values; this will be matched as NULL_VALUE.equals(NULL_VALUE).
if (values[i] != null && values[i].equals(v)) {
if (value1 != null && value1.equals(v)) {
return true;
}
}
Expand Down Expand Up @@ -222,34 +224,52 @@ public int[] keys() {
}

@Override
public Collection<V> values() {
return new AbstractCollection<V>() {
@Override
public Iterator<V> iterator() {
return new Iterator<V>() {
final Iterator<Entry<V>> iter = IntObjectHashMap.this.iterator();
@Override
public boolean hasNext() {
return iter.hasNext();
}
public V[] values(Class<V> clazz) {
@SuppressWarnings("unchecked")
V[] outValues = (V[]) Array.newInstance(clazz, size());
int targetIx = 0;
for (V value : values) {
if (value != null) {
outValues[targetIx++] = value;
}
}
return outValues;
}

@Override
public V next() {
return iter.next().value();
}
@Override
public Collection<V> values() {
Collection<V> valueCollection = this.valueCollection;
if (valueCollection == null) {
this.valueCollection = valueCollection = new AbstractCollection<V>() {
@Override
public Iterator<V> iterator() {
return new Iterator<V>() {
final Iterator<Entry<V>> iter = IntObjectHashMap.this.iterator();
@Override
public boolean hasNext() {
return iter.hasNext();
}

@Override
public V next() {
return iter.next().value();
}

@Override
public void remove() {
throw new UnsupportedOperationException();
}
};
}

@Override
public void remove() {
throw new UnsupportedOperationException();
}
};
}
@Override
public int size() {
return size;
}
};
}

@Override
public int size() {
return size;
}
};
return valueCollection;
}

@Override
Expand All @@ -258,15 +278,15 @@ public int hashCode() {
// array, which may have different lengths for two maps of same size(), so the
// capacity cannot be used as input for hashing but the size can.
int hash = size;
for (int i = 0; i < keys.length; ++i) {
for (int key : keys) {
// 0 can be a valid key or unused slot, but won't impact the hashcode in either case.
// This way we can use a cheap loop without conditionals, or hard-to-unroll operations,
// or the devastatingly bad memory locality of visiting value objects.
// Also, it's important to use a hash function that does not depend on the ordering
// of terms, only their values; since the map is an unordered collection and
// entries can end up in different positions in different maps that have the same
// elements, but with different history of puts/removes, due to conflicts.
hash ^= keys[i];
hash ^= key;
}
return hash;
}
Expand Down Expand Up @@ -380,8 +400,8 @@ private void removeAt(int index) {
int nextFree = index;
for (int i = probeNext(index); values[i] != null; i = probeNext(i)) {
int bucket = hashIndex(keys[i]);
if ((i < bucket && (bucket <= nextFree || nextFree <= i))
|| (bucket <= nextFree && nextFree <= i)) {
if (i < bucket && (bucket <= nextFree || nextFree <= i) ||
bucket <= nextFree && nextFree <= i) {
// Move the displaced entry "back" to the first available position.
keys[nextFree] = keys[i];
values[nextFree] = values[i];
Expand Down Expand Up @@ -412,7 +432,7 @@ private void rehash(int newCapacity) {
V[] oldVals = values;

keys = new int[newCapacity];
@SuppressWarnings({ "unchecked" })
@SuppressWarnings({ "unchecked", "SuspiciousArrayCast" })
V[] temp = (V[]) new Object[newCapacity];
values = temp;

Expand All @@ -425,8 +445,7 @@ private void rehash(int newCapacity) {
// Inlined put(), but much simpler: we don't need to worry about
// duplicated keys, growing/rehashing, or failing to insert.
int oldKey = oldKeys[i];
int startIndex = hashIndex(oldKey);
int index = startIndex;
int index = hashIndex(oldKey);

for (;;) {
if (values[index] == null) {
Expand Down
5 changes: 5 additions & 0 deletions common/src/main/java/io/netty/util/collection/IntObjectMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,5 +114,10 @@ interface Entry<V> {
/**
* Gets the values contained in this map.
*/
V[] values(Class<V> clazz);

/**
* Gets the values contatins in this map as a {@link Collection}.
*/
Collection<V> values();
}
10 changes: 10 additions & 0 deletions common/src/main/java/io/netty/util/collection/PrimitiveCollections.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ public int[] keys() {
return EmptyArrays.EMPTY_INTS;
}

@Override
public Object[] values(Class<Object> clazz) {
return EmptyArrays.EMPTY_OBJECTS;
}

@Override
public Collection<Object> values() {
return Collections.emptyList();
Expand Down Expand Up @@ -185,6 +190,11 @@ public int[] keys() {
return map.keys();
}

@Override
public V[] values(Class<V> clazz) {
return map.values(clazz);
}

@Override
public Collection<V> values() {
return map.values();
Expand Down
37 changes: 21 additions & 16 deletions common/src/test/java/io/netty/util/collection/IntObjectHashMapTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@
*/
package io.netty.util.collection;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

import org.junit.Before;
import org.junit.Test;

Expand All @@ -30,6 +24,8 @@
import java.util.Random;
import java.util.Set;

import static org.junit.Assert.*;

/**
* Tests for {@link IntObjectHashMap}.
*/
Expand Down Expand Up @@ -281,19 +277,28 @@ public void valuesShouldBeReturned() {
map.put(4, new Value("v4"));
map.remove(4);

Collection<Value> values = map.values();
assertEquals(3, values.size());

// Ensure values() return all values.
Set<Value> expected = new HashSet<Value>();
Set<Value> actual = new HashSet<Value>();

expected.add(v1);
expected.add(v2);
expected.add(v3);

Set<Value> found = new HashSet<Value>();
for (Value value : values) {
assertTrue(found.add(value));
Value[] valueArray = map.values(Value.class);
assertEquals(3, valueArray.length);
for (Value value : valueArray) {
assertTrue(actual.add(value));
}
assertEquals(expected, found);
assertEquals(expected, actual);
actual.clear();

Collection<Value> valueCollection = map.values();
assertEquals(3, valueCollection.size());
for (Value value : valueCollection) {
assertTrue(actual.add(value));
}
assertEquals(expected, actual);
}

@Test
Expand All @@ -319,14 +324,14 @@ public void hashcodeEqualsTest() {
map2.put(key, key);
}
assertEquals(map1.hashCode(), map2.hashCode());
assertTrue(map1.equals(map2));
assertEquals(map1, map2);
// Remove one "middle" element, maps should now be non-equals.
int[] keys = map1.keys();
map2.remove(keys[50]);
assertFalse(map1.equals(map2));
// Put it back; will likely be in a different position, but maps will be equal again.
map2.put(keys[50], map1.keys()[50]);
assertTrue(map1.equals(map2));
assertEquals(map1, map2);
assertEquals(map1.hashCode(), map2.hashCode());
// Make map2 have one extra element, will be non-equal.
map2.put(1000, 1000);
Expand All @@ -340,7 +345,7 @@ public void hashcodeEqualsTest() {
map2.put(key, key);
}
assertEquals(map1.hashCode(), map2.hashCode());
assertTrue(map1.equals(map2));
assertEquals(map1, map2);
}

@Test
Expand Down

0 comments on commit d87be08

Please sign in to comment.