Skip to content

Optimize collection iteration with shard cursors - #854

Open
DanielThomas wants to merge 6 commits into
masterfrom
dannyt/immutable-shard-iterators
Open

DanielThomas wants to merge 6 commits into
masterfrom
dannyt/immutable-shard-iterators

Conversation

@DanielThomas

@DanielThomas DanielThomas commented Sep 21, 2026 •

Copy link
Copy Markdown
Member

This picks up Michael Braun's work from #842, preserving his authorship, and reconciles it with the read-path invariants introduced by the preceding PRs.

When enabled, it adds snapshot iterators for list, set, and map reads. A snapshot iterator captures its holder, shard, and collection bounds once, then reads subsequent elements or buckets directly from that shard instead of resolving them for every operation:

HollowConsumer.newHollowConsumer()
        .withSnapshotCollectionIterators()
        // ...
        .build();

This also means that #345 is no longer a problem.

Benchmarks

Representative JMH measurements on JDK 25 with ZGC on this x86-64 host, traversing 2,000 lists in ordinal order per operation:

Immutable shards (WastefulRecycler)

Elements per list Existing iterator Snapshot iterator Improvement
4 92.3 us 28.8 us 68.8%
16 313.0 us 50.9 us 83.7%

Recycling shards

Elements per list Existing iterator Snapshot iterator Improvement
4 179.8 us 47.0 us 73.9%
16 469.7 us 117.5 us 75.0%

The set and map benchmarks also improved on this host, though the recycling cases varied more.

@DanielThomas
DanielThomas force-pushed the dannyt/immutable-shard-iterators branch from 85a3df2 to 5d705e4 Compare September 21, 2026 22:56
@DanielThomas DanielThomas changed the title Optimize iteration over immutable shards Optimize collection iteration with shard cursors Sep 21, 2026
@DanielThomas
DanielThomas force-pushed the dannyt/immutable-shard-iterators branch 2 times, most recently from e34d514 to 6f8658e Compare September 23, 2026 02:59
@DanielThomas
DanielThomas changed the base branch from dannyt/non-recycling-read-fast-path to dannyt/contiguous-fixed-length-data September 23, 2026 02:59
@DanielThomas
DanielThomas added this pull request to stack #857 September 23, 2026 03:02
@DanielThomas
DanielThomas force-pushed the dannyt/immutable-shard-iterators branch from 6f8658e to 8bbd80d Compare September 23, 2026 03:04
@DanielThomas
DanielThomas marked this pull request as ready for review September 23, 2026 03:54
@DanielThomas
DanielThomas force-pushed the dannyt/immutable-shard-iterators branch from 8bbd80d to 95970e1 Compare September 23, 2026 04:14
@DanielThomas
DanielThomas force-pushed the dannyt/immutable-shard-iterators branch from 95970e1 to f421028 Compare September 23, 2026 06:41
@DanielThomas
DanielThomas force-pushed the dannyt/immutable-shard-iterators branch from f421028 to 0219f34 Compare September 26, 2026 02:20
Comment thread hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java
Comment thread hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java Outdated
@DanielThomas

Copy link
Copy Markdown
Member Author

List iteration, bidirectional iteration and spliteration now share the captured ordinal read logic. Splits have independent validation state. Map and set iterators capture size and bucket bounds together, and empty Java list traversal uses the JDK empty adapters.

Added coverage for cached and proxy-backed lists, refreshes, parallel resharding and empty traversal.

@DanielThomas
DanielThomas force-pushed the dannyt/immutable-shard-iterators branch from 20b602a to 000050e Compare October 5, 2026 04:49
@joegoogle123
joegoogle123 force-pushed the dannyt/immutable-shard-iterators branch from 000050e to caa71cf Compare October 5, 2026 14:02
@Sunjeet

Sunjeet commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator
  • equals() and hashCode() stop agreeing. list.equals(expected) is true, but the hash codes differ. Any HashMap or HashSet keyed on such lists would miss lookups.
  • spliterator() only honors the override in HollowPerfBackedList. HollowList.spliterator() falls back to super.spliterator(), which the JDK builds on iterator(), so streams skip the override too.

Suggested fix for the PR: make HollowList.iterator() use delegate.ordinalCursor(ordinal) and fall back to super.iterator() when it returns null, the same way listIterator() does. Also add the getClass() check to HollowPerfBackedList.iterator(). Streams, copies, toString() and hashCode() all go through iterator(), so that one change fixes all of them. Extending lookupSubclassRetainsItsGetBasedListIterator to cover iterator() and stream() would catch it.

@DanielThomas

Copy link
Copy Markdown
Member Author

@Sunjeet Fixed both iterator paths as suggested, so they preserve custom get() behavior. I also extended the tests to cover iteration, streams, copies, toString(), equality and hash codes.

@DanielThomas
DanielThomas force-pushed the dannyt/immutable-shard-iterators branch 2 times, most recently from 04bae6a to c87275d Compare October 7, 2026 02:48
@DanielThomas

Copy link
Copy Markdown
Member Author

Wired the iterator opt-in through SHARD_CURSOR_ITERATORS. Kept withShardCursorIterators() as an alias; traversal remains unchanged by default.

@DanielThomas
DanielThomas force-pushed the dannyt/immutable-shard-iterators branch from c87275d to 617c0b0 Compare October 7, 2026 02:54
Base automatically changed from dannyt/contiguous-fixed-length-data to master October 7, 2026 03:02
michaelbraun and others added 6 commits October 7, 2026 14:02
Collection iteration previously resolved the shard and collection bounds for every element or bucket and performed two trailing read validations. Snapshot iterators capture the shard and bounds once, then validate each surfaced element against the captured shard.

For mutable on-heap states, a concurrent update or reshard causes the current read to be discarded and the snapshot to be reacquired before iteration continues. Concrete read states use the snapshot implementations while proxy, missing, and historical data access retain the generic iterators.

Add correctness and concurrent resharding tests for list, set, and map iteration, plus JMH benchmarks.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Retain the existing iterator implementation types while using single-object shard snapshot iterators. Preserve sampling and empty collection behavior, and make immutable shard validation a no-op.

Add HollowConsumer.Builder.withSnapshotCollectionIterators() as an experimental opt-in. Existing collection iterators remain the default.

Route HollowList iteration through delegates so lookup-backed lists use the selected ordinal iterator while cached lists retain their cached contents. Extend the benchmarks to cover recycling and wasteful allocators.
Use the delegate cursor opt-out for forward HollowList iteration and retain get-based traversal for perf-list subclasses. Remove the now-unnecessary Java 8 spliterator workaround. Cover streams, copies, equality and hashing for custom lookup/cached delegates and perf-list subclasses.
@DanielThomas
DanielThomas force-pushed the dannyt/immutable-shard-iterators branch from 617c0b0 to d8beb2e Compare October 7, 2026 03:02

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants