perf: improve ConstraintCollectors.toList()

[triceo]

The underlying collection is now optimized for removals, as those operations will be frequent inside group nodes. It still retains fast random access to elements, as that is required to reuse it in Neighborhoods.

The collection supports null values, and implements the full list interface; this makes it a suitable replacement to ArrayList.

Compared to ArrayList, remove() is super-fast if done via the entry. This introduces small overhead in addition and iteration, as the arrays may need to be compacted. Some operations compare equally poorly in both collections, such as additions into the middle of the array.

Note: this was co-developed with Claude code. It was reviewed by 2 different LLMs, as well as by me.

[Copilot]

Pull request overview

This PR refactors ElementAwareArrayList into an AbstractList-backed implementation optimized for frequent O(1) removals via entry handles, and updates constraint-stream/indexing code to use the new entry-based API (notably for ConstraintCollectors.toList()/group node workloads).

Changes:

• Reimplemented ElementAwareArrayList using flat arrays plus index maps, adding partial/full compaction paths and ListIterator support.
• Switched several hot-path components (collectors, indexer backend, neighborhood dataset) to use addEntry(...)/Entry.remove() for O(1) undo/removal.
• Expanded and reorganized tests for compaction, null handling, iterator behavior, and fail-fast semantics; clarified collector docs about mutability.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
core/src/main/java/ai/timefold/solver/core/impl/util/ElementAwareArrayList.java	Major rewrite to an AbstractList implementation with entry-based O(1) removal and compaction logic.
core/src/test/java/ai/timefold/solver/core/impl/util/ElementAwareArrayListTest.java	Large test suite expansion to validate new compaction/iterator/null/entry semantics.
core/src/main/java/ai/timefold/solver/core/impl/score/stream/collector/ListUndoableActionable.java	Uses ElementAwareArrayList entries to make undo removal O(1).
core/src/main/java/ai/timefold/solver/core/impl/neighborhood/stream/enumerating/common/AbstractLeftDatasetInstance.java	Stores entry handles and removes via Entry.remove() on retract.
core/src/main/java/ai/timefold/solver/core/impl/bavet/common/index/RandomAccessIndexerBackend.java	Uses addEntry(...) and removes via Entry.remove() instead of list-level removal.
core/src/main/java/ai/timefold/solver/core/impl/bavet/common/index/DefaultUniqueRandomIterator.java	Adjusted to ElementAwareArrayList<T> element-based get(int) semantics.
core/src/main/java/ai/timefold/solver/core/api/score/stream/ConstraintCollectors.java	Documentation updates warning users not to mutate returned collections.

[triceo]

Converting back to a draft for another experiment.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

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

[Christopher-Chianelli]

Changes look good upon initial review. Any benchmarks?

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

[Christopher-Chianelli]

LGTM.

[Christopher-Chianelli]

It seems a comment I made got loss, but in forEachCompacting, I am pretty sure the second if is impossible since the first if is always taken if the second if condition was true.

[Christopher-Chianelli]

(line 261, GitHub not allowing me to comment)

[Christopher-Chianelli]

Also, does remove and add preserving the original list order matter? If we only care about random access (but not the indicies), there a neat trick we can do: when an element is removed, swap it with the tail of the list. This invalidates only the position of the tail and eliminates the need for compacting, but it no longer folllows proper list semantics. Since inserting at a particular index does not really make sense since indices can be invalidated, you can only add to the end.

l = [1, 2, 3, 4]
l.remove(2)
// l = [1, 4, 3]
l.remove(1)
// l = [3, 4]

[triceo]

It seems a comment I made got loss, but in forEachCompacting, I am pretty sure the second if is impossible since the first if is always taken if the second if condition was true.

That method (clearIfPossible) is weirdly named. It does clear the list if possible, but it returns true if the list is compact - which means either empty, or non-empty but with no gaps. Which is why the second condition still makes sense. I refactored it slightly now to make the code easier to understand.

Also, does remove and add preserving the original list order matter? If we only care about random access (but not the indicies), there a neat trick we can do: when an element is removed, swap it with the tail of the list.

I originally had it implemented like that, but then I decided that insertion order is important. You expect that out of a list.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
91.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud