Skip to content

refactor: DH-22508: Remove trove from engine-rowset#7971

Open
cpwright wants to merge 9 commits intodeephaven:mainfrom
cpwright:nightly/cpw/remove-trove-from-rowset
Open

refactor: DH-22508: Remove trove from engine-rowset#7971
cpwright wants to merge 9 commits intodeephaven:mainfrom
cpwright:nightly/cpw/remove-trove-from-rowset

Conversation

@cpwright
Copy link
Copy Markdown
Contributor

@cpwright cpwright commented May 1, 2026

No description provided.

niloc132 and others added 7 commits April 30, 2026 15:01
@niloc132 @cpwright
Start replacing trove with fastutil
bb1c369
@niloc132 @cpwright
Replace usages in other files, only tests remain
35f2f5e
@niloc132 @cpwright
Finally, update tests to remove trove
a8073f2
@niloc132 @cpwright
Use trove in BMWI
13b7344
@niloc132 @cpwright
Replace more primitive arraylists
180bb9f
@cpwright @claude
Templatize primitive ArrayLists on Char and move to .list package
abee844 
Replace the hand-maintained ShortArrayList/IntArrayList/LongArrayList
in io.deephaven.util.datastructures.primitives with a single Char
template under io.deephaven.util.datastructures.list, and generate
the Byte/Short/Int/Long/Float/Double variants via a new
ReplicatePrimitiveArrayLists replicator (registered in
replicateAllSafe). Add a TestCharArrayList that covers
size/add/get/set/grow/clear/removeChar/removeElements; the same
test is replicated to all six numeric primitives.

The two existing callers in engine/rowset (ExternalizableRowSetUtils,
RowSetShiftData) are updated to import from the new .list package.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cpwright
WebRowsetBuilder compile fix.
ab2eeb8
@cpwright cpwright requested a review from Copilot May 1, 2026 10:47
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026 •
edited
Loading

No docs changes detected for 1b5a3b7

@cpwright cpwright added NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed. labels May 1, 2026
Copilot AI reviewed May 1, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors rowset-related code to remove Trove usage from engine-rowset by switching to fastutil where appropriate and introducing small internal primitive array list utilities to cover remaining needs, plus updating supporting web and replication artifacts.

Changes:

  • Replace Trove collections with fastutil primitives in rowset tests and some production code (e.g., SortedRanges array pools, barrage metadata offsets).
  • Introduce io.deephaven.util.datastructures.list.*ArrayList primitive list implementations (and generated tests), plus a replication task to regenerate them.
  • Update Gradle dependencies to drop libs.trove from engine/rowset and base-test-utils in favor of libs.dsi.fastutil.

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
web/client-api/src/main/resources/io/deephaven/web/super/io/deephaven/engine/rowset/WebRowSetBuilderSequentialImpl.java Implements new appendKey for web sequential builder.
web/client-api/src/main/resources/io/deephaven/web/super/io/deephaven/engine/rowset/RowSetBuilderSequential.java Adds appendKey(long) to the web interface to match engine expectations.
replication/static/src/main/java/io/deephaven/replicators/ReplicatePrimitiveArrayLists.java Adds replication driver for primitive array list sources/tests.
replication/static/build.gradle Wires replicatePrimitiveArrayLists into replicateAllSafe.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/BarrageMessageWriterImpl.java Removes Trove list usage; uses int[] for FlatBuffers offset vector construction.
engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/sortedranges/SortedRangesTest.java Migrates tests from Trove to fastutil primitives.
engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/rsp/RspBitmapTest.java Migrates tests from Trove to fastutil primitives.
engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/WritableRowSetImplTest.java Migrates tests from Trove to fastutil primitives.
engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/ValidationSet.java Replaces Trove set utilities with fastutil equivalents.
engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/RowSetCreationRandomPerfTest.java Removes Trove import and deletes unused IntStats block.
engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/sortedranges/SortedRangesShort.java Replaces Trove TIntObjectHashMap pooling with fastutil Int2ObjectOpenHashMap.
engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/sortedranges/SortedRangesLong.java Replaces Trove pooling map with fastutil; minor signature tweaks.
engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/sortedranges/SortedRangesInt.java Replaces Trove pooling map with fastutil.
engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/RowSetUtils.java Removes Trove-based helper and its dependency.
engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/ExternalizableRowSetUtils.java Switches from Trove short list to internal ShortArrayList.
engine/rowset/src/main/java/io/deephaven/engine/rowset/RowSetShiftData.java Replaces Trove primitive lists with internal primitive array lists.
engine/rowset/build.gradle Swaps Trove dependency for fastutil in rowset module.
base-test-utils/src/main/java/io/deephaven/base/testing/Shuffle.java Switches Trove list shuffle overload to fastutil LongArrayList.
base-test-utils/build.gradle Swaps Trove dependency for fastutil in base test utils.
Util/src/test/java/io/deephaven/util/datastructures/list/TestShortArrayList.java Adds generated tests for new ShortArrayList.
Util/src/test/java/io/deephaven/util/datastructures/list/TestLongArrayList.java Adds generated tests for new LongArrayList.
Util/src/test/java/io/deephaven/util/datastructures/list/TestIntArrayList.java Adds generated tests for new IntArrayList.
Util/src/test/java/io/deephaven/util/datastructures/list/TestFloatArrayListSpecial.java Adds float NaN / signed-zero contract tests.
Util/src/test/java/io/deephaven/util/datastructures/list/TestFloatArrayList.java Adds generated tests for new FloatArrayList.
Util/src/test/java/io/deephaven/util/datastructures/list/TestDoubleArrayListSpecial.java Adds double NaN / signed-zero contract tests.
Util/src/test/java/io/deephaven/util/datastructures/list/TestDoubleArrayList.java Adds generated tests for new DoubleArrayList.
Util/src/test/java/io/deephaven/util/datastructures/list/TestCharArrayList.java Adds tests for CharArrayList.
Util/src/test/java/io/deephaven/util/datastructures/list/TestByteArrayList.java Adds generated tests for new ByteArrayList.
Util/src/main/java/io/deephaven/util/datastructures/list/ShortArrayList.java Adds internal primitive short list implementation.
Util/src/main/java/io/deephaven/util/datastructures/list/LongArrayList.java Adds internal primitive long list implementation.
Util/src/main/java/io/deephaven/util/datastructures/list/IntArrayList.java Adds internal primitive int list implementation.
Util/src/main/java/io/deephaven/util/datastructures/list/FloatArrayList.java Adds internal primitive float list implementation.
Util/src/main/java/io/deephaven/util/datastructures/list/DoubleArrayList.java Adds internal primitive double list implementation.
Util/src/main/java/io/deephaven/util/datastructures/list/CharArrayList.java Adds internal primitive char list implementation (replication template).
Util/src/main/java/io/deephaven/util/datastructures/list/ByteArrayList.java Adds internal primitive byte list implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/ValidationSet.java Outdated
Comment thread engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/ValidationSet.java
@cpwright cpwright requested review from Copilot and niloc132 May 1, 2026 12:54
Copilot AI reviewed May 1, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors rowset-related code to eliminate Trove usage, primarily by switching to fastutil (and a small in-house primitive list set) across engine-rowset and associated tests/utilities.

Changes:

  • Replace Trove primitive collections with fastutil equivalents (and plain arrays where appropriate) across engine-rowset and tests.
  • Add replicated primitive *ArrayList implementations under io.deephaven.util.datastructures.list plus generated unit tests and a replication task.
  • Align the web client RowSetBuilderSequential surface with the engine API by adding appendKey.

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
web/client-api/src/main/resources/io/deephaven/web/super/io/deephaven/engine/rowset/WebRowSetBuilderSequentialImpl.java Implements new appendKey for web sequential builder.
web/client-api/src/main/resources/io/deephaven/web/super/io/deephaven/engine/rowset/RowSetBuilderSequential.java Adds appendKey to the web-facing sequential builder interface.
replication/static/src/main/java/io/deephaven/replicators/ReplicatePrimitiveArrayLists.java New replicator entrypoint for generating primitive array list classes/tests.
replication/static/build.gradle Registers the new primitive array list replication task.
extensions/barrage/src/main/java/io/deephaven/extensions/barrage/BarrageMessageWriterImpl.java Removes Trove TIntArrayList usage by switching to an int[] buffer.
engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/sortedranges/SortedRangesTest.java Migrates Trove usage in sorted-ranges tests to fastutil.
engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/rsp/RspBitmapTest.java Migrates Trove usage in RSP bitmap tests to fastutil.
engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/WritableRowSetImplTest.java Migrates Trove usage in rowset implementation tests to fastutil.
engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/ValidationSet.java Replaces Trove-based validation set helpers with fastutil-based helpers.
engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/RowSetCreationRandomPerfTest.java Removes Trove dependency from perf test utilities.
engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/sortedranges/SortedRangesShort.java Replaces Trove maps with fastutil maps for array pooling.
engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/sortedranges/SortedRangesLong.java Replaces Trove maps with fastutil maps for array pooling.
engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/sortedranges/SortedRangesInt.java Replaces Trove maps with fastutil maps for array pooling.
engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/RowSetUtils.java Removes Trove-backed helper logic no longer needed.
engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/ExternalizableRowSetUtils.java Switches short buffering from Trove to in-house ShortArrayList.
engine/rowset/src/main/java/io/deephaven/engine/rowset/RowSetShiftData.java Switches shift payload/index storage from Trove to in-house primitive lists.
engine/rowset/build.gradle Replaces Trove dependency with fastutil.
base-test-utils/src/main/java/io/deephaven/base/testing/Shuffle.java Switches shuffle helper from Trove to fastutil list type.
base-test-utils/build.gradle Replaces Trove dependency with fastutil.
Util/src/test/java/io/deephaven/util/datastructures/list/TestShortArrayList.java New generated tests for ShortArrayList.
Util/src/test/java/io/deephaven/util/datastructures/list/TestLongArrayList.java New generated tests for LongArrayList.
Util/src/test/java/io/deephaven/util/datastructures/list/TestIntArrayList.java New generated tests for IntArrayList.
Util/src/test/java/io/deephaven/util/datastructures/list/TestFloatArrayListSpecial.java New tests covering float NaN / signed-zero behaviors.
Util/src/test/java/io/deephaven/util/datastructures/list/TestFloatArrayList.java New generated tests for FloatArrayList.
Util/src/test/java/io/deephaven/util/datastructures/list/TestDoubleArrayListSpecial.java New tests covering double NaN / signed-zero behaviors.
Util/src/test/java/io/deephaven/util/datastructures/list/TestDoubleArrayList.java New generated tests for DoubleArrayList.
Util/src/test/java/io/deephaven/util/datastructures/list/TestCharArrayList.java New tests for CharArrayList (replication source).
Util/src/test/java/io/deephaven/util/datastructures/list/TestByteArrayList.java New generated tests for ByteArrayList.
Util/src/main/java/io/deephaven/util/datastructures/list/ShortArrayList.java Adds in-house primitive ShortArrayList.
Util/src/main/java/io/deephaven/util/datastructures/list/LongArrayList.java Adds in-house primitive LongArrayList.
Util/src/main/java/io/deephaven/util/datastructures/list/IntArrayList.java Adds in-house primitive IntArrayList.
Util/src/main/java/io/deephaven/util/datastructures/list/FloatArrayList.java Adds in-house primitive FloatArrayList.
Util/src/main/java/io/deephaven/util/datastructures/list/DoubleArrayList.java Adds in-house primitive DoubleArrayList.
Util/src/main/java/io/deephaven/util/datastructures/list/CharArrayList.java Adds in-house primitive CharArrayList (replication source).
Util/src/main/java/io/deephaven/util/datastructures/list/ByteArrayList.java Adds in-house primitive ByteArrayList.
Comments suppressed due to low confidence (5)

engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/sortedranges/SortedRangesTest.java:1004

  • LongOpenHashSet doesn't support Trove-style toArray(long[]) in many fastutil versions; this is likely to fail to compile now that set is a fastutil type. Prefer set.toLongArray() (or the fastutil equivalent) rather than allocating an array and calling set.toArray(arr).
        final LongOpenHashSet set = ValidationSet.make(count);
        final String pfxMsg = "doTestGetFind seed == " + seed;
        System.out.println(pfxMsg);
        final SortedRanges sar = populateRandom(
                pfxMsg, new Random(seed), count, 10, 3000000, 150, 50,
                set, true);
        assertEquals(set.size(), sar.getCardinality());
        final long[] arr = new long[set.size()];
        set.toArray(arr);

engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/rsp/RspBitmapTest.java:954

  • LongOpenHashSet doesn't support Trove-style toArray(long[]) in many fastutil versions; this is likely to fail to compile now that set is a fastutil type. Use set.toLongArray() (or the fastutil equivalent) instead of allocating an array and calling toArray.
        final LongOpenHashSet set = ValidationSet.make(count);
        final String pfxMsg = "doTestSearchIteratorSingle seed == " + seed;
        final Random r = new Random(seed);
        final RspBitmap rb = populateRandom(pfxMsg, r, count, 1, 10000, 150, 50, set, true);
        assertEquals(set.size(), rb.getCardinality());
        final long[] arr = new long[set.size()];
        set.toArray(arr);

engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/rsp/RspBitmapTest.java:1089

  • LongOpenHashSet doesn't support Trove-style toArray(long[]) in many fastutil versions; this is likely to fail to compile now that set is a fastutil type. Prefer set.toLongArray() (or the fastutil equivalent) rather than set.toArray(arr).
        final LongOpenHashSet set = ValidationSet.make(count);
        final String pfxMsg = "doTestGetFind2 seed == " + seed;
        final RspBitmap rb = populateRandom(pfxMsg, new Random(seed), count, 10, 30000000, 150, 50, set, true);
        assertEquals(set.size(), rb.getCardinality());
        rb.validate();
        final long[] arr = new long[set.size()];
        set.toArray(arr);

engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/rsp/RspBitmapTest.java:1392

  • LongOpenHashSet doesn't support Trove-style toArray(long[]) in many fastutil versions; this is likely to fail to compile now that set is a fastutil type. Prefer set.toLongArray() (or the fastutil equivalent) rather than set.toArray(arr).
        final LongOpenHashSet set = ValidationSet.make(count);
        final Random r = new Random(seed);
        final String pfxMsg = "doTestFindRange seed == " + seed;
        final RspBitmap rb = populateRandom(pfxMsg, r, count, 10, 30000000, 150, 50, set, true);
        assertEquals(set.size(), rb.getCardinality());
        final long[] arr = new long[set.size()];
        set.toArray(arr);

engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/rsp/RspBitmapTest.java:1572

  • LongOpenHashSet doesn't support Trove-style toArray(long[]) in many fastutil versions; this is likely to fail to compile now that set is a fastutil type. Prefer set.toLongArray() (or the fastutil equivalent) rather than set.toArray(arr).
        final LongOpenHashSet set = ValidationSet.make(count);
        final String pfxMsg = "doTestGetRange seed == " + seed;
        final Random r = new Random(seed);
        final RspBitmap rb = populateRandom(pfxMsg, r, count, 10, 30000000, 150, 50, set, true);
        assertEquals(set.size(), rb.getCardinality());
        final long[] arr = new long[set.size()];
        set.toArray(arr);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread engine/rowset/src/test/java/io/deephaven/engine/rowset/impl/ValidationSet.java Outdated
@cpwright
Potential fix for pull request finding
1b5a3b7 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@niloc132 niloc132 Awaiting requested review from niloc132

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cpwright @niloc132