Skip to content

Commit

Permalink
[Java] Ensure that Object2*HashMaps and ObjectHashSet always check eq…
Browse files Browse the repository at this point in the history
…uality using the value in the map and not vice versa. (#253)

* [Java] Ensure that Object2*HashMaps and ObjectHashSet always check equality using the value in the map and not vice versa. This allows implementing asymmetric keys/values which can match multiple other types. For an example see CharSequenceKey from the tests.

* [Java] Refactor map equality tests.

* [Java] Fix Object2ObjectHashMap#containsValue to perform equality check on the value stored rather than the value passed as an argument. Ensure that the equality is always done the same, i.e. use LangUtil#exactEquals.

* [Java] Add sanity tests for Object2IntHashMap and missing value handling.

* [Java] Simplify Object2ObjectHashMap.

* [Java] Fix Object2IntHashMap put/containsKey/removeKey key equality check using the key from the map as the source of the comparison.

* [Java] Avoid loading keys/values/entries multiple times during lookup and bulk operations.

* [Java] Fix EntryIterator/MapEntry equals/hashCode implementations, i.e. handle null values as keys/values and use `Objects.equals` for the equality checks.

* [Java] Replace LangUtil.exactEquals with Objects.equals.

* [Java] Add tests for `Int2Int*Map.compact()`.

* [Java] Suppress lgtm IOOBE warnings.

* [Java] Update LGTM suppression syntax.

(cherry picked from commit a320110)
  • Loading branch information
vyazelenko committed May 16, 2022
1 parent 3dbf8bf commit 164e20b
Show file tree
Hide file tree
Showing 25 changed files with 1,418 additions and 407 deletions.
2 changes: 1 addition & 1 deletion agrona/src/main/java/org/agrona/BitUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public static byte[] fromHexByteArray(final byte[] buffer)
for (int i = 0; i < buffer.length; i += 2)
{
final int hi = FROM_HEX_DIGIT_TABLE[buffer[i]] << 4;
final int lo = FROM_HEX_DIGIT_TABLE[buffer[i + 1]]; // lgtm [java/index-out-of-bounds]
final int lo = FROM_HEX_DIGIT_TABLE[buffer[i + 1]]; // lgtm[java/index-out-of-bounds]
outputBuffer[i >> 1] = (byte)(hi | lo);
}

Expand Down
1 change: 1 addition & 0 deletions agrona/src/main/java/org/agrona/LangUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,5 @@ private static <T extends Throwable> void rethrow(final Throwable t) throws T
{
throw (T)t;
}

}
35 changes: 25 additions & 10 deletions agrona/src/main/java/org/agrona/collections/BiInt2ObjectMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,16 @@ public V put(final int keyPartA, final int keyPartB, final V value)
{
final long key = compoundKey(keyPartA, keyPartB);

V oldValue = null;
final long[] keys = this.keys;
final Object[] values = this.values;
final int mask = values.length - 1;
int index = Hashing.hash(key, mask);

while (null != values[index])
Object oldValue;
while (null != (oldValue = values[index]))
{
if (key == keys[index])
{
oldValue = (V)values[index];
break;
}

Expand All @@ -192,7 +193,7 @@ public V put(final int keyPartA, final int keyPartB, final V value)
increaseCapacity();
}

return oldValue;
return (V)oldValue;
}

/**
Expand All @@ -206,6 +207,8 @@ public V put(final int keyPartA, final int keyPartB, final V value)
public V get(final int keyPartA, final int keyPartB)
{
final long key = compoundKey(keyPartA, keyPartB);
final long[] keys = this.keys;
final Object[] values = this.values;
final int mask = values.length - 1;
int index = Hashing.hash(key, mask);

Expand Down Expand Up @@ -234,6 +237,8 @@ public V get(final int keyPartA, final int keyPartB)
public V remove(final int keyPartA, final int keyPartB)
{
final long key = compoundKey(keyPartA, keyPartB);
final long[] keys = this.keys;
final Object[] values = this.values;
final int mask = values.length - 1;
int index = Hashing.hash(key, mask);

Expand Down Expand Up @@ -268,10 +273,10 @@ public V remove(final int keyPartA, final int keyPartB)
public V computeIfAbsent(final int keyPartA, final int keyPartB, final EntryFunction<? extends V> mappingFunction)
{
V value = get(keyPartA, keyPartB);
if (value == null)
if (null == value)
{
value = mappingFunction.apply(keyPartA, keyPartB);
if (value != null)
if (null != value)
{
put(keyPartA, keyPartB, value);
}
Expand All @@ -290,6 +295,7 @@ public void forEach(final Consumer<V> consumer)
{
int remaining = size;

final Object[] values = this.values;
for (int i = 0, size = values.length; remaining > 0 && i < size; i++)
{
final Object value = values[i];
Expand All @@ -311,6 +317,8 @@ public void forEach(final EntryConsumer<V> consumer)
{
int remaining = size;

final long[] keys = this.keys;
final Object[] values = this.values;
for (int i = 0, size = values.length; remaining > 0 && i < size; i++)
{
final Object value = values[i];
Expand Down Expand Up @@ -354,6 +362,8 @@ public String toString()
final StringBuilder sb = new StringBuilder();
sb.append('{');

final long[] keys = this.keys;
final Object[] values = this.values;
for (int i = 0, size = values.length; i < size; i++)
{
final Object value = values[i];
Expand Down Expand Up @@ -385,6 +395,8 @@ private void rehash(final int newCapacity)
final long[] tempKeys = new long[newCapacity];
final Object[] tempValues = new Object[newCapacity];

final long[] keys = this.keys;
final Object[] values = this.values;
for (int i = 0, size = values.length; i < size; i++)
{
final Object value = values[i];
Expand All @@ -403,19 +415,22 @@ private void rehash(final int newCapacity)
}
}

keys = tempKeys;
values = tempValues;
this.keys = tempKeys;
this.values = tempValues;
}

@SuppressWarnings("FinalParameters")
private void compactChain(int deleteIndex)
{
final int mask = values.length - 1;
int index = deleteIndex;
final long[] keys = this.keys;
final Object[] values = this.values;
while (true)
{
index = ++index & mask;
if (null == values[index])
final Object value = values[index];
if (null == value)
{
break;
}
Expand All @@ -427,7 +442,7 @@ private void compactChain(int deleteIndex)
(hash <= deleteIndex && deleteIndex <= index))
{
keys[deleteIndex] = key;
values[deleteIndex] = values[index];
values[deleteIndex] = value;

values[index] = null;
deleteIndex = index;
Expand Down
Loading

0 comments on commit 164e20b

Please sign in to comment.