Skip to content

Commit

Permalink
Stop using apache commons MutableInt/MutableLong (#5450)
Browse files Browse the repository at this point in the history
Replaces nearly all usages of MutableInt/MutableLong from Apache
Commons with our own implementation that is missing the various
footguns. This corrects some int/long confusion issues stemming from
methods that take Number, and incorrect intValue/longValue calls made
on these objects.

Partial #5412
  • Loading branch information
niloc132 authored May 30, 2024
1 parent 33238dc commit 6ebd418
Show file tree
Hide file tree
Showing 355 changed files with 2,224 additions and 1,937 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import io.deephaven.util.QueryConstants;
import io.deephaven.chunk.Chunk;
import io.deephaven.chunk.LongChunk;
import org.apache.commons.lang3.mutable.MutableLong;
import io.deephaven.util.mutable.MutableLong;

import java.util.Arrays;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -224,24 +224,24 @@ public RowSet makeRowSet() {
RowSetUtils.forAllInvertedLongRanges(rowSet, nullsForCol, (first, last) -> {
if (first > 0) {
// Advance to (first - 1)
keysIterator.getNextRowSequenceWithLength(first - 1 - position.longValue());
keysIterator.getNextRowSequenceWithLength(first - 1 - position.get());
build.addKey(keysIterator.peekNextKey());
// Advance to first
keysIterator.getNextRowSequenceWithLength(1);
build.addKey(keysIterator.peekNextKey());

position.setValue(first);
position.set(first);
}

if (last < indexSize - 1) {
// Advance to last
keysIterator.getNextRowSequenceWithLength(last - position.longValue());
keysIterator.getNextRowSequenceWithLength(last - position.get());
build.addKey(keysIterator.peekNextKey());
// Advance to (last + 1)
keysIterator.getNextRowSequenceWithLength(1);
build.addKey(keysIterator.peekNextKey());

position.setValue(last + 1);
position.set(last + 1);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import io.deephaven.engine.testutil.testcase.RefreshingTableTestCase;
import io.deephaven.engine.util.TableTools;
import io.deephaven.engine.table.ColumnSource;
import org.apache.commons.lang3.mutable.MutableLong;
import io.deephaven.util.mutable.MutableLong;
import org.jetbrains.annotations.NotNull;

import java.util.concurrent.CountDownLatch;
Expand Down Expand Up @@ -192,7 +192,7 @@ public Long uniqueIdPrev(long index) {
@Override
public void loadData(MutableLong data, long index, boolean usePrev) {
final ColumnSource<Long> columnSource = table().getColumnSource("Value", long.class);
data.setValue(usePrev ? columnSource.getPrevLong(index) : columnSource.getLong(index));
data.set(usePrev ? columnSource.getPrevLong(index) : columnSource.getLong(index));
}
},
nKeys);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
package io.deephaven.util.datastructures;

import io.deephaven.configuration.Configuration;
import org.apache.commons.lang3.mutable.MutableInt;
import io.deephaven.util.mutable.MutableInt;
import org.jetbrains.annotations.NotNull;

import java.lang.ref.ReferenceQueue;
Expand Down Expand Up @@ -227,7 +227,7 @@ private static String build(@NotNull final Collection<StackTraceElement[]> leaks
"Leaked " + leaks.size() + " resources (" + dupDetector.size() + " unique traces):\n");
final MutableInt i = new MutableInt();
dupDetector.entrySet().stream().limit(maxUniqueTraces).forEach(entry -> {
sb.append(" Leak #").append(i.intValue());
sb.append(" Leak #").append(i.get());
if (entry.getValue() > 0L) {
sb.append(", detected " + entry.getValue() + " times, was acquired:\n");
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
package io.deephaven.util.datastructures;

import io.deephaven.base.reference.SimpleReference;
import org.apache.commons.lang3.mutable.MutableInt;
import io.deephaven.util.mutable.MutableInt;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

Expand Down Expand Up @@ -221,7 +221,7 @@ public boolean isEmpty() {
public int size() {
final MutableInt size = new MutableInt(0);
forEach((ref, source) -> size.increment());
return size.intValue();
return size.get();
}

/**
Expand Down
68 changes: 68 additions & 0 deletions Util/src/main/java/io/deephaven/util/mutable/MutableInt.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
//
// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending
//
package io.deephaven.util.mutable;

/**
* Minimal mutable wrapper for an {@code int} value. Loosely based on
* {@code org.apache.commons.lang3.mutable.MutableInt}, but without inheriting from {@link Number}, or providing any
* overloads that accept {@code Number} or any boxed types.
* <p>
* Deliberately does not extend {@code Number}, does not implement {@code toString()}/{@code equals}/{@code hashcode()},
* or implement {@code Comparable}.
*/

public class MutableInt {
private int value;

public MutableInt() {

}

public MutableInt(final int value) {
this.value = value;
}

public int get() {
return value;
}

public void set(int value) {
this.value = value;
}

public void add(int addend) {
this.value += addend;
}

public int addAndGet(int addend) {
value += addend;
return value;
}

public int getAndAdd(int addend) {
int old = value;
value += addend;
return old;
}

public int getAndIncrement() {
return value++;
}

public void increment() {
value++;
}

public void decrement() {
value--;
}

public int incrementAndGet() {
return ++value;
}

public void subtract(final int subtrahend) {
this.value -= subtrahend;
}
}
67 changes: 67 additions & 0 deletions Util/src/main/java/io/deephaven/util/mutable/MutableLong.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
//
// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending
//
package io.deephaven.util.mutable;

/**
* Minimal mutable wrapper for a {@code long} value. Loosely based on
* {@code org.apache.commons.lang3.mutable.MutableLong}, but without inheriting from {@link Number}, or providing any
* overloads that accept {@code Number} or any boxed types.
* <p>
* Deliberately does not extend {@code Number}, does not implement {@code toString()}/{@code equals}/{@code hashcode()},
* or implement {@code Comparable}.
*/
public class MutableLong {
private long value;

public MutableLong() {

}

public MutableLong(final long value) {
this.value = value;
}

public long get() {
return value;
}

public void set(long value) {
this.value = value;
}

public void add(long addend) {
this.value += addend;
}

public long addAndGet(long addend) {
value += addend;
return value;
}

public long getAndAdd(long addend) {
long old = value;
value += addend;
return old;
}

public long getAndIncrement() {
return value++;
}

public void increment() {
value++;
}

public void decrement() {
value--;
}

public long incrementAndGet() {
return ++value;
}

public void subtract(final long subtrahend) {
this.value -= subtrahend;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
package io.deephaven.util.datastructures;

import io.deephaven.util.annotations.ReferentialIntegrity;
import io.deephaven.util.mutable.MutableInt;
import junit.framework.TestCase;
import org.apache.commons.lang3.mutable.MutableInt;
import org.jetbrains.annotations.NotNull;
import org.junit.Assume;
import org.junit.Test;
Expand Down Expand Up @@ -51,8 +51,8 @@ public void testWithFactory() {
final SegmentedSoftPool<Integer> pool = new SegmentedSoftPool<>(10,
() -> {
counter.increment();
sumAllocated.add(counter);
return counter.toInteger();
sumAllocated.add(counter.get());
return counter.get();
},
sumCleared::add);

Expand All @@ -64,7 +64,7 @@ public void testWithFactory() {

IntStream.range(0, 1000).boxed().forEach(II -> TestCase.assertEquals(II, pool.take()));
IntStream.range(0, 1000).boxed().forEach(pool::give);
TestCase.assertEquals(sumAllocated, sumCleared);
TestCase.assertEquals(sumAllocated.get(), sumCleared.get());
}

private static final BitSet OUTSTANDING_INSTANCES = new BitSet(1_000_000);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
//
package io.deephaven.util.datastructures;

import io.deephaven.base.reference.HardSimpleReference;
import io.deephaven.base.reference.SimpleReference;
import io.deephaven.util.mutable.MutableInt;
import junit.framework.TestCase;
import org.apache.commons.lang3.mutable.MutableInt;
import org.jetbrains.annotations.NotNull;
import org.junit.Test;

import java.util.Arrays;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

/**
Expand All @@ -27,81 +29,66 @@ public void testSerial() {
doTest(false);
}

private static final class IntRef extends MutableInt implements SimpleReference<MutableInt> {

private boolean cleared;

public IntRef(final int value) {
super(value);
}

@Override
public MutableInt get() {
return cleared ? null : this;
}

@Override
public void clear() {
cleared = true;
}
}

@SuppressWarnings({"NumberEquality", "PointlessArithmeticExpression"})
private void doTest(final boolean concurrent) {
final SimpleReferenceManager<MutableInt, SimpleReference<MutableInt>> SUT =
new SimpleReferenceManager<>((final MutableInt item) -> ((IntRef) item), concurrent);
final IntRef[] items = IntStream.range(0, 1000).mapToObj(IntRef::new).toArray(IntRef[]::new);
// noinspection unchecked
final SimpleReference<Integer>[] refs =
IntStream.range(0, 1000).mapToObj(HardSimpleReference::new).toArray(SimpleReference[]::new);
final SimpleReferenceManager<Integer, SimpleReference<Integer>> SUT =
new SimpleReferenceManager<>(val -> refs[val], concurrent);

Arrays.stream(items, 0, 500).forEach(SUT::add);
Arrays.stream(refs, 0, 500).map(SimpleReference::get).forEach(SUT::add);

int expectedSum = 500 * (499 + 0) / 2;
testSumExpectations(SUT, expectedSum);

Arrays.stream(items, 0, 500).forEach((final IntRef item) -> TestCase.assertSame(item,
SUT.getFirstItem((final MutableInt other) -> item == other)));
Arrays.stream(items, 0, 500).forEach((final IntRef item) -> TestCase.assertSame(item,
SUT.getFirstReference((final MutableInt other) -> item == other)));
Arrays.stream(refs, 0, 500).forEach(ref -> TestCase.assertSame(ref.get(),
SUT.getFirstItem((final Integer other) -> ref.get() == other)));
Arrays.stream(refs, 0, 500).forEach(ref -> TestCase.assertSame(ref,
SUT.getFirstReference((final Integer other) -> ref.get() == other)));

items[200].clear();
refs[200].clear();
expectedSum -= 200;
TestCase.assertSame(items[199], SUT.getFirstItem((final MutableInt other) -> items[199] == other));
TestCase.assertNull(SUT.getFirstItem((final MutableInt other) -> items[200] == other));
TestCase.assertSame(items[201], SUT.getFirstItem((final MutableInt other) -> items[201] == other));
TestCase.assertSame(refs[199].get(), SUT.getFirstItem((final Integer other) -> refs[199].get() == other));
TestCase.assertNull(SUT.getFirstItem((final Integer other) -> refs[200].get() == other));
TestCase.assertSame(refs[201].get(), SUT.getFirstItem((final Integer other) -> refs[201].get() == other));
testSumExpectations(SUT, expectedSum);

items[300].clear();
refs[300].clear();
expectedSum -= 300;
TestCase.assertSame(items[299], SUT.getFirstReference((final MutableInt other) -> items[299] == other));
TestCase.assertNull(SUT.getFirstReference((final MutableInt other) -> items[300] == other));
TestCase.assertSame(items[301], SUT.getFirstReference((final MutableInt other) -> items[301] == other));
TestCase.assertSame(refs[299], SUT.getFirstReference((final Integer other) -> refs[299].get() == other));
TestCase.assertNull(SUT.getFirstReference((final Integer other) -> refs[300].get() == other));
TestCase.assertSame(refs[301], SUT.getFirstReference((final Integer other) -> refs[301].get() == other));
testSumExpectations(SUT, expectedSum);

items[400].clear();
refs[400].clear();
expectedSum -= 400;
testSumExpectations(SUT, expectedSum);

Arrays.stream(items, 500, 1000).forEach(SUT::add);
Arrays.stream(refs, 500, 1000).map(SimpleReference::get).forEach(SUT::add);
expectedSum += 500 * (999 + 500) / 2;
testSumExpectations(SUT, expectedSum);

SUT.removeAll(Arrays.asList(Arrays.copyOfRange(items, 600, 700)));
Arrays.stream(items, 700, 800).forEach(IntRef::clear);
SUT.removeAll(Arrays.stream(Arrays.copyOfRange(refs, 600, 700)).map(SimpleReference::get)
.collect(Collectors.toList()));
Arrays.stream(refs, 700, 800).forEach(SimpleReference::clear);
expectedSum -= 200 * (799 + 600) / 2;
testSumExpectations(SUT, expectedSum);

Arrays.stream(items, 0, 100).forEach(IntRef::clear);
SUT.remove(items[0]);
Arrays.stream(refs, 0, 100).forEach(SimpleReference::clear);
SUT.remove(refs[0].get());
expectedSum -= 100 * (99 + 0) / 2;
testSumExpectations(SUT, expectedSum);
}

private void testSumExpectations(@NotNull final SimpleReferenceManager<MutableInt, SimpleReference<MutableInt>> SUT,
private void testSumExpectations(
@NotNull final SimpleReferenceManager<Integer, ? extends SimpleReference<Integer>> SUT,
final int expectedSum) {
final MutableInt sum = new MutableInt();
SUT.forEach((final SimpleReference<MutableInt> ref, final MutableInt item) -> {
TestCase.assertSame(ref, item);
SUT.forEach((ref, item) -> {
TestCase.assertSame(ref.get(), item);
sum.add(item);
});
TestCase.assertEquals(expectedSum, sum.intValue());
TestCase.assertEquals(expectedSum, sum.get());
}
}
Loading

0 comments on commit 6ebd418

Please sign in to comment.