From 32608145610e735753777486b9e06e26d6d37ea1 Mon Sep 17 00:00:00 2001 From: Michael Braun Date: Thu, 16 Jul 2026 14:20:11 -0400 Subject: [PATCH 1/6] Zero-allocation snapshot iterators for list/set/map read paths 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) --- hollow-perf/build.gradle | 9 + ...owListTypeReadStateIterationBenchmark.java | 115 ++++++++++++ ...lowMapTypeReadStateIterationBenchmark.java | 113 ++++++++++++ ...lowSetTypeReadStateIterationBenchmark.java | 113 ++++++++++++ .../HollowListSnapshotOrdinalIterator.java | 111 ++++++++++++ .../engine/list/HollowListTypeReadState.java | 5 +- ...HollowMapSnapshotEntryOrdinalIterator.java | 133 ++++++++++++++ .../engine/map/HollowMapTypeReadState.java | 5 +- .../set/HollowSetSnapshotOrdinalIterator.java | 115 ++++++++++++ .../engine/set/HollowSetTypeReadState.java | 5 +- .../core/write/copy/HollowMapCopier.java | 4 +- .../core/write/copy/HollowSetCopier.java | 4 +- ...owMapSnapshotEntryOrdinalIteratorTest.java | 116 ++++++++++++ .../HollowSetSnapshotOrdinalIteratorTest.java | 133 ++++++++++++++ ...HollowListSnapshotOrdinalIteratorTest.java | 167 ++++++++++++++++++ 15 files changed, 1135 insertions(+), 13 deletions(-) create mode 100644 hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateIterationBenchmark.java create mode 100644 hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadStateIterationBenchmark.java create mode 100644 hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadStateIterationBenchmark.java create mode 100644 hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListSnapshotOrdinalIterator.java create mode 100644 hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIterator.java create mode 100644 hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIterator.java create mode 100644 hollow/src/test/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIteratorTest.java create mode 100644 hollow/src/test/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIteratorTest.java create mode 100644 hollow/src/test/java/com/netflix/hollow/core/read/iterator/HollowListSnapshotOrdinalIteratorTest.java diff --git a/hollow-perf/build.gradle b/hollow-perf/build.gradle index 4000799121..f36184d37f 100644 --- a/hollow-perf/build.gradle +++ b/hollow-perf/build.gradle @@ -17,4 +17,13 @@ tasks.withType(JavaCompile).configureEach { jmh { duplicateClassesStrategy = DuplicatesStrategy.WARN + if (project.hasProperty('jmhInclude')) { + includes = [ project.property('jmhInclude') ] + } + if (project.hasProperty('jmhJvm')) { + jvm = project.property('jmhJvm') + } + if (project.hasProperty('jmhProf')) { + profilers = [ project.property('jmhProf') ] + } } diff --git a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateIterationBenchmark.java b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateIterationBenchmark.java new file mode 100644 index 0000000000..7f6dfa5ecf --- /dev/null +++ b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateIterationBenchmark.java @@ -0,0 +1,115 @@ +package com.netflix.hollow.core.read.engine.list; + +import com.netflix.hollow.core.read.engine.HollowReadStateEngine; +import com.netflix.hollow.core.read.engine.HollowTypeReadState; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.core.util.StateEngineRoundTripper; +import com.netflix.hollow.core.write.HollowWriteStateEngine; +import com.netflix.hollow.core.write.objectmapper.HollowObjectMapper; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +/** + * Compares list iteration cost of the per-element read path (the historical {@code HollowListOrdinalIterator} + * pattern: {@code size()} + {@code getElementOrdinal(ordinal, i)} per element, i.e. 2N+1 {@code Unsafe.loadFence()}s) + * against the single-snapshot path ({@code ordinalIterator()} backed by + * {@code HollowListTypeReadState.readElementOrdinals}, i.e. 2 fences for the whole list). + * + * The gap should widen with list size, since the per-element path pays two fences per element while the snapshot + * path pays two fences total. + */ +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.MICROSECONDS) +@Warmup(iterations = 5, time = 1) +@Measurement(iterations = 6, time = 1) +@Fork(1) +public class HollowListTypeReadStateIterationBenchmark { + + static class ListHolder { + List vals; + ListHolder(List vals) { this.vals = vals; } + } + + @Param({ "1", "4", "16", "64", "256" }) + int listSize; + + @Param({ "2000" }) + int numLists; + + HollowListTypeReadState listTypeState; + int[] ordinals; + + @Setup + public void setUp() throws IOException { + HollowWriteStateEngine writeStateEngine = new HollowWriteStateEngine(); + HollowObjectMapper objectMapper = new HollowObjectMapper(writeStateEngine); + objectMapper.initializeTypeState(ListHolder.class); + + for (int i = 0; i < numLists; i++) { + List vals = new ArrayList<>(listSize); + for (int j = 0; j < listSize; j++) + vals.add(i + j); // distinct lists so they aren't deduped to one ordinal + objectMapper.add(new ListHolder(vals)); + } + + HollowReadStateEngine readStateEngine = new HollowReadStateEngine(); + StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); + + listTypeState = null; + for (HollowTypeReadState ts : readStateEngine.getTypeStates()) { + if (ts instanceof HollowListTypeReadState) { + listTypeState = (HollowListTypeReadState) ts; + break; + } + } + if (listTypeState == null) + throw new IllegalStateException("no list type state found"); + + int max = listTypeState.maxOrdinal(); + List populated = new ArrayList<>(); + for (int o = 0; o <= max; o++) + populated.add(o); + ordinals = new int[populated.size()]; + for (int i = 0; i < ordinals.length; i++) + ordinals[i] = populated.get(i); + } + + /** New path: one validated snapshot per list (2 fences), served from the materialized iterator. */ + @Benchmark + public void iterateSnapshot(Blackhole bh) { + for (int ordinal : ordinals) { + HollowOrdinalIterator iter = listTypeState.ordinalIterator(ordinal); + int o = iter.next(); + while (o != HollowOrdinalIterator.NO_MORE_ORDINALS) { + bh.consume(o); + o = iter.next(); + } + } + } + + /** Old path: size() then getElementOrdinal(ordinal, i) per element (2N+1 fences per list). */ + @Benchmark + public void iteratePerElement(Blackhole bh) { + for (int ordinal : ordinals) { + int size = listTypeState.size(ordinal); + for (int i = 0; i < size; i++) { + bh.consume(listTypeState.getElementOrdinal(ordinal, i)); + } + } + } +} diff --git a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadStateIterationBenchmark.java b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadStateIterationBenchmark.java new file mode 100644 index 0000000000..a821119d24 --- /dev/null +++ b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadStateIterationBenchmark.java @@ -0,0 +1,113 @@ +package com.netflix.hollow.core.read.engine.map; + +import com.netflix.hollow.core.memory.encoding.HashCodes; +import com.netflix.hollow.core.read.engine.HollowReadStateEngine; +import com.netflix.hollow.core.read.engine.HollowTypeReadState; +import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIterator; +import com.netflix.hollow.core.util.StateEngineRoundTripper; +import com.netflix.hollow.core.write.HollowWriteStateEngine; +import com.netflix.hollow.core.write.objectmapper.HollowObjectMapper; +import java.io.IOException; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +/** + * Compares map entry iteration cost of the per-bucket read path (the historical + * {@code HollowMapEntryOrdinalIteratorImpl} pattern: {@code size()} + {@code relativeBucket(ordinal, bucket)} + * over every hash bucket, i.e. two fences per bucket) against the single-snapshot entry iterator + * ({@code ordinalIterator()}, one fence per surfaced entry). + */ +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.MICROSECONDS) +@Warmup(iterations = 5, time = 1) +@Measurement(iterations = 6, time = 1) +@Fork(1) +public class HollowMapTypeReadStateIterationBenchmark { + + static class MapHolder { + Map vals; + MapHolder(Map vals) { this.vals = vals; } + } + + @Param({ "1", "4", "16", "64", "256" }) + int mapSize; + + @Param({ "2000" }) + int numMaps; + + HollowMapTypeReadState mapTypeState; + int[] ordinals; + + @Setup + public void setUp() throws IOException { + HollowWriteStateEngine writeStateEngine = new HollowWriteStateEngine(); + HollowObjectMapper objectMapper = new HollowObjectMapper(writeStateEngine); + objectMapper.initializeTypeState(MapHolder.class); + + for (int i = 0; i < numMaps; i++) { + Map vals = new LinkedHashMap<>(); + for (int j = 0; j < mapSize; j++) + vals.put(i + j, i + j + 1); // distinct maps so they aren't deduped to one ordinal + objectMapper.add(new MapHolder(vals)); + } + + HollowReadStateEngine readStateEngine = new HollowReadStateEngine(); + StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); + + mapTypeState = null; + for (HollowTypeReadState ts : readStateEngine.getTypeStates()) { + if (ts instanceof HollowMapTypeReadState) { + mapTypeState = (HollowMapTypeReadState) ts; + break; + } + } + if (mapTypeState == null) + throw new IllegalStateException("no map type state found"); + + int max = mapTypeState.maxOrdinal(); + ordinals = new int[max + 1]; + for (int o = 0; o <= max; o++) + ordinals[o] = o; + } + + /** New path: one validated snapshot per map, one fence per surfaced entry. */ + @Benchmark + public void iterateSnapshot(Blackhole bh) { + for (int ordinal : ordinals) { + HollowMapEntryOrdinalIterator iter = mapTypeState.ordinalIterator(ordinal); + while (iter.next()) { + bh.consume(iter.getKey()); + bh.consume(iter.getValue()); + } + } + } + + /** Old path: size() then relativeBucket(ordinal, bucket) over every hash bucket (2 fences per bucket). */ + @Benchmark + public void iteratePerBucket(Blackhole bh) { + for (int ordinal : ordinals) { + int numBuckets = HashCodes.hashTableSize(mapTypeState.size(ordinal)); + for (int b = 0; b < numBuckets; b++) { + long bucketVal = mapTypeState.relativeBucket(ordinal, b); + if (bucketVal != -1L) { + bh.consume((int) (bucketVal >>> 32)); + bh.consume((int) bucketVal); + } + } + } + } +} diff --git a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadStateIterationBenchmark.java b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadStateIterationBenchmark.java new file mode 100644 index 0000000000..567f8bcff9 --- /dev/null +++ b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadStateIterationBenchmark.java @@ -0,0 +1,113 @@ +package com.netflix.hollow.core.read.engine.set; + +import static com.netflix.hollow.core.HollowConstants.ORDINAL_NONE; + +import com.netflix.hollow.core.memory.encoding.HashCodes; +import com.netflix.hollow.core.read.engine.HollowReadStateEngine; +import com.netflix.hollow.core.read.engine.HollowTypeReadState; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.core.util.StateEngineRoundTripper; +import com.netflix.hollow.core.write.HollowWriteStateEngine; +import com.netflix.hollow.core.write.objectmapper.HollowObjectMapper; +import java.io.IOException; +import java.util.LinkedHashSet; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +/** + * Compares set iteration cost of the per-bucket read path (the historical {@code HollowSetOrdinalIterator} + * pattern: {@code size()} + {@code relativeBucketValue(ordinal, bucket)} over every hash bucket, i.e. two fences + * per bucket) against the single-snapshot iterator ({@code ordinalIterator()}, one fence per surfaced element). + */ +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.MICROSECONDS) +@Warmup(iterations = 5, time = 1) +@Measurement(iterations = 6, time = 1) +@Fork(1) +public class HollowSetTypeReadStateIterationBenchmark { + + static class SetHolder { + Set vals; + SetHolder(Set vals) { this.vals = vals; } + } + + @Param({ "1", "4", "16", "64", "256" }) + int setSize; + + @Param({ "2000" }) + int numSets; + + HollowSetTypeReadState setTypeState; + int[] ordinals; + + @Setup + public void setUp() throws IOException { + HollowWriteStateEngine writeStateEngine = new HollowWriteStateEngine(); + HollowObjectMapper objectMapper = new HollowObjectMapper(writeStateEngine); + objectMapper.initializeTypeState(SetHolder.class); + + for (int i = 0; i < numSets; i++) { + Set vals = new LinkedHashSet<>(); + for (int j = 0; j < setSize; j++) + vals.add(i + j); // distinct sets so they aren't deduped to one ordinal + objectMapper.add(new SetHolder(vals)); + } + + HollowReadStateEngine readStateEngine = new HollowReadStateEngine(); + StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); + + setTypeState = null; + for (HollowTypeReadState ts : readStateEngine.getTypeStates()) { + if (ts instanceof HollowSetTypeReadState) { + setTypeState = (HollowSetTypeReadState) ts; + break; + } + } + if (setTypeState == null) + throw new IllegalStateException("no set type state found"); + + int max = setTypeState.maxOrdinal(); + ordinals = new int[max + 1]; + for (int o = 0; o <= max; o++) + ordinals[o] = o; + } + + /** New path: one validated snapshot per set, one fence per surfaced element. */ + @Benchmark + public void iterateSnapshot(Blackhole bh) { + for (int ordinal : ordinals) { + HollowOrdinalIterator iter = setTypeState.ordinalIterator(ordinal); + int o = iter.next(); + while (o != HollowOrdinalIterator.NO_MORE_ORDINALS) { + bh.consume(o); + o = iter.next(); + } + } + } + + /** Old path: size() then relativeBucketValue(ordinal, bucket) over every hash bucket (2 fences per bucket). */ + @Benchmark + public void iteratePerBucket(Blackhole bh) { + for (int ordinal : ordinals) { + int numBuckets = HashCodes.hashTableSize(setTypeState.size(ordinal)); + for (int b = 0; b < numBuckets; b++) { + int v = setTypeState.relativeBucketValue(ordinal, b); + if (v != ORDINAL_NONE) + bh.consume(v); + } + } + } +} diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListSnapshotOrdinalIterator.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListSnapshotOrdinalIterator.java new file mode 100644 index 0000000000..d3c106f15b --- /dev/null +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListSnapshotOrdinalIterator.java @@ -0,0 +1,111 @@ +/* + * Copyright 2016-2019 Netflix, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +package com.netflix.hollow.core.read.engine.list; + +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; + +/** + * An ordinal iterator over a list record backed by a single validated snapshot of its shard and bounds. + *

+ * The historical per-element pattern re-reads {@code shardsVolatile} and executes {@code Unsafe.loadFence()} + * twice for every element (the inner loop validating {@code start}/{@code end} plus the trailing + * validation of the element read), i.e. {@code 2N+1} fences for a list of size N including the initial + * {@code size()}. This iterator instead validates the bounds once at construction and then executes a single + * fence per {@link #next()}, i.e. {@code N+1} fences, while allocating nothing per element. + *

+ * Why holding the snapshot across {@code next()} calls is safe. The bounds {@code startElement}/ + * {@code endElement} are validated non-torn before use, so every {@code listIndex < size} maps to a bit offset + * within the shard's element region — an in-bounds, non-throwing read on-heap and within the buffer extent in + * shared-memory mode. A concurrent delta or re-shard can only (a) publish a new shard (the captured reference is + * untouched) and (b) later recycle the old shard's backing arrays, which are pooled but never truncated or + * nulled — so a stale read yields garbage of the correct length, never an out-of-bounds access. The captured + * {@code dataElements} reference also keeps that backing storage reachable for the life of the iterator. Each + * {@link #next()} re-validates shard identity after reading (the {@code loadFence} in + * {@link HollowListTypeReadState#readWasUnsafe} orders the element read before the re-read of + * {@code shardsVolatile}); if the shard changed, the value read is discarded and the snapshot is re-established + * against the newly observed version before the value is returned. Thus no torn value ever escapes, matching the + * per-element validation semantics of the original iterator without its redundant second fence. + */ +class HollowListSnapshotOrdinalIterator implements HollowOrdinalIterator { + + private final HollowListTypeReadState readState; + private final int ordinal; + + private HollowListTypeShardsHolder shardsHolder; + private HollowListTypeReadStateShard shard; + private long startElement; + private long endElement; + private int size; + + private int index; + + HollowListSnapshotOrdinalIterator(int ordinal, HollowListTypeReadState readState) { + this.readState = readState; + this.ordinal = ordinal; + snapshot(); + } + + /** + * Establish a validated snapshot: a shard whose {@code startElement}/{@code endElement} for this ordinal were + * read while the shard was current (hence non-torn). Mirrors the inner validation loop of + * {@link HollowListTypeReadState#getElementOrdinal(int, int)}. + */ + private void snapshot() { + HollowListTypeShardsHolder holder; + HollowListTypeReadStateShard s; + long start; + long end; + do { + holder = readState.shardsVolatile; + s = holder.shards[ordinal & holder.shardNumberMask]; + int shardOrdinal = ordinal >> s.shardOrdinalShift; + + start = s.dataElements.getStartElement(shardOrdinal); + end = s.dataElements.getEndElement(shardOrdinal); + } while(readState.readWasUnsafe(holder, ordinal, s)); + + this.shardsHolder = holder; + this.shard = s; + this.startElement = start; + this.endElement = end; + this.size = (int)(end - start); + } + + @Override + public int next() { + if(index >= size) + return NO_MORE_ORDINALS; + + int listIndex = index; + while(true) { + // Bounds were validated in snapshot(), so listIndex < size is an in-bounds, non-throwing read. + int elementOrdinal = shard.getElementOrdinal(startElement, endElement, listIndex); + + if(!readState.readWasUnsafe(shardsHolder, ordinal, shard)) { + index++; + return elementOrdinal; + } + + // A delta or re-shard invalidated the snapshot mid-read; re-establish against the current version + // and retry this index. The discarded value is never returned. + snapshot(); + if(listIndex >= size) // the record shrank in the newly observed version + return NO_MORE_ORDINALS; + } + } + +} diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadState.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadState.java index dacac9795f..cd12450de4 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadState.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadState.java @@ -36,7 +36,6 @@ import com.netflix.hollow.core.read.engine.ShardsHolder; import com.netflix.hollow.core.read.engine.SnapshotPopulatedOrdinalsReader; import com.netflix.hollow.core.read.filter.HollowFilterConfig; -import com.netflix.hollow.core.read.iterator.HollowListOrdinalIterator; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; import com.netflix.hollow.core.schema.HollowListSchema; import com.netflix.hollow.core.schema.HollowSchema; @@ -229,10 +228,10 @@ public int size(int ordinal) { @Override public HollowOrdinalIterator ordinalIterator(int ordinal) { sampler.recordIterator(); - return new HollowListOrdinalIterator(ordinal, this); + return new HollowListSnapshotOrdinalIterator(ordinal, this); } - private boolean readWasUnsafe(HollowListTypeShardsHolder shardsHolder, int ordinal, HollowListTypeReadStateShard shard) { + boolean readWasUnsafe(HollowListTypeShardsHolder shardsHolder, int ordinal, HollowListTypeReadStateShard shard) { if(useShardReadFastPaths && shardsAreImmutable) return false; diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIterator.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIterator.java new file mode 100644 index 0000000000..9e22464572 --- /dev/null +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIterator.java @@ -0,0 +1,133 @@ +/* + * Copyright 2016-2019 Netflix, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +package com.netflix.hollow.core.read.engine.map; + +import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIterator; + +/** + * A key/value entry iterator over a map record backed by a single validated snapshot of its shard and bucket + * range. + *

+ * The historical per-bucket pattern re-reads {@code shardsVolatile} and executes {@code Unsafe.loadFence()} + * twice for every bucket probed (via {@code relativeBucket}), plus a {@code size()} at construction — for a map + * whose hash table has M buckets that is {@code 2M+1} fences. This iterator validates the bucket range once at + * construction and then executes a single fence per {@link #next()} (one per surfaced entry), allocating + * nothing. + *

+ * Correctness mirrors the list/set snapshot iterators: {@code startBucket}/{@code endBucket} are validated + * non-torn before use, so every relative bucket index in {@code [0, numBuckets)} maps to an in-bounds read of + * the shard's bucket region (garbage-but-safe if recycled after a concurrent delta, never out-of-bounds). Each + * {@link #next()} scans forward to the next occupied bucket reading from the snapshot, then re-validates shard + * identity via the {@code loadFence} in {@link HollowMapTypeReadState#readWasUnsafe}; if the shard changed, the + * scan is discarded and re-run against a freshly established snapshot before any entry is surfaced. + */ +public class HollowMapSnapshotEntryOrdinalIterator implements HollowMapEntryOrdinalIterator { + + private final HollowMapTypeReadState readState; + private final int ordinal; + + private HollowMapTypeShardsHolder shardsHolder; + private HollowMapTypeReadStateShard shard; + private long startBucket; + private long numBuckets; + + private int currentBucket = -1; + private int key = -1; + private int value = -1; + + public HollowMapSnapshotEntryOrdinalIterator(int ordinal, HollowMapTypeReadState readState) { + this.readState = readState; + this.ordinal = ordinal; + snapshot(); + } + + private void snapshot() { + HollowMapTypeShardsHolder holder; + HollowMapTypeReadStateShard s; + long start; + long end; + do { + holder = readState.shardsVolatile; + s = holder.shards[ordinal & holder.shardNumberMask]; + int shardOrdinal = ordinal >> s.shardOrdinalShift; + + start = s.dataElements.getStartBucket(shardOrdinal); + end = s.dataElements.getEndBucket(shardOrdinal); + } while(readState.readWasUnsafe(holder, ordinal, s)); + + this.shardsHolder = holder; + this.shard = s; + this.startBucket = start; + this.numBuckets = end - start; + } + + @Override + public boolean next() { + while(true) { + int cb = currentBucket; + long bucketValue = -1L; + boolean end; + while(true) { + cb++; + if(cb >= numBuckets) { + end = true; + break; + } + // in-bounds: cb < numBuckets, and [startBucket, startBucket+numBuckets) was validated + bucketValue = shard.relativeBucket(startBucket + cb); + if(bucketValue != -1L) { + end = false; + break; + } + } + + if(!readState.readWasUnsafe(shardsHolder, ordinal, shard)) { + currentBucket = cb; + if(end) { + key = -1; + value = -1; + return false; + } + key = (int)(bucketValue >>> 32); + value = (int)bucketValue; + return true; + } + + // A delta or re-shard invalidated the snapshot mid-scan; re-establish and rescan from currentBucket. + snapshot(); + } + } + + @Override + public int getKey() { + return key; + } + + @Override + public int getValue() { + return value; + } + + /** + * @return the relative bucket position (within this map's bucket range) that the last entry returned by + * {@link #next()} was retrieved from. + */ + public int getCurrentBucket() { + return currentBucket; + } + +} diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java index dc42875d1c..72b8a4371f 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java @@ -43,7 +43,6 @@ import com.netflix.hollow.core.read.filter.HollowFilterConfig; import com.netflix.hollow.core.read.iterator.EmptyMapOrdinalIterator; import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIterator; -import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIteratorImpl; import com.netflix.hollow.core.schema.HollowMapSchema; import com.netflix.hollow.core.schema.HollowObjectSchema.FieldType; import com.netflix.hollow.core.schema.HollowSchema; @@ -369,7 +368,7 @@ public HollowMapEntryOrdinalIterator ordinalIterator(int ordinal) { if(size(ordinal) == 0) return EmptyMapOrdinalIterator.INSTANCE; - return new HollowMapEntryOrdinalIteratorImpl(ordinal, this); + return new HollowMapSnapshotEntryOrdinalIterator(ordinal, this); } @Override @@ -392,7 +391,7 @@ public long relativeBucket(int ordinal, int bucketIndex) { return bucketValue; } - private boolean readWasUnsafe(HollowMapTypeShardsHolder shardsHolder, int ordinal, HollowMapTypeReadStateShard shard) { + boolean readWasUnsafe(HollowMapTypeShardsHolder shardsHolder, int ordinal, HollowMapTypeReadStateShard shard) { if(useShardReadFastPaths && shardsAreImmutable) return false; diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIterator.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIterator.java new file mode 100644 index 0000000000..f4eac8fe12 --- /dev/null +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIterator.java @@ -0,0 +1,115 @@ +/* + * Copyright 2016-2019 Netflix, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +package com.netflix.hollow.core.read.engine.set; + +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; + +/** + * An ordinal iterator over a set record backed by a single validated snapshot of its shard and bucket range. + *

+ * The historical per-bucket pattern re-reads {@code shardsVolatile} and executes {@code Unsafe.loadFence()} + * twice for every bucket probed (via {@code relativeBucketValue}), plus a {@code size()} at construction — for a + * set whose hash table has M buckets that is {@code 2M+1} fences. This iterator validates the bucket range once + * at construction and then executes a single fence per {@link #next()} (one per returned element), allocating + * nothing. + *

+ * Correctness mirrors the list snapshot iterator: {@code startBucket}/{@code endBucket} are validated non-torn + * before use, so every relative bucket index in {@code [0, numBuckets)} maps to an in-bounds read of the shard's + * bucket region (garbage-but-safe if the shard's backing arrays were recycled after a concurrent delta, never + * out-of-bounds). Each {@link #next()} scans forward to the next non-empty bucket reading from the snapshot, + * then re-validates shard identity via the {@code loadFence} in + * {@link HollowSetTypeReadState#readWasUnsafe}; if the shard changed, the scan is discarded and re-run against a + * freshly established snapshot before any value is returned. + */ +public class HollowSetSnapshotOrdinalIterator implements HollowOrdinalIterator { + + private final HollowSetTypeReadState readState; + private final int ordinal; + + private HollowSetTypeShardsHolder shardsHolder; + private HollowSetTypeReadStateShard shard; + private long startBucket; + private long numBuckets; + + private int currentBucket = -1; + + public HollowSetSnapshotOrdinalIterator(int ordinal, HollowSetTypeReadState readState) { + this.readState = readState; + this.ordinal = ordinal; + snapshot(); + } + + private void snapshot() { + HollowSetTypeShardsHolder holder; + HollowSetTypeReadStateShard s; + long start; + long end; + do { + holder = readState.shardsVolatile; + s = holder.shards[ordinal & holder.shardNumberMask]; + int shardOrdinal = ordinal >> s.shardOrdinalShift; + + start = s.dataElements.getStartBucket(shardOrdinal); + end = s.dataElements.getEndBucket(shardOrdinal); + } while(readState.readWasUnsafe(holder, ordinal, s)); + + this.shardsHolder = holder; + this.shard = s; + this.startBucket = start; + this.numBuckets = end - start; + } + + @Override + public int next() { + while(true) { + int cb = currentBucket; + int bucketValue; + boolean end; + while(true) { + cb++; + if(cb >= numBuckets) { + bucketValue = NO_MORE_ORDINALS; + end = true; + break; + } + // in-bounds: cb < numBuckets, and [startBucket, startBucket+numBuckets) was validated + bucketValue = shard.dataElements.getBucketValue(startBucket + cb); + if(bucketValue != shard.dataElements.emptyBucketValue) { + end = false; + break; + } + } + + if(!readState.readWasUnsafe(shardsHolder, ordinal, shard)) { + currentBucket = cb; + return end ? NO_MORE_ORDINALS : bucketValue; + } + + // A delta or re-shard invalidated the snapshot mid-scan; re-establish and rescan from currentBucket. + snapshot(); + } + } + + /** + * @return the relative bucket position (within this set's bucket range) that the last ordinal returned by + * {@link #next()} was retrieved from. + */ + public int getCurrentBucket() { + return currentBucket; + } + +} diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java index 4b883547ed..1ad7f19650 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java @@ -44,7 +44,6 @@ import com.netflix.hollow.core.read.filter.HollowFilterConfig; import com.netflix.hollow.core.read.iterator.EmptyOrdinalIterator; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; -import com.netflix.hollow.core.read.iterator.HollowSetOrdinalIterator; import com.netflix.hollow.core.schema.HollowObjectSchema.FieldType; import com.netflix.hollow.core.schema.HollowSchema; import com.netflix.hollow.core.schema.HollowSetSchema; @@ -331,10 +330,10 @@ public HollowOrdinalIterator ordinalIterator(int ordinal) { sampler.recordIterator(); if(size(ordinal) == 0) return EmptyOrdinalIterator.INSTANCE; - return new HollowSetOrdinalIterator(ordinal, this); + return new HollowSetSnapshotOrdinalIterator(ordinal, this); } - private boolean readWasUnsafe(HollowSetTypeShardsHolder shardsHolder, int ordinal, HollowSetTypeReadStateShard shard) { + boolean readWasUnsafe(HollowSetTypeShardsHolder shardsHolder, int ordinal, HollowSetTypeReadStateShard shard) { if(useShardReadFastPaths && shardsAreImmutable) return false; diff --git a/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowMapCopier.java b/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowMapCopier.java index 0fe2731c0e..c78e23287f 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowMapCopier.java +++ b/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowMapCopier.java @@ -21,7 +21,7 @@ import com.netflix.hollow.core.read.engine.map.HollowMapTypeReadState; import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIterator; -import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIteratorImpl; +import com.netflix.hollow.core.read.engine.map.HollowMapSnapshotEntryOrdinalIterator; import com.netflix.hollow.core.write.HollowMapWriteRecord; import com.netflix.hollow.core.write.HollowWriteRecord; import com.netflix.hollow.tools.combine.OrdinalRemapper; @@ -44,7 +44,7 @@ public HollowWriteRecord copy(int ordinal) { while(iter.next()) { int remappedKeyOrdinal = ordinalRemapper.getMappedOrdinal(keyType, iter.getKey()); int remappedValueOrdinal = ordinalRemapper.getMappedOrdinal(valueType, iter.getValue()); - int hashCode = preserveHashPositions ? ((HollowMapEntryOrdinalIteratorImpl)iter).getCurrentBucket() : remappedKeyOrdinal; + int hashCode = preserveHashPositions ? ((HollowMapSnapshotEntryOrdinalIterator)iter).getCurrentBucket() : remappedKeyOrdinal; rec.addEntry(remappedKeyOrdinal, remappedValueOrdinal, hashCode); } diff --git a/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowSetCopier.java b/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowSetCopier.java index 261127adb5..5588bbf313 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowSetCopier.java +++ b/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowSetCopier.java @@ -21,7 +21,7 @@ import com.netflix.hollow.core.read.engine.set.HollowSetTypeReadState; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; -import com.netflix.hollow.core.read.iterator.HollowSetOrdinalIterator; +import com.netflix.hollow.core.read.engine.set.HollowSetSnapshotOrdinalIterator; import com.netflix.hollow.core.write.HollowSetWriteRecord; import com.netflix.hollow.core.write.HollowWriteRecord; import com.netflix.hollow.tools.combine.OrdinalRemapper; @@ -45,7 +45,7 @@ public HollowWriteRecord copy(int ordinal) { while(elementOrdinal != HollowOrdinalIterator.NO_MORE_ORDINALS) { int remappedElementOrdinal = ordinalRemapper.getMappedOrdinal(elementType, elementOrdinal); - int hashCode = preserveHashPositions ? ((HollowSetOrdinalIterator)ordinalIterator).getCurrentBucket() : remappedElementOrdinal; + int hashCode = preserveHashPositions ? ((HollowSetSnapshotOrdinalIterator)ordinalIterator).getCurrentBucket() : remappedElementOrdinal; rec.addElement(remappedElementOrdinal, hashCode); elementOrdinal = ordinalIterator.next(); } diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIteratorTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIteratorTest.java new file mode 100644 index 0000000000..734363ea96 --- /dev/null +++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIteratorTest.java @@ -0,0 +1,116 @@ +package com.netflix.hollow.core.read.engine.map; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import com.netflix.hollow.core.read.engine.HollowTypeReshardingStrategy; +import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIterator; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.Test; + +/** + * Correctness tests for the snapshot-backed map entry iterator returned by + * {@link HollowMapTypeReadState#ordinalIterator(int)}, including a concurrency stress test that iterates maps + * while another thread reshards the type state in place. Every entry produced by {@code generateMapContents} + * satisfies {@code value == key + 1}; a torn read (mixing recycled memory across a shard swap) would surface an + * entry violating that invariant. + */ +public class HollowMapSnapshotEntryOrdinalIteratorTest extends AbstractHollowMapTypeDataElementsSplitJoinTest { + + /** Iterate the map's entries, asserting the value == key + 1 invariant; returns the entry count. */ + private int drainAndAssert(HollowMapTypeReadState readState, int ordinal) { + HollowMapEntryOrdinalIterator iter = readState.ordinalIterator(ordinal); + int count = 0; + while (iter.next()) { + int key = iter.getKey(); + int value = iter.getValue(); + if (value != key + 1) + fail("Torn/incorrect entry at map ordinal " + ordinal + ": key=" + key + " value=" + value + + " (expected value=" + (key + 1) + ")"); + count++; + } + return count; + } + + @Test + public void iteratesMapEntries() throws IOException { + int numRecords = 200; + int[][][] mapContents = generateMapContents(numRecords); + HollowMapTypeReadState readState = populateTypeStateWith(mapContents); + + boolean sawEntries = false; + for (int ordinal = 0; ordinal <= readState.maxOrdinal(); ordinal++) { + sawEntries |= drainAndAssert(readState, ordinal) > 0; + } + assertTrue(sawEntries); + } + + @Test(timeout = 60_000) + public void iterateConcurrentlyWhileResharding() throws Exception { + int numRecords = 500; + int[][][] mapContents = generateMapContents(numRecords); + final HollowMapTypeReadState readState = populateTypeStateWith(mapContents); + final HollowTypeReshardingStrategy reshardingStrategy = HollowTypeReshardingStrategy.getInstance(readState); + + final int numReaders = 6; + final AtomicBoolean writerDone = new AtomicBoolean(false); + final AtomicReference failure = new AtomicReference<>(); + final CountDownLatch startLatch = new CountDownLatch(1); + final CountDownLatch doneLatch = new CountDownLatch(numReaders + 1); + + List threads = new ArrayList<>(); + + for (int r = 0; r < numReaders; r++) { + final long seed = 0x9E3779B97F4A7C15L * (r + 1); + Thread reader = new Thread(() -> { + try { + startLatch.await(); + long x = seed; + while (!writerDone.get() && failure.get() == null) { + x ^= x << 13; x ^= x >>> 7; x ^= x << 17; + int ordinal = (int) ((x >>> 1) % (readState.maxOrdinal() + 1)); + drainAndAssert(readState, ordinal); + } + } catch (Throwable t) { + failure.compareAndSet(null, t); + } finally { + doneLatch.countDown(); + } + }, "map-reader-" + r); + threads.add(reader); + } + + Thread writer = new Thread(() -> { + try { + startLatch.await(); + for (int cycle = 0; cycle < 200 && failure.get() == null; cycle++) { + int cur = readState.numShards(); + int target = cur < 8 ? cur * 2 : 1; + reshardingStrategy.reshard(readState, cur, target); + } + } catch (Throwable t) { + failure.compareAndSet(null, t); + } finally { + writerDone.set(true); + doneLatch.countDown(); + } + }, "map-resharder"); + threads.add(writer); + + for (Thread t : threads) t.start(); + startLatch.countDown(); + assertTrue("threads did not finish in time", doneLatch.await(55, TimeUnit.SECONDS)); + for (Thread t : threads) t.join(TimeUnit.SECONDS.toMillis(5)); + + if (failure.get() != null) + throw new AssertionError("Concurrent iteration observed a failure", failure.get()); + + assertDataUnchanged(readState, mapContents); + } +} diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIteratorTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIteratorTest.java new file mode 100644 index 0000000000..c577a1ba53 --- /dev/null +++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIteratorTest.java @@ -0,0 +1,133 @@ +package com.netflix.hollow.core.read.engine.set; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import com.netflix.hollow.core.read.engine.HollowTypeReshardingStrategy; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.core.util.IntList; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.Test; + +/** + * Correctness tests for the snapshot-backed set ordinal iterator returned by + * {@link HollowSetTypeReadState#ordinalIterator(int)}, including a concurrency stress test that iterates sets + * while another thread reshards the type state in place. Every set produced by {@code generateSetContents} is the + * element set {@code {0, 1, ..., k}}, so a correct read of any set yields exactly {@code {0 .. count-1}} with no + * gaps or duplicates; a torn read (mixing recycled memory across a shard swap) would surface an out-of-range or + * duplicate element. + */ +public class HollowSetSnapshotOrdinalIteratorTest extends AbstractHollowSetTypeDataElementsSplitJoinTest { + + private IntList drain(HollowSetTypeReadState readState, int ordinal) { + HollowOrdinalIterator iter = readState.ordinalIterator(ordinal); + IntList out = new IntList(); + int o = iter.next(); + while (o != HollowOrdinalIterator.NO_MORE_ORDINALS) { + out.add(o); + o = iter.next(); + } + return out; + } + + /** A valid set of count elements is exactly {0, 1, ..., count-1} (order is hash-dependent). */ + private void assertContiguousSet(IntList actual, int ordinal) { + int n = actual.size(); + boolean[] seen = new boolean[n]; + for (int i = 0; i < n; i++) { + int v = actual.get(i); + if (v < 0 || v >= n) + fail("Torn/incorrect read at set ordinal " + ordinal + ": element " + v + " out of range [0," + n + ")"); + if (seen[v]) + fail("Torn/incorrect read at set ordinal " + ordinal + ": duplicate element " + v); + seen[v] = true; + } + } + + @Test + public void iteratesEmptyAndSingleAndMultiElementSets() throws IOException { + int numRecords = 50; + int[][] setContents = generateSetContents(numRecords); + HollowSetTypeReadState readState = populateTypeStateWith(setContents); + + boolean sawSizeOne = false; + boolean sawLarge = false; + for (int ordinal = 0; ordinal <= readState.maxOrdinal(); ordinal++) { + IntList actual = drain(readState, ordinal); + assertContiguousSet(actual, ordinal); + sawSizeOne |= actual.size() == 1; + sawLarge |= actual.size() == numRecords; + } + assertTrue(sawSizeOne); + assertTrue(sawLarge); + } + + @Test(timeout = 60_000) + public void iterateConcurrentlyWhileResharding() throws Exception { + int numRecords = 500; + int[][] setContents = generateSetContents(numRecords); + final HollowSetTypeReadState readState = populateTypeStateWith(setContents); + final HollowTypeReshardingStrategy reshardingStrategy = HollowTypeReshardingStrategy.getInstance(readState); + + final int numReaders = 6; + final AtomicBoolean writerDone = new AtomicBoolean(false); + final AtomicReference failure = new AtomicReference<>(); + final CountDownLatch startLatch = new CountDownLatch(1); + final CountDownLatch doneLatch = new CountDownLatch(numReaders + 1); + + List threads = new ArrayList<>(); + + for (int r = 0; r < numReaders; r++) { + final long seed = 0x9E3779B97F4A7C15L * (r + 1); + Thread reader = new Thread(() -> { + try { + startLatch.await(); + long x = seed; + while (!writerDone.get() && failure.get() == null) { + x ^= x << 13; x ^= x >>> 7; x ^= x << 17; + int ordinal = (int) ((x >>> 1) % (readState.maxOrdinal() + 1)); + assertContiguousSet(drain(readState, ordinal), ordinal); + } + } catch (Throwable t) { + failure.compareAndSet(null, t); + } finally { + doneLatch.countDown(); + } + }, "set-reader-" + r); + threads.add(reader); + } + + Thread writer = new Thread(() -> { + try { + startLatch.await(); + for (int cycle = 0; cycle < 200 && failure.get() == null; cycle++) { + int cur = readState.numShards(); + int target = cur < 8 ? cur * 2 : 1; + reshardingStrategy.reshard(readState, cur, target); + } + } catch (Throwable t) { + failure.compareAndSet(null, t); + } finally { + writerDone.set(true); + doneLatch.countDown(); + } + }, "set-resharder"); + threads.add(writer); + + for (Thread t : threads) t.start(); + startLatch.countDown(); + assertTrue("threads did not finish in time", doneLatch.await(55, TimeUnit.SECONDS)); + for (Thread t : threads) t.join(TimeUnit.SECONDS.toMillis(5)); + + if (failure.get() != null) + throw new AssertionError("Concurrent iteration observed a failure", failure.get()); + + assertDataUnchanged(readState, setContents); + } +} diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/iterator/HollowListSnapshotOrdinalIteratorTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/iterator/HollowListSnapshotOrdinalIteratorTest.java new file mode 100644 index 0000000000..d9a7ad6566 --- /dev/null +++ b/hollow/src/test/java/com/netflix/hollow/core/read/iterator/HollowListSnapshotOrdinalIteratorTest.java @@ -0,0 +1,167 @@ +package com.netflix.hollow.core.read.iterator; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import com.netflix.hollow.core.read.engine.HollowTypeReshardingStrategy; +import com.netflix.hollow.core.read.engine.list.AbstractHollowListTypeDataElementsSplitJoinTest; +import com.netflix.hollow.core.read.engine.list.HollowListTypeReadState; +import com.netflix.hollow.core.util.IntList; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.Test; + +/** + * Correctness tests for the snapshot-backed list ordinal iterator returned by + * {@link HollowListTypeReadState#ordinalIterator(int)}, including a concurrency stress test that iterates lists + * while another thread reshards the type state in place. Resharding atomically swaps {@code shardsVolatile} and + * then recycles the old {@code dataElements}' backing arrays, which is precisely the window the iterator's + * validated-snapshot read protocol must tolerate. Every list produced by {@code generateListContents} is the + * contiguous run {@code [0, 1, ..., k]}, so any correct read satisfies {@code v[j] == j}; a torn read (mixing + * recycled memory across a shard swap) would violate that invariant. + */ +public class HollowListSnapshotOrdinalIteratorTest extends AbstractHollowListTypeDataElementsSplitJoinTest { + + private IntList drain(HollowListTypeReadState readState, int ordinal) { + HollowOrdinalIterator iter = readState.ordinalIterator(ordinal); + IntList out = new IntList(); + int o = iter.next(); + while (o != HollowOrdinalIterator.NO_MORE_ORDINALS) { + out.add(o); + o = iter.next(); + } + return out; + } + + /** A valid list of size n is exactly [0, 1, ..., n-1]. */ + private void assertContiguousFromZero(IntList actual, int ordinal) { + for (int j = 0; j < actual.size(); j++) { + if (actual.get(j) != j) { + fail("Torn/incorrect read at ordinal " + ordinal + ", index " + j + + ": expected " + j + " but got " + actual.get(j)); + } + } + } + + @Test + public void iteratesEmptyAndSingleAndMultiElementLists() throws IOException { + // sizes 1..50 (generateListContents produces list i of size i+1) + int numRecords = 50; + int[][] listContents = generateListContents(numRecords); + HollowListTypeReadState readState = populateTypeStateWith(listContents); + + boolean sawSizeOne = false; + boolean sawLarge = false; + for (int ordinal = 0; ordinal <= readState.maxOrdinal(); ordinal++) { + IntList actual = drain(readState, ordinal); + assertContiguousFromZero(actual, ordinal); + sawSizeOne |= actual.size() == 1; + sawLarge |= actual.size() == numRecords; + } + assertTrue(sawSizeOne); + assertTrue(sawLarge); + } + + @Test + public void iteratorExhaustsCleanlyAndRepeatsNoMoreOrdinals() throws IOException { + HollowListTypeReadState readState = populateTypeStateWith(generateListContents(10)); + HollowOrdinalIterator iter = readState.ordinalIterator(0); + int count = 0; + while (iter.next() != HollowOrdinalIterator.NO_MORE_ORDINALS) { + count++; + } + assertTrue(count > 0); + // Past the end, next() must keep returning NO_MORE_ORDINALS. + assertEquals(HollowOrdinalIterator.NO_MORE_ORDINALS, iter.next()); + assertEquals(HollowOrdinalIterator.NO_MORE_ORDINALS, iter.next()); + } + + @Test + public void iteratorMatchesPerElementReads() throws IOException { + HollowListTypeReadState readState = populateTypeStateWith(generateListContents(100)); + for (int ordinal = 0; ordinal <= readState.maxOrdinal(); ordinal++) { + int size = readState.size(ordinal); + IntList iterated = drain(readState, ordinal); + assertEquals("size mismatch at ordinal " + ordinal, size, iterated.size()); + for (int i = 0; i < size; i++) { + assertEquals("element mismatch at ordinal " + ordinal + " index " + i, + readState.getElementOrdinal(ordinal, i), iterated.get(i)); + } + } + } + + @Test(timeout = 60_000) + public void iterateConcurrentlyWhileResharding() throws Exception { + int numRecords = 1000; + int[][] listContents = generateListContents(numRecords); + final HollowListTypeReadState readState = populateTypeStateWith(listContents); + final HollowTypeReshardingStrategy reshardingStrategy = HollowTypeReshardingStrategy.getInstance(readState); + + final int numReaders = 6; + final AtomicBoolean writerDone = new AtomicBoolean(false); + final AtomicReference failure = new AtomicReference<>(); + final CountDownLatch startLatch = new CountDownLatch(1); + final CountDownLatch doneLatch = new CountDownLatch(numReaders + 1); + + List threads = new ArrayList<>(); + + // Reader threads: continuously iterate random lists and validate the contiguous invariant. + for (int r = 0; r < numReaders; r++) { + final long seed = 0x9E3779B97F4A7C15L * (r + 1); // deterministic per-reader, no shared RNG + Thread reader = new Thread(() -> { + try { + startLatch.await(); + long x = seed; + while (!writerDone.get() && failure.get() == null) { + // xorshift for a cheap, allocation-free, thread-local pseudo-random ordinal + x ^= x << 13; x ^= x >>> 7; x ^= x << 17; + int ordinal = (int) ((x >>> 1) % (readState.maxOrdinal() + 1)); + IntList actual = drain(readState, ordinal); + assertContiguousFromZero(actual, ordinal); + } + } catch (Throwable t) { + failure.compareAndSet(null, t); + } finally { + doneLatch.countDown(); + } + }, "list-reader-" + r); + threads.add(reader); + } + + // Writer thread: repeatedly split and join shards, swapping shardsVolatile and recycling old memory. + Thread writer = new Thread(() -> { + try { + startLatch.await(); + for (int cycle = 0; cycle < 300 && failure.get() == null; cycle++) { + int cur = readState.numShards(); + int target = cur < 8 ? cur * 2 : 1; // ...->2->4->8->1->2->... + reshardingStrategy.reshard(readState, cur, target); + } + } catch (Throwable t) { + failure.compareAndSet(null, t); + } finally { + writerDone.set(true); + doneLatch.countDown(); + } + }, "list-resharder"); + threads.add(writer); + + for (Thread t : threads) t.start(); + startLatch.countDown(); + assertTrue("threads did not finish in time", doneLatch.await(55, TimeUnit.SECONDS)); + for (Thread t : threads) t.join(TimeUnit.SECONDS.toMillis(5)); + + if (failure.get() != null) { + throw new AssertionError("Concurrent iteration observed a failure", failure.get()); + } + + // Data must be fully intact after all the resharding churn. + assertDataUnchanged(readState, listContents); + } +} From f59cc069e6a1ab1246fc11fd2df1ebe62619f8be Mon Sep 17 00:00:00 2001 From: Danny Thomas Date: Tue, 22 Sep 2026 10:21:26 +1000 Subject: [PATCH 2/6] Integrate immutable shard collection iteration 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. --- ...owListTypeReadStateIterationBenchmark.java | 27 ++++-- ...lowMapTypeReadStateIterationBenchmark.java | 21 +++-- ...lowSetTypeReadStateIterationBenchmark.java | 20 ++++- .../api/client/HollowClientUpdater.java | 18 +++- .../hollow/api/consumer/HollowConsumer.java | 12 +++ .../hollow/api/objects/HollowList.java | 30 +++++++ .../delegate/HollowListCachedDelegate.java | 15 ++++ .../objects/delegate/HollowListDelegate.java | 5 ++ .../api/perfapi/HollowPerfBackedList.java | 30 +++++++ .../read/engine/HollowReadStateEngine.java | 12 +++ .../HollowListSnapshotOrdinalIterator.java | 32 ++++--- .../engine/list/HollowListTypeReadState.java | 5 +- ...HollowMapSnapshotEntryOrdinalIterator.java | 24 ++++-- .../engine/map/HollowMapTypeReadState.java | 5 +- .../set/HollowSetSnapshotOrdinalIterator.java | 27 +++--- .../engine/set/HollowSetTypeReadState.java | 5 +- .../iterator/HollowListOrdinalIterator.java | 10 ++- .../HollowMapEntryOrdinalIteratorImpl.java | 11 ++- .../iterator/HollowSetOrdinalIterator.java | 10 ++- .../core/write/copy/HollowMapCopier.java | 4 +- .../core/write/copy/HollowSetCopier.java | 4 +- .../consumer/HollowConsumerBuilderTest.java | 23 +++++ .../api/objects/HollowListIteratorTest.java | 68 +++++++++++++++ .../api/perfapi/HollowPerfBackedListTest.java | 36 ++++++++ .../list/HollowListTypeReadStateTest.java | 84 +++++++++++++++++++ ...owMapSnapshotEntryOrdinalIteratorTest.java | 6 ++ .../HollowSetSnapshotOrdinalIteratorTest.java | 6 ++ ...HollowListSnapshotOrdinalIteratorTest.java | 4 + 28 files changed, 486 insertions(+), 68 deletions(-) create mode 100644 hollow/src/test/java/com/netflix/hollow/api/objects/HollowListIteratorTest.java create mode 100644 hollow/src/test/java/com/netflix/hollow/api/perfapi/HollowPerfBackedListTest.java diff --git a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateIterationBenchmark.java b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateIterationBenchmark.java index 7f6dfa5ecf..e6a04efa3d 100644 --- a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateIterationBenchmark.java +++ b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateIterationBenchmark.java @@ -1,5 +1,8 @@ package com.netflix.hollow.core.read.engine.list; +import com.netflix.hollow.core.memory.pool.ArraySegmentRecycler; +import com.netflix.hollow.core.memory.pool.RecyclingRecycler; +import com.netflix.hollow.core.memory.pool.WastefulRecycler; import com.netflix.hollow.core.read.engine.HollowReadStateEngine; import com.netflix.hollow.core.read.engine.HollowTypeReadState; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; @@ -25,12 +28,11 @@ /** * Compares list iteration cost of the per-element read path (the historical {@code HollowListOrdinalIterator} - * pattern: {@code size()} + {@code getElementOrdinal(ordinal, i)} per element, i.e. 2N+1 {@code Unsafe.loadFence()}s) - * against the single-snapshot path ({@code ordinalIterator()} backed by - * {@code HollowListTypeReadState.readElementOrdinals}, i.e. 2 fences for the whole list). - * - * The gap should widen with list size, since the per-element path pays two fences per element while the snapshot - * path pays two fences total. + * pattern: {@code size()} + {@code getElementOrdinal(ordinal, i)} per element, i.e. 2N+1 + * {@code Unsafe.loadFence()}s when arrays can be recycled) + * against the single-snapshot path ({@code ordinalIterator()} backed by a shard cursor). With recycling, the + * cursor validates once per element; without recycling, its captured shard is immutable and requires no trailing + * validation. */ @State(Scope.Thread) @BenchmarkMode(Mode.AverageTime) @@ -51,6 +53,9 @@ static class ListHolder { @Param({ "2000" }) int numLists; + @Param({ "recycling", "wasteful" }) + String recycler; + HollowListTypeReadState listTypeState; int[] ordinals; @@ -67,7 +72,11 @@ public void setUp() throws IOException { objectMapper.add(new ListHolder(vals)); } - HollowReadStateEngine readStateEngine = new HollowReadStateEngine(); + ArraySegmentRecycler memoryRecycler = recycler.equals("recycling") + ? new RecyclingRecycler() + : WastefulRecycler.DEFAULT_INSTANCE; + HollowReadStateEngine readStateEngine = new HollowReadStateEngine(memoryRecycler); + readStateEngine.setSnapshotCollectionIterators(true); StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); listTypeState = null; @@ -89,7 +98,7 @@ public void setUp() throws IOException { ordinals[i] = populated.get(i); } - /** New path: one validated snapshot per list (2 fences), served from the materialized iterator. */ + /** New path: one shard snapshot per list, validated per element only when arrays can be recycled. */ @Benchmark public void iterateSnapshot(Blackhole bh) { for (int ordinal : ordinals) { @@ -102,7 +111,7 @@ public void iterateSnapshot(Blackhole bh) { } } - /** Old path: size() then getElementOrdinal(ordinal, i) per element (2N+1 fences per list). */ + /** Old path: size() then getElementOrdinal(ordinal, i) per element. */ @Benchmark public void iteratePerElement(Blackhole bh) { for (int ordinal : ordinals) { diff --git a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadStateIterationBenchmark.java b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadStateIterationBenchmark.java index a821119d24..ac24074902 100644 --- a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadStateIterationBenchmark.java +++ b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadStateIterationBenchmark.java @@ -1,6 +1,9 @@ package com.netflix.hollow.core.read.engine.map; import com.netflix.hollow.core.memory.encoding.HashCodes; +import com.netflix.hollow.core.memory.pool.ArraySegmentRecycler; +import com.netflix.hollow.core.memory.pool.RecyclingRecycler; +import com.netflix.hollow.core.memory.pool.WastefulRecycler; import com.netflix.hollow.core.read.engine.HollowReadStateEngine; import com.netflix.hollow.core.read.engine.HollowTypeReadState; import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIterator; @@ -27,8 +30,9 @@ /** * Compares map entry iteration cost of the per-bucket read path (the historical * {@code HollowMapEntryOrdinalIteratorImpl} pattern: {@code size()} + {@code relativeBucket(ordinal, bucket)} - * over every hash bucket, i.e. two fences per bucket) against the single-snapshot entry iterator - * ({@code ordinalIterator()}, one fence per surfaced entry). + * over every hash bucket, i.e. two fences per bucket when arrays can be recycled) against the single-snapshot + * entry iterator. With recycling, the cursor validates once per surfaced entry; without recycling, its captured + * shard is immutable and requires no trailing validation. */ @State(Scope.Thread) @BenchmarkMode(Mode.AverageTime) @@ -49,6 +53,9 @@ static class MapHolder { @Param({ "2000" }) int numMaps; + @Param({ "recycling", "wasteful" }) + String recycler; + HollowMapTypeReadState mapTypeState; int[] ordinals; @@ -65,7 +72,11 @@ public void setUp() throws IOException { objectMapper.add(new MapHolder(vals)); } - HollowReadStateEngine readStateEngine = new HollowReadStateEngine(); + ArraySegmentRecycler memoryRecycler = recycler.equals("recycling") + ? new RecyclingRecycler() + : WastefulRecycler.DEFAULT_INSTANCE; + HollowReadStateEngine readStateEngine = new HollowReadStateEngine(memoryRecycler); + readStateEngine.setSnapshotCollectionIterators(true); StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); mapTypeState = null; @@ -84,7 +95,7 @@ public void setUp() throws IOException { ordinals[o] = o; } - /** New path: one validated snapshot per map, one fence per surfaced entry. */ + /** New path: one shard snapshot per map, validated per entry only when arrays can be recycled. */ @Benchmark public void iterateSnapshot(Blackhole bh) { for (int ordinal : ordinals) { @@ -96,7 +107,7 @@ public void iterateSnapshot(Blackhole bh) { } } - /** Old path: size() then relativeBucket(ordinal, bucket) over every hash bucket (2 fences per bucket). */ + /** Old path: size() then relativeBucket(ordinal, bucket) over every hash bucket. */ @Benchmark public void iteratePerBucket(Blackhole bh) { for (int ordinal : ordinals) { diff --git a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadStateIterationBenchmark.java b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadStateIterationBenchmark.java index 567f8bcff9..a6342104af 100644 --- a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadStateIterationBenchmark.java +++ b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadStateIterationBenchmark.java @@ -3,6 +3,9 @@ import static com.netflix.hollow.core.HollowConstants.ORDINAL_NONE; import com.netflix.hollow.core.memory.encoding.HashCodes; +import com.netflix.hollow.core.memory.pool.ArraySegmentRecycler; +import com.netflix.hollow.core.memory.pool.RecyclingRecycler; +import com.netflix.hollow.core.memory.pool.WastefulRecycler; import com.netflix.hollow.core.read.engine.HollowReadStateEngine; import com.netflix.hollow.core.read.engine.HollowTypeReadState; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; @@ -29,7 +32,9 @@ /** * Compares set iteration cost of the per-bucket read path (the historical {@code HollowSetOrdinalIterator} * pattern: {@code size()} + {@code relativeBucketValue(ordinal, bucket)} over every hash bucket, i.e. two fences - * per bucket) against the single-snapshot iterator ({@code ordinalIterator()}, one fence per surfaced element). + * per bucket when arrays can be recycled) against the single-snapshot iterator. With recycling, the cursor + * validates once per surfaced element; without recycling, its captured shard is immutable and requires no + * trailing validation. */ @State(Scope.Thread) @BenchmarkMode(Mode.AverageTime) @@ -50,6 +55,9 @@ static class SetHolder { @Param({ "2000" }) int numSets; + @Param({ "recycling", "wasteful" }) + String recycler; + HollowSetTypeReadState setTypeState; int[] ordinals; @@ -66,7 +74,11 @@ public void setUp() throws IOException { objectMapper.add(new SetHolder(vals)); } - HollowReadStateEngine readStateEngine = new HollowReadStateEngine(); + ArraySegmentRecycler memoryRecycler = recycler.equals("recycling") + ? new RecyclingRecycler() + : WastefulRecycler.DEFAULT_INSTANCE; + HollowReadStateEngine readStateEngine = new HollowReadStateEngine(memoryRecycler); + readStateEngine.setSnapshotCollectionIterators(true); StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); setTypeState = null; @@ -85,7 +97,7 @@ public void setUp() throws IOException { ordinals[o] = o; } - /** New path: one validated snapshot per set, one fence per surfaced element. */ + /** New path: one shard snapshot per set, validated per element only when arrays can be recycled. */ @Benchmark public void iterateSnapshot(Blackhole bh) { for (int ordinal : ordinals) { @@ -98,7 +110,7 @@ public void iterateSnapshot(Blackhole bh) { } } - /** Old path: size() then relativeBucketValue(ordinal, bucket) over every hash bucket (2 fences per bucket). */ + /** Old path: size() then relativeBucketValue(ordinal, bucket) over every hash bucket. */ @Benchmark public void iteratePerBucket(Blackhole bh) { for (int ordinal : ordinals) { diff --git a/hollow/src/main/java/com/netflix/hollow/api/client/HollowClientUpdater.java b/hollow/src/main/java/com/netflix/hollow/api/client/HollowClientUpdater.java index 128a6b39e0..1fb11227dd 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/client/HollowClientUpdater.java +++ b/hollow/src/main/java/com/netflix/hollow/api/client/HollowClientUpdater.java @@ -66,6 +66,7 @@ public class HollowClientUpdater { private boolean skipTypeShardUpdateWithNoAdditions; private ExperimentalFeature[] experimentalFeatures = new ExperimentalFeature[0]; private MemoryRecyclingMode memoryRecyclingMode = MemoryRecyclingMode.AUTO; + private boolean snapshotCollectionIterators; private TypeFilter filter; @@ -135,6 +136,13 @@ public void setSkipShardUpdateWithNoAdditions(boolean skipTypeShardUpdateWithNoA dataHolder.getStateEngine().setSkipTypeShardUpdateWithNoAdditions(skipTypeShardUpdateWithNoAdditions); } + public void setSnapshotCollectionIterators(boolean snapshotCollectionIterators) { + this.snapshotCollectionIterators = snapshotCollectionIterators; + HollowDataHolder dataHolder = hollowDataHolderVolatile; + if(dataHolder != null) + dataHolder.getStateEngine().setSnapshotCollectionIterators(snapshotCollectionIterators); + } + /** * Updates the client's state to the requested version, or to the version closest to but less than the requested version. * @@ -349,14 +357,18 @@ private HollowDataHolder newHollowDataHolder() { private HollowReadStateEngine newStateEngine() { HollowDataHolder hollowDataHolderLocal = hollowDataHolderVolatile; + HollowReadStateEngine stateEngine; if (hollowDataHolderLocal != null) { ArraySegmentRecycler existingRecycler = hollowDataHolderLocal.getStateEngine().getMemoryRecycler(); - return new HollowReadStateEngine(hashCodeFinder, true, + stateEngine = new HollowReadStateEngine(hashCodeFinder, true, new HollowReadConfiguration(memoryMode, existingRecycler, experimentalFeatures)); + } else { + stateEngine = new HollowReadStateEngine(hashCodeFinder, true, + new HollowReadConfiguration(memoryMode, memoryRecyclingMode.createRecycler(), experimentalFeatures)); } - return new HollowReadStateEngine(hashCodeFinder, true, - new HollowReadConfiguration(memoryMode, memoryRecyclingMode.createRecycler(), experimentalFeatures)); + stateEngine.setSnapshotCollectionIterators(snapshotCollectionIterators); + return stateEngine; } public StackTraceRecorder getStaleReferenceUsageStackTraceRecorder() { diff --git a/hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java b/hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java index 95f51333e7..e889cc9eda 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java +++ b/hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java @@ -210,6 +210,8 @@ protected > HollowConsumer(B builder) { updater.setMemoryRecyclingMode(builder.memoryRecyclingMode); if(builder.skipTypeShardUpdateWithNoAdditions) updater.setSkipShardUpdateWithNoAdditions(true); + if(builder.snapshotCollectionIterators) + updater.setSnapshotCollectionIterators(true); this.announcementWatcher = builder.announcementWatcher; this.refreshExecutor = builder.refreshExecutor; this.refreshLock = new ReentrantReadWriteLock(); @@ -1165,6 +1167,7 @@ public static class Builder> { protected HollowMetricsCollector metricsCollector; protected boolean skipTypeShardUpdateWithNoAdditions = false; protected final EnumSet experimentalFeatures = EnumSet.noneOf(ExperimentalFeature.class); + protected boolean snapshotCollectionIterators = false; public B withBlobRetriever(HollowConsumer.BlobRetriever blobRetriever) { this.blobRetriever = blobRetriever; @@ -1478,6 +1481,15 @@ public B withSkipTypeShardUpdateWithNoAdditions() { return (B)this; } + /** + * Experimental: Use snapshot iterators for collection types. Snapshot iterators capture a + * collection's shard and bounds once rather than resolving them for every element or bucket. + */ + public B withSnapshotCollectionIterators() { + this.snapshotCollectionIterators = true; + return (B)this; + } + @Deprecated public B withHashCodeFinder(HollowObjectHashCodeFinder hashCodeFinder) { this.hashCodeFinder = hashCodeFinder; diff --git a/hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java b/hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java index 5d5a507315..e152cdc330 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java +++ b/hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java @@ -19,8 +19,11 @@ import com.netflix.hollow.api.objects.delegate.HollowListDelegate; import com.netflix.hollow.api.objects.delegate.HollowRecordDelegate; import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; import com.netflix.hollow.core.schema.HollowListSchema; import java.util.AbstractList; +import java.util.Iterator; +import java.util.NoSuchElementException; /** * A HollowList provides an implementation of the {@link java.util.List} interface over @@ -51,6 +54,33 @@ public final T get(int index) { return delegate.get(this, ordinal, index); } + @Override + public Iterator iterator() { + HollowOrdinalIterator ordinalIterator = delegate.iterator(ordinal); + return new Iterator() { + private int nextOrdinal = HollowOrdinalIterator.NO_MORE_ORDINALS; + private boolean nextOrdinalLoaded; + + @Override + public boolean hasNext() { + if(!nextOrdinalLoaded) { + nextOrdinal = ordinalIterator.next(); + nextOrdinalLoaded = true; + } + return nextOrdinal != HollowOrdinalIterator.NO_MORE_ORDINALS; + } + + @Override + public T next() { + if(!hasNext()) + throw new NoSuchElementException(); + int currentOrdinal = nextOrdinal; + nextOrdinalLoaded = false; + return instantiateElement(currentOrdinal); + } + }; + } + @Override public final boolean contains(Object o) { return delegate.contains(this, ordinal, o); diff --git a/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListCachedDelegate.java b/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListCachedDelegate.java index a2863c4eb6..7e9bb615ef 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListCachedDelegate.java +++ b/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListCachedDelegate.java @@ -20,6 +20,7 @@ import com.netflix.hollow.api.custom.HollowTypeAPI; import com.netflix.hollow.api.objects.HollowList; import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; import com.netflix.hollow.core.schema.HollowListSchema; /** @@ -85,6 +86,20 @@ public final int lastIndexOf(HollowList list, int ordinal, Object o) { return -1; } + @Override + public HollowOrdinalIterator iterator(int ordinal) { + return new HollowOrdinalIterator() { + private int index; + + @Override + public int next() { + if(index == ordinals.length) + return NO_MORE_ORDINALS; + return ordinals[index++]; + } + }; + } + @Override public HollowListSchema getSchema() { return dataAccess.getSchema(); diff --git a/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListDelegate.java b/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListDelegate.java index f46934a4a9..ce383cf7ef 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListDelegate.java +++ b/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListDelegate.java @@ -19,6 +19,7 @@ import com.netflix.hollow.api.custom.HollowListTypeAPI; import com.netflix.hollow.api.objects.HollowList; import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; import com.netflix.hollow.core.schema.HollowListSchema; /** @@ -42,6 +43,10 @@ public interface HollowListDelegate extends HollowRecordDelegate { public HollowListTypeDataAccess getTypeDataAccess(); + default HollowOrdinalIterator iterator(int ordinal) { + return getTypeDataAccess().ordinalIterator(ordinal); + } + public HollowListTypeAPI getTypeAPI(); } diff --git a/hollow/src/main/java/com/netflix/hollow/api/perfapi/HollowPerfBackedList.java b/hollow/src/main/java/com/netflix/hollow/api/perfapi/HollowPerfBackedList.java index 750c72a3e6..4c60784174 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/perfapi/HollowPerfBackedList.java +++ b/hollow/src/main/java/com/netflix/hollow/api/perfapi/HollowPerfBackedList.java @@ -17,7 +17,10 @@ package com.netflix.hollow.api.perfapi; import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; import java.util.AbstractList; +import java.util.Iterator; +import java.util.NoSuchElementException; import java.util.RandomAccess; public class HollowPerfBackedList extends AbstractList implements RandomAccess { @@ -45,4 +48,31 @@ public int size() { return dataAccess.size(ordinal); } + @Override + public Iterator iterator() { + HollowOrdinalIterator ordinalIterator = dataAccess.ordinalIterator(ordinal); + return new Iterator() { + private int nextOrdinal = HollowOrdinalIterator.NO_MORE_ORDINALS; + private boolean nextOrdinalLoaded; + + @Override + public boolean hasNext() { + if(!nextOrdinalLoaded) { + nextOrdinal = ordinalIterator.next(); + nextOrdinalLoaded = true; + } + return nextOrdinal != HollowOrdinalIterator.NO_MORE_ORDINALS; + } + + @Override + public T next() { + if(!hasNext()) + throw new NoSuchElementException(); + int currentOrdinal = nextOrdinal; + nextOrdinalLoaded = false; + return instantiator.instantiate(elementMaskedTypeIdx | currentOrdinal); + } + }; + } + } diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java index a77e763a52..ca5c1a091e 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java @@ -59,6 +59,7 @@ public class HollowReadStateEngine implements HollowStateEngine, HollowDataAcces private final boolean listenToAllPopulatedOrdinals; private final HollowReadConfiguration readConfiguration; private boolean skipTypeShardUpdateWithNoAdditions; + private boolean snapshotCollectionIterators; private ArraySegmentRecycler memoryRecycler; private Map headerTags; private Set typesWithDefinedHashCodes = new HashSet(); @@ -263,6 +264,17 @@ public boolean isSkipTypeShardUpdateWithNoAdditions() { return skipTypeShardUpdateWithNoAdditions; } + /** + * Experimental: Select snapshot iterators for list, set, and map type read states. + */ + public void setSnapshotCollectionIterators(boolean snapshotCollectionIterators) { + this.snapshotCollectionIterators = snapshotCollectionIterators; + } + + public boolean isSnapshotCollectionIteratorsEnabled() { + return snapshotCollectionIterators; + } + @Override public List getSchemas() { List schemas = new ArrayList(); diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListSnapshotOrdinalIterator.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListSnapshotOrdinalIterator.java index d3c106f15b..1f12482e6f 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListSnapshotOrdinalIterator.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListSnapshotOrdinalIterator.java @@ -16,7 +16,8 @@ */ package com.netflix.hollow.core.read.engine.list; -import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.api.sampling.HollowListSampler; +import com.netflix.hollow.core.read.iterator.HollowListOrdinalIterator; /** * An ordinal iterator over a list record backed by a single validated snapshot of its shard and bounds. @@ -28,19 +29,15 @@ * fence per {@link #next()}, i.e. {@code N+1} fences, while allocating nothing per element. *

* Why holding the snapshot across {@code next()} calls is safe. The bounds {@code startElement}/ - * {@code endElement} are validated non-torn before use, so every {@code listIndex < size} maps to a bit offset - * within the shard's element region — an in-bounds, non-throwing read on-heap and within the buffer extent in - * shared-memory mode. A concurrent delta or re-shard can only (a) publish a new shard (the captured reference is - * untouched) and (b) later recycle the old shard's backing arrays, which are pooled but never truncated or - * nulled — so a stale read yields garbage of the correct length, never an out-of-bounds access. The captured - * {@code dataElements} reference also keeps that backing storage reachable for the life of the iterator. Each - * {@link #next()} re-validates shard identity after reading (the {@code loadFence} in - * {@link HollowListTypeReadState#readWasUnsafe} orders the element read before the re-read of - * {@code shardsVolatile}); if the shard changed, the value read is discarded and the snapshot is re-established - * against the newly observed version before the value is returned. Thus no torn value ever escapes, matching the - * per-element validation semantics of the original iterator without its redundant second fence. + * {@code endElement} are validated before use, so every {@code listIndex < size} maps to an in-bounds read from + * the captured shard. During an on-heap update, the captured data remains strongly reachable and recycled arrays + * retain their fixed size. Each {@link #next()} re-validates shard identity after reading; if an update or + * reshard replaced it, the value is discarded and the snapshot is re-established before a value is returned. + *

+ * When retired on-heap arrays are not recycled, the captured shard remains immutable and strongly reachable. + * In that case {@code readWasUnsafe} is a no-op, so the iterator retains its initial snapshot without fences. */ -class HollowListSnapshotOrdinalIterator implements HollowOrdinalIterator { +class HollowListSnapshotOrdinalIterator extends HollowListOrdinalIterator { private final HollowListTypeReadState readState; private final int ordinal; @@ -53,9 +50,11 @@ class HollowListSnapshotOrdinalIterator implements HollowOrdinalIterator { private int index; - HollowListSnapshotOrdinalIterator(int ordinal, HollowListTypeReadState readState) { + HollowListSnapshotOrdinalIterator( + int ordinal, HollowListTypeReadState readState, HollowListSampler sampler) { this.readState = readState; this.ordinal = ordinal; + sampler.recordSize(); snapshot(); } @@ -100,10 +99,9 @@ public int next() { return elementOrdinal; } - // A delta or re-shard invalidated the snapshot mid-read; re-establish against the current version - // and retry this index. The discarded value is never returned. + // The snapshot changed mid-read; re-establish it and retry this index. snapshot(); - if(listIndex >= size) // the record shrank in the newly observed version + if(listIndex >= size) return NO_MORE_ORDINALS; } } diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadState.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadState.java index cd12450de4..1397229468 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadState.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadState.java @@ -36,6 +36,7 @@ import com.netflix.hollow.core.read.engine.ShardsHolder; import com.netflix.hollow.core.read.engine.SnapshotPopulatedOrdinalsReader; import com.netflix.hollow.core.read.filter.HollowFilterConfig; +import com.netflix.hollow.core.read.iterator.HollowListOrdinalIterator; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; import com.netflix.hollow.core.schema.HollowListSchema; import com.netflix.hollow.core.schema.HollowSchema; @@ -228,7 +229,9 @@ public int size(int ordinal) { @Override public HollowOrdinalIterator ordinalIterator(int ordinal) { sampler.recordIterator(); - return new HollowListSnapshotOrdinalIterator(ordinal, this); + if(stateEngine == null || !stateEngine.isSnapshotCollectionIteratorsEnabled()) + return new HollowListOrdinalIterator(ordinal, this); + return new HollowListSnapshotOrdinalIterator(ordinal, this, sampler); } boolean readWasUnsafe(HollowListTypeShardsHolder shardsHolder, int ordinal, HollowListTypeReadStateShard shard) { diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIterator.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIterator.java index 9e22464572..1e73851c7d 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIterator.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIterator.java @@ -16,7 +16,8 @@ */ package com.netflix.hollow.core.read.engine.map; -import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIterator; +import com.netflix.hollow.api.sampling.HollowMapSampler; +import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIteratorImpl; /** * A key/value entry iterator over a map record backed by a single validated snapshot of its shard and bucket @@ -29,13 +30,15 @@ * nothing. *

* Correctness mirrors the list/set snapshot iterators: {@code startBucket}/{@code endBucket} are validated - * non-torn before use, so every relative bucket index in {@code [0, numBuckets)} maps to an in-bounds read of - * the shard's bucket region (garbage-but-safe if recycled after a concurrent delta, never out-of-bounds). Each - * {@link #next()} scans forward to the next occupied bucket reading from the snapshot, then re-validates shard - * identity via the {@code loadFence} in {@link HollowMapTypeReadState#readWasUnsafe}; if the shard changed, the - * scan is discarded and re-run against a freshly established snapshot before any entry is surfaced. + * before use, so every relative bucket index in {@code [0, numBuckets)} maps to an in-bounds read from the + * captured shard. Each {@link #next()} scans to the next occupied bucket, then re-validates shard identity. If + * an on-heap update or reshard replaced it, the scan is discarded and repeated against a fresh snapshot before + * an entry is returned. + *

+ * When retired on-heap arrays are not recycled, the captured shard remains immutable and strongly reachable. + * In that case {@code readWasUnsafe} is a no-op, so the iterator retains its initial snapshot without fences. */ -public class HollowMapSnapshotEntryOrdinalIterator implements HollowMapEntryOrdinalIterator { +final class HollowMapSnapshotEntryOrdinalIterator extends HollowMapEntryOrdinalIteratorImpl { private final HollowMapTypeReadState readState; private final int ordinal; @@ -49,9 +52,11 @@ public class HollowMapSnapshotEntryOrdinalIterator implements HollowMapEntryOrdi private int key = -1; private int value = -1; - public HollowMapSnapshotEntryOrdinalIterator(int ordinal, HollowMapTypeReadState readState) { + HollowMapSnapshotEntryOrdinalIterator( + int ordinal, HollowMapTypeReadState readState, HollowMapSampler sampler) { this.readState = readState; this.ordinal = ordinal; + sampler.recordSize(); snapshot(); } @@ -107,7 +112,7 @@ public boolean next() { return true; } - // A delta or re-shard invalidated the snapshot mid-scan; re-establish and rescan from currentBucket. + // The snapshot changed mid-scan; re-establish it and rescan from currentBucket. snapshot(); } } @@ -126,6 +131,7 @@ public int getValue() { * @return the relative bucket position (within this map's bucket range) that the last entry returned by * {@link #next()} was retrieved from. */ + @Override public int getCurrentBucket() { return currentBucket; } diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java index 72b8a4371f..6c08646931 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java @@ -43,6 +43,7 @@ import com.netflix.hollow.core.read.filter.HollowFilterConfig; import com.netflix.hollow.core.read.iterator.EmptyMapOrdinalIterator; import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIteratorImpl; import com.netflix.hollow.core.schema.HollowMapSchema; import com.netflix.hollow.core.schema.HollowObjectSchema.FieldType; import com.netflix.hollow.core.schema.HollowSchema; @@ -368,7 +369,9 @@ public HollowMapEntryOrdinalIterator ordinalIterator(int ordinal) { if(size(ordinal) == 0) return EmptyMapOrdinalIterator.INSTANCE; - return new HollowMapSnapshotEntryOrdinalIterator(ordinal, this); + if(stateEngine == null || !stateEngine.isSnapshotCollectionIteratorsEnabled()) + return new HollowMapEntryOrdinalIteratorImpl(ordinal, this); + return new HollowMapSnapshotEntryOrdinalIterator(ordinal, this, sampler); } @Override diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIterator.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIterator.java index f4eac8fe12..34ec62424f 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIterator.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIterator.java @@ -16,7 +16,8 @@ */ package com.netflix.hollow.core.read.engine.set; -import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.api.sampling.HollowSetSampler; +import com.netflix.hollow.core.read.iterator.HollowSetOrdinalIterator; /** * An ordinal iterator over a set record backed by a single validated snapshot of its shard and bucket range. @@ -27,15 +28,16 @@ * at construction and then executes a single fence per {@link #next()} (one per returned element), allocating * nothing. *

- * Correctness mirrors the list snapshot iterator: {@code startBucket}/{@code endBucket} are validated non-torn - * before use, so every relative bucket index in {@code [0, numBuckets)} maps to an in-bounds read of the shard's - * bucket region (garbage-but-safe if the shard's backing arrays were recycled after a concurrent delta, never - * out-of-bounds). Each {@link #next()} scans forward to the next non-empty bucket reading from the snapshot, - * then re-validates shard identity via the {@code loadFence} in - * {@link HollowSetTypeReadState#readWasUnsafe}; if the shard changed, the scan is discarded and re-run against a - * freshly established snapshot before any value is returned. + * Correctness mirrors the list snapshot iterator: {@code startBucket}/{@code endBucket} are validated before + * use, so every relative bucket index in {@code [0, numBuckets)} maps to an in-bounds read from the captured + * shard. Each {@link #next()} scans to the next non-empty bucket, then re-validates shard identity. If an on-heap + * update or reshard replaced it, the scan is discarded and repeated against a fresh snapshot before a value is + * returned. + *

+ * When retired on-heap arrays are not recycled, the captured shard remains immutable and strongly reachable. + * In that case {@code readWasUnsafe} is a no-op, so the iterator retains its initial snapshot without fences. */ -public class HollowSetSnapshotOrdinalIterator implements HollowOrdinalIterator { +final class HollowSetSnapshotOrdinalIterator extends HollowSetOrdinalIterator { private final HollowSetTypeReadState readState; private final int ordinal; @@ -47,9 +49,11 @@ public class HollowSetSnapshotOrdinalIterator implements HollowOrdinalIterator { private int currentBucket = -1; - public HollowSetSnapshotOrdinalIterator(int ordinal, HollowSetTypeReadState readState) { + HollowSetSnapshotOrdinalIterator( + int ordinal, HollowSetTypeReadState readState, HollowSetSampler sampler) { this.readState = readState; this.ordinal = ordinal; + sampler.recordSize(); snapshot(); } @@ -99,7 +103,7 @@ public int next() { return end ? NO_MORE_ORDINALS : bucketValue; } - // A delta or re-shard invalidated the snapshot mid-scan; re-establish and rescan from currentBucket. + // The snapshot changed mid-scan; re-establish it and rescan from currentBucket. snapshot(); } } @@ -108,6 +112,7 @@ public int next() { * @return the relative bucket position (within this set's bucket range) that the last ordinal returned by * {@link #next()} was retrieved from. */ + @Override public int getCurrentBucket() { return currentBucket; } diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java index 1ad7f19650..943139f565 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java @@ -44,6 +44,7 @@ import com.netflix.hollow.core.read.filter.HollowFilterConfig; import com.netflix.hollow.core.read.iterator.EmptyOrdinalIterator; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowSetOrdinalIterator; import com.netflix.hollow.core.schema.HollowObjectSchema.FieldType; import com.netflix.hollow.core.schema.HollowSchema; import com.netflix.hollow.core.schema.HollowSetSchema; @@ -330,7 +331,9 @@ public HollowOrdinalIterator ordinalIterator(int ordinal) { sampler.recordIterator(); if(size(ordinal) == 0) return EmptyOrdinalIterator.INSTANCE; - return new HollowSetSnapshotOrdinalIterator(ordinal, this); + if(stateEngine == null || !stateEngine.isSnapshotCollectionIteratorsEnabled()) + return new HollowSetOrdinalIterator(ordinal, this); + return new HollowSetSnapshotOrdinalIterator(ordinal, this, sampler); } boolean readWasUnsafe(HollowSetTypeShardsHolder shardsHolder, int ordinal, HollowSetTypeReadStateShard shard) { diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowListOrdinalIterator.java b/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowListOrdinalIterator.java index 2ca3d3b023..d5e14bddec 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowListOrdinalIterator.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowListOrdinalIterator.java @@ -26,9 +26,17 @@ public class HollowListOrdinalIterator implements HollowOrdinalIterator { private int currentElement; public HollowListOrdinalIterator(int listOrdinal, HollowListTypeDataAccess dataAccess) { + this(listOrdinal, dataAccess, dataAccess.size(listOrdinal)); + } + + protected HollowListOrdinalIterator() { + this(-1, null, 0); + } + + private HollowListOrdinalIterator(int listOrdinal, HollowListTypeDataAccess dataAccess, int size) { this.listOrdinal = listOrdinal; this.dataAccess = dataAccess; - this.size = dataAccess.size(listOrdinal); + this.size = size; } @Override diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowMapEntryOrdinalIteratorImpl.java b/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowMapEntryOrdinalIteratorImpl.java index 99debfd2ae..f97992effc 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowMapEntryOrdinalIteratorImpl.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowMapEntryOrdinalIteratorImpl.java @@ -30,9 +30,18 @@ public class HollowMapEntryOrdinalIteratorImpl implements HollowMapEntryOrdinalI private int value; public HollowMapEntryOrdinalIteratorImpl(int mapOrdinal, HollowMapTypeDataAccess dataAccess) { + this(mapOrdinal, dataAccess, HashCodes.hashTableSize(dataAccess.size(mapOrdinal))); + } + + protected HollowMapEntryOrdinalIteratorImpl() { + this(-1, null, 0); + } + + private HollowMapEntryOrdinalIteratorImpl( + int mapOrdinal, HollowMapTypeDataAccess dataAccess, int numBuckets) { this.mapOrdinal = mapOrdinal; this.dataAccess = dataAccess; - this.numBuckets = HashCodes.hashTableSize(dataAccess.size(mapOrdinal)); + this.numBuckets = numBuckets; } @Override diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowSetOrdinalIterator.java b/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowSetOrdinalIterator.java index 7fb5463c29..b17f1b4245 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowSetOrdinalIterator.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowSetOrdinalIterator.java @@ -30,9 +30,17 @@ public class HollowSetOrdinalIterator implements HollowOrdinalIterator { private int currentBucket = -1; public HollowSetOrdinalIterator(int setOrdinal, HollowSetTypeDataAccess dataAccess) { + this(setOrdinal, dataAccess, HashCodes.hashTableSize(dataAccess.size(setOrdinal))); + } + + protected HollowSetOrdinalIterator() { + this(-1, null, 0); + } + + private HollowSetOrdinalIterator(int setOrdinal, HollowSetTypeDataAccess dataAccess, int numBuckets) { this.setOrdinal = setOrdinal; this.dataAccess = dataAccess; - this.numBuckets = HashCodes.hashTableSize(dataAccess.size(setOrdinal)); + this.numBuckets = numBuckets; } @Override diff --git a/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowMapCopier.java b/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowMapCopier.java index c78e23287f..0fe2731c0e 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowMapCopier.java +++ b/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowMapCopier.java @@ -21,7 +21,7 @@ import com.netflix.hollow.core.read.engine.map.HollowMapTypeReadState; import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIterator; -import com.netflix.hollow.core.read.engine.map.HollowMapSnapshotEntryOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIteratorImpl; import com.netflix.hollow.core.write.HollowMapWriteRecord; import com.netflix.hollow.core.write.HollowWriteRecord; import com.netflix.hollow.tools.combine.OrdinalRemapper; @@ -44,7 +44,7 @@ public HollowWriteRecord copy(int ordinal) { while(iter.next()) { int remappedKeyOrdinal = ordinalRemapper.getMappedOrdinal(keyType, iter.getKey()); int remappedValueOrdinal = ordinalRemapper.getMappedOrdinal(valueType, iter.getValue()); - int hashCode = preserveHashPositions ? ((HollowMapSnapshotEntryOrdinalIterator)iter).getCurrentBucket() : remappedKeyOrdinal; + int hashCode = preserveHashPositions ? ((HollowMapEntryOrdinalIteratorImpl)iter).getCurrentBucket() : remappedKeyOrdinal; rec.addEntry(remappedKeyOrdinal, remappedValueOrdinal, hashCode); } diff --git a/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowSetCopier.java b/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowSetCopier.java index 5588bbf313..261127adb5 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowSetCopier.java +++ b/hollow/src/main/java/com/netflix/hollow/core/write/copy/HollowSetCopier.java @@ -21,7 +21,7 @@ import com.netflix.hollow.core.read.engine.set.HollowSetTypeReadState; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; -import com.netflix.hollow.core.read.engine.set.HollowSetSnapshotOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowSetOrdinalIterator; import com.netflix.hollow.core.write.HollowSetWriteRecord; import com.netflix.hollow.core.write.HollowWriteRecord; import com.netflix.hollow.tools.combine.OrdinalRemapper; @@ -45,7 +45,7 @@ public HollowWriteRecord copy(int ordinal) { while(elementOrdinal != HollowOrdinalIterator.NO_MORE_ORDINALS) { int remappedElementOrdinal = ordinalRemapper.getMappedOrdinal(elementType, elementOrdinal); - int hashCode = preserveHashPositions ? ((HollowSetSnapshotOrdinalIterator)ordinalIterator).getCurrentBucket() : remappedElementOrdinal; + int hashCode = preserveHashPositions ? ((HollowSetOrdinalIterator)ordinalIterator).getCurrentBucket() : remappedElementOrdinal; rec.addElement(remappedElementOrdinal, hashCode); elementOrdinal = ordinalIterator.next(); } diff --git a/hollow/src/test/java/com/netflix/hollow/api/consumer/HollowConsumerBuilderTest.java b/hollow/src/test/java/com/netflix/hollow/api/consumer/HollowConsumerBuilderTest.java index c00adc7645..0f0c49fd53 100644 --- a/hollow/src/test/java/com/netflix/hollow/api/consumer/HollowConsumerBuilderTest.java +++ b/hollow/src/test/java/com/netflix/hollow/api/consumer/HollowConsumerBuilderTest.java @@ -19,6 +19,29 @@ public class HollowConsumerBuilderTest { + @Test + public void snapshotCollectionIteratorsAreOptIn() throws IOException { + TestBlobRetriever blobRetriever = new TestBlobRetriever(); + TestHollowConsumer consumer = new TestHollowConsumer.Builder() + .withBlobRetriever(blobRetriever) + .build(); + consumer.addSnapshot(1L, new HollowWriteStateEngineBuilder() + .add(new com.netflix.hollow.test.model.Movie(1, "movie", 2026)) + .build()); + consumer.triggerRefreshTo(1L); + assertFalse(consumer.getStateEngine().isSnapshotCollectionIteratorsEnabled()); + + TestHollowConsumer enabledConsumer = new TestHollowConsumer.Builder() + .withBlobRetriever(new TestBlobRetriever()) + .withSnapshotCollectionIterators() + .build(); + enabledConsumer.addSnapshot(1L, new HollowWriteStateEngineBuilder() + .add(new com.netflix.hollow.test.model.Movie(1, "movie", 2026)) + .build()); + enabledConsumer.triggerRefreshTo(1L); + assertTrue(enabledConsumer.getStateEngine().isSnapshotCollectionIteratorsEnabled()); + } + @Test public void testCachedTypes() throws IOException { TestHollowConsumer consumer = new TestHollowConsumer.Builder() diff --git a/hollow/src/test/java/com/netflix/hollow/api/objects/HollowListIteratorTest.java b/hollow/src/test/java/com/netflix/hollow/api/objects/HollowListIteratorTest.java new file mode 100644 index 0000000000..0dcc2a5065 --- /dev/null +++ b/hollow/src/test/java/com/netflix/hollow/api/objects/HollowListIteratorTest.java @@ -0,0 +1,68 @@ +package com.netflix.hollow.api.objects; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.netflix.hollow.api.objects.delegate.HollowListCachedDelegate; +import com.netflix.hollow.api.objects.delegate.HollowListDelegate; +import com.netflix.hollow.api.objects.delegate.HollowListLookupDelegate; +import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import java.util.Iterator; +import org.junit.Test; + +public class HollowListIteratorTest { + + @Test + public void iteratesUsingOrdinalIterator() { + HollowListTypeDataAccess dataAccess = mock(HollowListTypeDataAccess.class); + HollowOrdinalIterator ordinalIterator = mock(HollowOrdinalIterator.class); + when(dataAccess.ordinalIterator(7)).thenReturn(ordinalIterator); + when(ordinalIterator.next()).thenReturn(3, 5, HollowOrdinalIterator.NO_MORE_ORDINALS); + + HollowList list = list(new HollowListLookupDelegate<>(dataAccess)); + + Iterator iterator = list.iterator(); + assertTrue(iterator.hasNext()); + assertTrue(iterator.hasNext()); + assertEquals(Integer.valueOf(3), iterator.next()); + assertEquals(Integer.valueOf(5), iterator.next()); + assertFalse(iterator.hasNext()); + verify(dataAccess).ordinalIterator(7); + } + + @Test + public void cachedListIteratesCachedOrdinals() { + HollowListTypeDataAccess dataAccess = mock(HollowListTypeDataAccess.class); + when(dataAccess.size(7)).thenReturn(2); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); + when(dataAccess.getElementOrdinal(7, 1)).thenReturn(5); + + HollowList list = list(new HollowListCachedDelegate<>(dataAccess, 7)); + + Iterator iterator = list.iterator(); + assertEquals(Integer.valueOf(3), iterator.next()); + assertEquals(Integer.valueOf(5), iterator.next()); + assertFalse(iterator.hasNext()); + verify(dataAccess, never()).ordinalIterator(7); + } + + private HollowList list(HollowListDelegate delegate) { + return new HollowList(delegate, 7) { + @Override + public Integer instantiateElement(int elementOrdinal) { + return elementOrdinal; + } + + @Override + public boolean equalsElement(int elementOrdinal, Object testObject) { + return Integer.valueOf(elementOrdinal).equals(testObject); + } + }; + } +} diff --git a/hollow/src/test/java/com/netflix/hollow/api/perfapi/HollowPerfBackedListTest.java b/hollow/src/test/java/com/netflix/hollow/api/perfapi/HollowPerfBackedListTest.java new file mode 100644 index 0000000000..d0550d90b3 --- /dev/null +++ b/hollow/src/test/java/com/netflix/hollow/api/perfapi/HollowPerfBackedListTest.java @@ -0,0 +1,36 @@ +package com.netflix.hollow.api.perfapi; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import java.util.Iterator; +import org.junit.Test; + +public class HollowPerfBackedListTest { + + @Test + public void iteratesUsingOrdinalIterator() { + HollowListTypePerfAPI typeAPI = mock(HollowListTypePerfAPI.class); + HollowListTypeDataAccess dataAccess = mock(HollowListTypeDataAccess.class); + HollowOrdinalIterator ordinalIterator = mock(HollowOrdinalIterator.class); + when(typeAPI.typeAccess()).thenReturn(dataAccess); + when(dataAccess.ordinalIterator(7)).thenReturn(ordinalIterator); + when(ordinalIterator.next()).thenReturn(3, 5, HollowOrdinalIterator.NO_MORE_ORDINALS); + + HollowPerfBackedList list = new HollowPerfBackedList<>(typeAPI, 7, ref -> ref); + + Iterator iterator = list.iterator(); + assertTrue(iterator.hasNext()); + assertTrue(iterator.hasNext()); + assertEquals(Long.valueOf(3), iterator.next()); + assertEquals(Long.valueOf(5), iterator.next()); + assertFalse(iterator.hasNext()); + verify(dataAccess).ordinalIterator(7); + } +} diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java index e18b914d77..55a156242a 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java @@ -1,8 +1,15 @@ package com.netflix.hollow.core.read.engine.list; import static junit.framework.TestCase.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import com.netflix.hollow.core.memory.pool.WastefulRecycler; +import com.netflix.hollow.core.read.engine.HollowReadStateEngine; import com.netflix.hollow.core.read.engine.HollowTypeReshardingStrategy; +import com.netflix.hollow.core.read.iterator.HollowListOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.core.util.StateEngineRoundTripper; import com.netflix.hollow.core.write.HollowListTypeWriteState; import com.netflix.hollow.core.write.HollowObjectTypeWriteState; import com.netflix.hollow.core.write.HollowWriteStateEngine; @@ -11,6 +18,83 @@ public class HollowListTypeReadStateTest extends AbstractHollowListTypeDataElementsSplitJoinTest { + @Test + public void usesExistingIteratorByDefault() throws Exception { + populateWriteStateEngine(3); + populateWriteStateEngineWithListRecords(new int[][] {{0, 1, 2}}); + + readStateEngine = new HollowReadStateEngine(WastefulRecycler.DEFAULT_INSTANCE); + StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); + + HollowListTypeReadState typeState = + (HollowListTypeReadState) readStateEngine.getTypeState("TestList"); + HollowOrdinalIterator iterator = typeState.ordinalIterator(0); + + assertTrue(iterator instanceof HollowListOrdinalIterator); + assertFalse(iterator instanceof HollowListSnapshotOrdinalIterator); + } + + @Test + public void usesCompatibleSnapshotIteratorWhenEnabled() throws Exception { + populateWriteStateEngine(3); + populateWriteStateEngineWithListRecords(new int[][] {{0, 1, 2}}); + + readStateEngine = new HollowReadStateEngine(WastefulRecycler.DEFAULT_INSTANCE); + readStateEngine.setSnapshotCollectionIterators(true); + StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); + + HollowListTypeReadState typeState = + (HollowListTypeReadState) readStateEngine.getTypeState("TestList"); + HollowOrdinalIterator iterator = typeState.ordinalIterator(0); + + assertTrue(iterator instanceof HollowListSnapshotOrdinalIterator); + assertTrue(iterator instanceof HollowListOrdinalIterator); + } + + @Test + public void iteratorRetainsImmutableShardAcrossRefresh() throws Exception { + populateWriteStateEngine(3); + populateWriteStateEngineWithListRecords(new int[][] {{0, 1, 2}}); + + readStateEngine = new HollowReadStateEngine(WastefulRecycler.DEFAULT_INSTANCE); + readStateEngine.setSnapshotCollectionIterators(true); + StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); + + HollowListTypeReadState typeState = + (HollowListTypeReadState) readStateEngine.getTypeState("TestList"); + HollowOrdinalIterator iterator = typeState.ordinalIterator(0); + assertEquals(0, iterator.next()); + + populateWriteStateEngine(writeStateEngine, schema, 3); + populateWriteStateEngineWithListRecords(new int[][] {{2}}); + roundTripDelta(); + + assertEquals(1, iterator.next()); + assertEquals(2, iterator.next()); + assertEquals(HollowOrdinalIterator.NO_MORE_ORDINALS, iterator.next()); + } + + @Test + public void iteratorRefreshesRecycledShardAcrossRefresh() throws Exception { + populateWriteStateEngine(3); + populateWriteStateEngineWithListRecords(new int[][] {{0, 1, 2}, {0}}); + roundTripSnapshot(); + readStateEngine.setSnapshotCollectionIterators(true); + + HollowListTypeReadState typeState = + (HollowListTypeReadState) readStateEngine.getTypeState("TestList"); + HollowOrdinalIterator iterator = typeState.ordinalIterator(0); + assertEquals(0, iterator.next()); + + populateWriteStateEngine(writeStateEngine, schema, 3); + populateWriteStateEngineWithListRecords(new int[][] {{0, 1, 2}, {1}}); + roundTripDelta(); + + assertEquals(1, iterator.next()); + assertEquals(2, iterator.next()); + assertEquals(HollowOrdinalIterator.NO_MORE_ORDINALS, iterator.next()); + } + @Test public void testApproximateHoleCostWithShards() throws Exception { // Setup with 2 shards diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIteratorTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIteratorTest.java index 734363ea96..8bf12e580c 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIteratorTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIteratorTest.java @@ -5,6 +5,7 @@ import com.netflix.hollow.core.read.engine.HollowTypeReshardingStrategy; import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIteratorImpl; import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -43,6 +44,10 @@ public void iteratesMapEntries() throws IOException { int numRecords = 200; int[][][] mapContents = generateMapContents(numRecords); HollowMapTypeReadState readState = populateTypeStateWith(mapContents); + readStateEngine.setSnapshotCollectionIterators(true); + HollowMapEntryOrdinalIterator iterator = readState.ordinalIterator(0); + assertTrue(iterator instanceof HollowMapSnapshotEntryOrdinalIterator); + assertTrue(iterator instanceof HollowMapEntryOrdinalIteratorImpl); boolean sawEntries = false; for (int ordinal = 0; ordinal <= readState.maxOrdinal(); ordinal++) { @@ -56,6 +61,7 @@ public void iterateConcurrentlyWhileResharding() throws Exception { int numRecords = 500; int[][][] mapContents = generateMapContents(numRecords); final HollowMapTypeReadState readState = populateTypeStateWith(mapContents); + readStateEngine.setSnapshotCollectionIterators(true); final HollowTypeReshardingStrategy reshardingStrategy = HollowTypeReshardingStrategy.getInstance(readState); final int numReaders = 6; diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIteratorTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIteratorTest.java index c577a1ba53..8d53318b95 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIteratorTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIteratorTest.java @@ -5,6 +5,7 @@ import com.netflix.hollow.core.read.engine.HollowTypeReshardingStrategy; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowSetOrdinalIterator; import com.netflix.hollow.core.util.IntList; import java.io.IOException; import java.util.ArrayList; @@ -55,6 +56,10 @@ public void iteratesEmptyAndSingleAndMultiElementSets() throws IOException { int numRecords = 50; int[][] setContents = generateSetContents(numRecords); HollowSetTypeReadState readState = populateTypeStateWith(setContents); + readStateEngine.setSnapshotCollectionIterators(true); + HollowOrdinalIterator iterator = readState.ordinalIterator(0); + assertTrue(iterator instanceof HollowSetSnapshotOrdinalIterator); + assertTrue(iterator instanceof HollowSetOrdinalIterator); boolean sawSizeOne = false; boolean sawLarge = false; @@ -73,6 +78,7 @@ public void iterateConcurrentlyWhileResharding() throws Exception { int numRecords = 500; int[][] setContents = generateSetContents(numRecords); final HollowSetTypeReadState readState = populateTypeStateWith(setContents); + readStateEngine.setSnapshotCollectionIterators(true); final HollowTypeReshardingStrategy reshardingStrategy = HollowTypeReshardingStrategy.getInstance(readState); final int numReaders = 6; diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/iterator/HollowListSnapshotOrdinalIteratorTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/iterator/HollowListSnapshotOrdinalIteratorTest.java index d9a7ad6566..95318129eb 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/read/iterator/HollowListSnapshotOrdinalIteratorTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/read/iterator/HollowListSnapshotOrdinalIteratorTest.java @@ -55,6 +55,7 @@ public void iteratesEmptyAndSingleAndMultiElementLists() throws IOException { int numRecords = 50; int[][] listContents = generateListContents(numRecords); HollowListTypeReadState readState = populateTypeStateWith(listContents); + readStateEngine.setSnapshotCollectionIterators(true); boolean sawSizeOne = false; boolean sawLarge = false; @@ -71,6 +72,7 @@ public void iteratesEmptyAndSingleAndMultiElementLists() throws IOException { @Test public void iteratorExhaustsCleanlyAndRepeatsNoMoreOrdinals() throws IOException { HollowListTypeReadState readState = populateTypeStateWith(generateListContents(10)); + readStateEngine.setSnapshotCollectionIterators(true); HollowOrdinalIterator iter = readState.ordinalIterator(0); int count = 0; while (iter.next() != HollowOrdinalIterator.NO_MORE_ORDINALS) { @@ -85,6 +87,7 @@ public void iteratorExhaustsCleanlyAndRepeatsNoMoreOrdinals() throws IOException @Test public void iteratorMatchesPerElementReads() throws IOException { HollowListTypeReadState readState = populateTypeStateWith(generateListContents(100)); + readStateEngine.setSnapshotCollectionIterators(true); for (int ordinal = 0; ordinal <= readState.maxOrdinal(); ordinal++) { int size = readState.size(ordinal); IntList iterated = drain(readState, ordinal); @@ -101,6 +104,7 @@ public void iterateConcurrentlyWhileResharding() throws Exception { int numRecords = 1000; int[][] listContents = generateListContents(numRecords); final HollowListTypeReadState readState = populateTypeStateWith(listContents); + readStateEngine.setSnapshotCollectionIterators(true); final HollowTypeReshardingStrategy reshardingStrategy = HollowTypeReshardingStrategy.getInstance(readState); final int numReaders = 6; From f4dc405bc315e3104e9624caef53ca4eb4603f78 Mon Sep 17 00:00:00 2001 From: Danny Thomas Date: Sun, 4 Oct 2026 21:25:13 +1100 Subject: [PATCH 3/6] Share captured collection traversal and gate shard cursors --- ...owListTypeReadStateIterationBenchmark.java | 4 +- ...lowMapTypeReadStateIterationBenchmark.java | 4 +- ...lowSetTypeReadStateIterationBenchmark.java | 4 +- .../api/client/HollowClientUpdater.java | 10 +- .../hollow/api/consumer/HollowConsumer.java | 15 +- .../hollow/api/objects/HollowList.java | 47 ++-- .../api/objects/HollowListTraversal.java | 211 +++++++++++++++++ .../delegate/HollowListCachedDelegate.java | 31 ++- .../objects/delegate/HollowListDelegate.java | 8 + .../delegate/HollowListLookupDelegate.java | 11 + .../api/perfapi/HollowPerfBackedList.java | 57 +++-- .../read/engine/HollowReadStateEngine.java | 12 +- .../HollowListSnapshotOrdinalIterator.java | 59 +++-- .../engine/list/HollowListTypeReadState.java | 2 +- ...HollowMapSnapshotEntryOrdinalIterator.java | 46 +++- .../engine/map/HollowMapTypeReadState.java | 6 +- .../set/HollowSetSnapshotOrdinalIterator.java | 46 +++- .../engine/set/HollowSetTypeReadState.java | 6 +- .../iterator/HollowListOrdinalIterator.java | 28 ++- .../consumer/HollowConsumerBuilderTest.java | 13 +- .../api/objects/HollowListIteratorTest.java | 212 +++++++++++++++++- .../api/perfapi/HollowPerfBackedListTest.java | 112 ++++++++- .../list/HollowListTraversalSnapshotTest.java | 185 +++++++++++++++ .../list/HollowListTypeReadStateTest.java | 28 ++- ...owMapSnapshotEntryOrdinalIteratorTest.java | 41 +++- .../HollowSetSnapshotOrdinalIteratorTest.java | 37 ++- ...HollowListSnapshotOrdinalIteratorTest.java | 8 +- 27 files changed, 1108 insertions(+), 135 deletions(-) create mode 100644 hollow/src/main/java/com/netflix/hollow/api/objects/HollowListTraversal.java create mode 100644 hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTraversalSnapshotTest.java diff --git a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateIterationBenchmark.java b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateIterationBenchmark.java index e6a04efa3d..08acc8b4c8 100644 --- a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateIterationBenchmark.java +++ b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateIterationBenchmark.java @@ -27,7 +27,7 @@ import org.openjdk.jmh.infra.Blackhole; /** - * Compares list iteration cost of the per-element read path (the historical {@code HollowListOrdinalIterator} + * Compares list iteration cost of the per-element read path (the generic {@code HollowListOrdinalIterator} * pattern: {@code size()} + {@code getElementOrdinal(ordinal, i)} per element, i.e. 2N+1 * {@code Unsafe.loadFence()}s when arrays can be recycled) * against the single-snapshot path ({@code ordinalIterator()} backed by a shard cursor). With recycling, the @@ -76,7 +76,7 @@ public void setUp() throws IOException { ? new RecyclingRecycler() : WastefulRecycler.DEFAULT_INSTANCE; HollowReadStateEngine readStateEngine = new HollowReadStateEngine(memoryRecycler); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); listTypeState = null; diff --git a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadStateIterationBenchmark.java b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadStateIterationBenchmark.java index ac24074902..1b7e62a17a 100644 --- a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadStateIterationBenchmark.java +++ b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadStateIterationBenchmark.java @@ -28,7 +28,7 @@ import org.openjdk.jmh.infra.Blackhole; /** - * Compares map entry iteration cost of the per-bucket read path (the historical + * Compares map entry iteration cost of the per-bucket read path (the generic * {@code HollowMapEntryOrdinalIteratorImpl} pattern: {@code size()} + {@code relativeBucket(ordinal, bucket)} * over every hash bucket, i.e. two fences per bucket when arrays can be recycled) against the single-snapshot * entry iterator. With recycling, the cursor validates once per surfaced entry; without recycling, its captured @@ -76,7 +76,7 @@ public void setUp() throws IOException { ? new RecyclingRecycler() : WastefulRecycler.DEFAULT_INSTANCE; HollowReadStateEngine readStateEngine = new HollowReadStateEngine(memoryRecycler); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); mapTypeState = null; diff --git a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadStateIterationBenchmark.java b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadStateIterationBenchmark.java index a6342104af..7ac8142e7a 100644 --- a/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadStateIterationBenchmark.java +++ b/hollow-perf/src/jmh/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadStateIterationBenchmark.java @@ -30,7 +30,7 @@ import org.openjdk.jmh.infra.Blackhole; /** - * Compares set iteration cost of the per-bucket read path (the historical {@code HollowSetOrdinalIterator} + * Compares set iteration cost of the per-bucket read path (the generic {@code HollowSetOrdinalIterator} * pattern: {@code size()} + {@code relativeBucketValue(ordinal, bucket)} over every hash bucket, i.e. two fences * per bucket when arrays can be recycled) against the single-snapshot iterator. With recycling, the cursor * validates once per surfaced element; without recycling, its captured shard is immutable and requires no @@ -78,7 +78,7 @@ public void setUp() throws IOException { ? new RecyclingRecycler() : WastefulRecycler.DEFAULT_INSTANCE; HollowReadStateEngine readStateEngine = new HollowReadStateEngine(memoryRecycler); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); setTypeState = null; diff --git a/hollow/src/main/java/com/netflix/hollow/api/client/HollowClientUpdater.java b/hollow/src/main/java/com/netflix/hollow/api/client/HollowClientUpdater.java index 1fb11227dd..e26aa19a16 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/client/HollowClientUpdater.java +++ b/hollow/src/main/java/com/netflix/hollow/api/client/HollowClientUpdater.java @@ -66,7 +66,7 @@ public class HollowClientUpdater { private boolean skipTypeShardUpdateWithNoAdditions; private ExperimentalFeature[] experimentalFeatures = new ExperimentalFeature[0]; private MemoryRecyclingMode memoryRecyclingMode = MemoryRecyclingMode.AUTO; - private boolean snapshotCollectionIterators; + private boolean shardCursorIterators; private TypeFilter filter; @@ -136,11 +136,11 @@ public void setSkipShardUpdateWithNoAdditions(boolean skipTypeShardUpdateWithNoA dataHolder.getStateEngine().setSkipTypeShardUpdateWithNoAdditions(skipTypeShardUpdateWithNoAdditions); } - public void setSnapshotCollectionIterators(boolean snapshotCollectionIterators) { - this.snapshotCollectionIterators = snapshotCollectionIterators; + public void setShardCursorIterators(boolean shardCursorIterators) { + this.shardCursorIterators = shardCursorIterators; HollowDataHolder dataHolder = hollowDataHolderVolatile; if(dataHolder != null) - dataHolder.getStateEngine().setSnapshotCollectionIterators(snapshotCollectionIterators); + dataHolder.getStateEngine().setShardCursorIterators(shardCursorIterators); } /** @@ -367,7 +367,7 @@ private HollowReadStateEngine newStateEngine() { stateEngine = new HollowReadStateEngine(hashCodeFinder, true, new HollowReadConfiguration(memoryMode, memoryRecyclingMode.createRecycler(), experimentalFeatures)); } - stateEngine.setSnapshotCollectionIterators(snapshotCollectionIterators); + stateEngine.setShardCursorIterators(shardCursorIterators); return stateEngine; } diff --git a/hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java b/hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java index e889cc9eda..e4bdc12caf 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java +++ b/hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java @@ -210,8 +210,8 @@ protected > HollowConsumer(B builder) { updater.setMemoryRecyclingMode(builder.memoryRecyclingMode); if(builder.skipTypeShardUpdateWithNoAdditions) updater.setSkipShardUpdateWithNoAdditions(true); - if(builder.snapshotCollectionIterators) - updater.setSnapshotCollectionIterators(true); + if(builder.shardCursorIterators) + updater.setShardCursorIterators(true); this.announcementWatcher = builder.announcementWatcher; this.refreshExecutor = builder.refreshExecutor; this.refreshLock = new ReentrantReadWriteLock(); @@ -1167,7 +1167,7 @@ public static class Builder> { protected HollowMetricsCollector metricsCollector; protected boolean skipTypeShardUpdateWithNoAdditions = false; protected final EnumSet experimentalFeatures = EnumSet.noneOf(ExperimentalFeature.class); - protected boolean snapshotCollectionIterators = false; + protected boolean shardCursorIterators = false; public B withBlobRetriever(HollowConsumer.BlobRetriever blobRetriever) { this.blobRetriever = blobRetriever; @@ -1482,11 +1482,12 @@ public B withSkipTypeShardUpdateWithNoAdditions() { } /** - * Experimental: Use snapshot iterators for collection types. Snapshot iterators capture a - * collection's shard and bounds once rather than resolving them for every element or bucket. + * Experimental. Enable shard cursor iterators for collection types and Java list traversal. + * Shard cursors capture a collection's shard and bounds once rather than resolving them for + * every element or bucket. Existing get-based Java list traversal remains the default. */ - public B withSnapshotCollectionIterators() { - this.snapshotCollectionIterators = true; + public B withShardCursorIterators() { + this.shardCursorIterators = true; return (B)this; } diff --git a/hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java b/hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java index e152cdc330..1c5edd4cc1 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java +++ b/hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java @@ -19,11 +19,12 @@ import com.netflix.hollow.api.objects.delegate.HollowListDelegate; import com.netflix.hollow.api.objects.delegate.HollowRecordDelegate; import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; -import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowListOrdinalIterator; import com.netflix.hollow.core.schema.HollowListSchema; import java.util.AbstractList; import java.util.Iterator; -import java.util.NoSuchElementException; +import java.util.ListIterator; +import java.util.Spliterator; /** * A HollowList provides an implementation of the {@link java.util.List} interface over @@ -56,29 +57,27 @@ public final T get(int index) { @Override public Iterator iterator() { - HollowOrdinalIterator ordinalIterator = delegate.iterator(ordinal); - return new Iterator() { - private int nextOrdinal = HollowOrdinalIterator.NO_MORE_ORDINALS; - private boolean nextOrdinalLoaded; - - @Override - public boolean hasNext() { - if(!nextOrdinalLoaded) { - nextOrdinal = ordinalIterator.next(); - nextOrdinalLoaded = true; - } - return nextOrdinal != HollowOrdinalIterator.NO_MORE_ORDINALS; - } + if(!HollowListTraversal.isEnabled(delegate.getTypeDataAccess())) + return super.iterator(); + return HollowListTraversal.iterator(delegate.iterator(ordinal), this::instantiateElement); + } - @Override - public T next() { - if(!hasNext()) - throw new NoSuchElementException(); - int currentOrdinal = nextOrdinal; - nextOrdinalLoaded = false; - return instantiateElement(currentOrdinal); - } - }; + @Override + public ListIterator listIterator(int index) { + if(!HollowListTraversal.isEnabled(delegate.getTypeDataAccess())) + return super.listIterator(index); + HollowListOrdinalIterator cursor = delegate.ordinalCursor(ordinal); + return cursor == null ? super.listIterator(index) + : HollowListTraversal.listIterator(cursor, index, this::instantiateElement); + } + + @Override + public Spliterator spliterator() { + if(!HollowListTraversal.isEnabled(delegate.getTypeDataAccess())) + return super.spliterator(); + HollowListOrdinalIterator cursor = delegate.ordinalCursor(ordinal); + return cursor == null ? super.spliterator() + : HollowListTraversal.spliterator(cursor, this::instantiateElement); } @Override diff --git a/hollow/src/main/java/com/netflix/hollow/api/objects/HollowListTraversal.java b/hollow/src/main/java/com/netflix/hollow/api/objects/HollowListTraversal.java new file mode 100644 index 0000000000..cbcca2efa0 --- /dev/null +++ b/hollow/src/main/java/com/netflix/hollow/api/objects/HollowListTraversal.java @@ -0,0 +1,211 @@ +/* + * Copyright 2026 Netflix, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +package com.netflix.hollow.api.objects; + +import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; +import com.netflix.hollow.core.read.dataaccess.proxy.HollowListProxyDataAccess; +import com.netflix.hollow.core.read.engine.HollowReadStateEngine; +import com.netflix.hollow.core.read.engine.list.HollowListTypeReadState; +import com.netflix.hollow.core.read.iterator.HollowListOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import java.util.Collections; +import java.util.ConcurrentModificationException; +import java.util.Iterator; +import java.util.ListIterator; +import java.util.NoSuchElementException; +import java.util.Objects; +import java.util.Spliterator; +import java.util.Spliterators; +import java.util.function.Consumer; +import java.util.function.IntFunction; + +/** + * Java collection adapters over a captured ordinal cursor, shared by generic and performance APIs. + */ +public final class HollowListTraversal { + + private HollowListTraversal() { } + + public static boolean isEnabled(HollowListTypeDataAccess access) { + // Inspect only the rollout flag. Traversal must still go through the proxy for object longevity. + while(access instanceof HollowListProxyDataAccess) + access = (HollowListTypeDataAccess)((HollowListProxyDataAccess)access).getCurrentDataAccess(); + if(!(access instanceof HollowListTypeReadState)) + return false; + HollowReadStateEngine engine = ((HollowListTypeReadState)access).getStateEngine(); + return engine != null && engine.isShardCursorIteratorsEnabled(); + } + + public static Iterator iterator(HollowOrdinalIterator source, IntFunction instantiate) { + if(source instanceof HollowListOrdinalIterator) { + HollowListOrdinalIterator cursor = (HollowListOrdinalIterator)source; + return cursor.size() == 0 ? Collections.emptyIterator() : listIterator(cursor, 0, instantiate); + } + return new Iterator() { + private int nextOrdinal = HollowOrdinalIterator.NO_MORE_ORDINALS; + private boolean loaded; + + @Override + public boolean hasNext() { + if(!loaded) { + nextOrdinal = source.next(); + loaded = true; + } + return nextOrdinal != HollowOrdinalIterator.NO_MORE_ORDINALS; + } + + @Override + public T next() { + if(!hasNext()) + throw new NoSuchElementException(); + int ordinal = nextOrdinal; + loaded = false; + return instantiate.apply(ordinal); + } + }; + } + + public static ListIterator listIterator( + HollowListOrdinalIterator source, int index, IntFunction instantiate) { + int size = source.size(); + if(index < 0 || index > size) + throw new IndexOutOfBoundsException("Index: " + index + ", size: " + size); + if(size == 0) + return Collections.emptyListIterator(); + return new ListIterator() { + private int cursor = index; + + @Override + public boolean hasNext() { + return cursor < source.size(); + } + + @Override + public T next() { + if(!hasNext()) + throw new NoSuchElementException(); + T value = element(cursor); + cursor++; + return value; + } + + @Override + public boolean hasPrevious() { + return cursor > 0; + } + + @Override + public T previous() { + if(!hasPrevious()) + throw new NoSuchElementException(); + T value = element(cursor - 1); + cursor--; + return value; + } + + private T element(int elementIndex) { + int ordinal; + try { + ordinal = source.getElementOrdinal(elementIndex); + } catch(IndexOutOfBoundsException e) { + throw new NoSuchElementException(); + } + return instantiate.apply(ordinal); + } + + @Override + public int nextIndex() { + return cursor; + } + + @Override + public int previousIndex() { + return cursor - 1; + } + + @Override + public void remove() { + throw new UnsupportedOperationException(); + } + + @Override + public void set(T value) { + throw new UnsupportedOperationException(); + } + + @Override + public void add(T value) { + throw new UnsupportedOperationException(); + } + }; + } + + public static Spliterator spliterator(HollowListOrdinalIterator source, IntFunction instantiate) { + int size = source.size(); + return size == 0 ? Spliterators.emptySpliterator() : new RangeSpliterator<>(source, 0, size, instantiate); + } + + private static final class RangeSpliterator implements Spliterator { + private final HollowListOrdinalIterator source; + private final int end; + private final IntFunction instantiate; + private int index; + + private RangeSpliterator(HollowListOrdinalIterator source, int index, int end, IntFunction instantiate) { + this.source = source; + this.index = index; + this.end = end; + this.instantiate = instantiate; + } + + @Override + public Spliterator trySplit() { + int midpoint = index + ((end - index) >>> 1); + if(midpoint == index) + return null; + Spliterator prefix = new RangeSpliterator<>(source.copy(), index, midpoint, instantiate); + index = midpoint; + return prefix; + } + + @Override + public boolean tryAdvance(Consumer action) { + Objects.requireNonNull(action); + if(index >= end) + return false; + int ordinal; + try { + ordinal = source.getElementOrdinal(index); + } catch(IndexOutOfBoundsException e) { + throw new ConcurrentModificationException(e); + } + index++; + action.accept(instantiate.apply(ordinal)); + return true; + } + + @Override + public long estimateSize() { + return end - index; + } + + @Override + public int characteristics() { + return ORDERED | SIZED | SUBSIZED; + } + } +} diff --git a/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListCachedDelegate.java b/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListCachedDelegate.java index 7e9bb615ef..e1e7c18172 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListCachedDelegate.java +++ b/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListCachedDelegate.java @@ -21,6 +21,7 @@ import com.netflix.hollow.api.objects.HollowList; import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowListOrdinalIterator; import com.netflix.hollow.core.schema.HollowListSchema; /** @@ -88,14 +89,32 @@ public final int lastIndexOf(HollowList list, int ordinal, Object o) { @Override public HollowOrdinalIterator iterator(int ordinal) { - return new HollowOrdinalIterator() { - private int index; + return cachedCursor(); + } + + @Override + public HollowListOrdinalIterator ordinalCursor(int ordinal) { + // Preserve get-based list traversal for subclasses with custom element behavior. + return getClass() == HollowListCachedDelegate.class ? cachedCursor() : null; + } + + private HollowListOrdinalIterator cachedCursor() { + return new HollowListOrdinalIterator() { + @Override + public int size() { + return ordinals.length; + } + + @Override + public int getElementOrdinal(int index) { + if(index < 0 || index >= ordinals.length) + throw new IndexOutOfBoundsException("Index: " + index + ", size: " + ordinals.length); + return ordinals[index]; + } @Override - public int next() { - if(index == ordinals.length) - return NO_MORE_ORDINALS; - return ordinals[index++]; + public HollowListOrdinalIterator copy() { + return cachedCursor(); } }; } diff --git a/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListDelegate.java b/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListDelegate.java index ce383cf7ef..995891b6f8 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListDelegate.java +++ b/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListDelegate.java @@ -20,6 +20,7 @@ import com.netflix.hollow.api.objects.HollowList; import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowListOrdinalIterator; import com.netflix.hollow.core.schema.HollowListSchema; /** @@ -47,6 +48,13 @@ default HollowOrdinalIterator iterator(int ordinal) { return getTypeDataAccess().ordinalIterator(ordinal); } + /** + * An indexed ordinal cursor, or {@code null} to retain get-based traversal for custom delegates. + */ + default HollowListOrdinalIterator ordinalCursor(int ordinal) { + return null; + } + public HollowListTypeAPI getTypeAPI(); } diff --git a/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListLookupDelegate.java b/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListLookupDelegate.java index 6e5e524f99..8a07fa4d65 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListLookupDelegate.java +++ b/hollow/src/main/java/com/netflix/hollow/api/objects/delegate/HollowListLookupDelegate.java @@ -19,6 +19,8 @@ import com.netflix.hollow.api.custom.HollowListTypeAPI; import com.netflix.hollow.api.objects.HollowList; import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; +import com.netflix.hollow.core.read.iterator.HollowListOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; import com.netflix.hollow.core.schema.HollowListSchema; /** @@ -82,6 +84,15 @@ public final int lastIndexOf(HollowList list, int ordinal, Object o) { return -1; } + @Override + public HollowListOrdinalIterator ordinalCursor(int ordinal) { + // Subclasses may override get() with behavior that an ordinal cursor cannot reproduce. + if(getClass() != HollowListLookupDelegate.class) + return null; + HollowOrdinalIterator iterator = iterator(ordinal); + return iterator instanceof HollowListOrdinalIterator ? (HollowListOrdinalIterator)iterator : null; + } + @Override public HollowListSchema getSchema() { return dataAccess.getSchema(); diff --git a/hollow/src/main/java/com/netflix/hollow/api/perfapi/HollowPerfBackedList.java b/hollow/src/main/java/com/netflix/hollow/api/perfapi/HollowPerfBackedList.java index 4c60784174..397aa1cbd1 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/perfapi/HollowPerfBackedList.java +++ b/hollow/src/main/java/com/netflix/hollow/api/perfapi/HollowPerfBackedList.java @@ -16,11 +16,15 @@ */ package com.netflix.hollow.api.perfapi; +import com.netflix.hollow.api.objects.HollowListTraversal; import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; +import com.netflix.hollow.core.read.iterator.HollowListOrdinalIterator; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; import java.util.AbstractList; import java.util.Iterator; -import java.util.NoSuchElementException; +import java.util.ListIterator; +import java.util.Spliterator; +import java.util.Spliterators; import java.util.RandomAccess; public class HollowPerfBackedList extends AbstractList implements RandomAccess { @@ -50,29 +54,38 @@ public int size() { @Override public Iterator iterator() { - HollowOrdinalIterator ordinalIterator = dataAccess.ordinalIterator(ordinal); - return new Iterator() { - private int nextOrdinal = HollowOrdinalIterator.NO_MORE_ORDINALS; - private boolean nextOrdinalLoaded; + if(!HollowListTraversal.isEnabled(dataAccess)) + return super.iterator(); + return HollowListTraversal.iterator(dataAccess.ordinalIterator(ordinal), this::instantiateElement); + } + + @Override + public ListIterator listIterator(int index) { + // Subclasses may provide custom get() behavior instead of ordinal-based instantiation. + if(getClass() != HollowPerfBackedList.class || !HollowListTraversal.isEnabled(dataAccess)) + return super.listIterator(index); + HollowOrdinalIterator cursor = dataAccess.ordinalIterator(ordinal); + return cursor instanceof HollowListOrdinalIterator + ? HollowListTraversal.listIterator((HollowListOrdinalIterator)cursor, index, this::instantiateElement) + : super.listIterator(index); + } - @Override - public boolean hasNext() { - if(!nextOrdinalLoaded) { - nextOrdinal = ordinalIterator.next(); - nextOrdinalLoaded = true; - } - return nextOrdinal != HollowOrdinalIterator.NO_MORE_ORDINALS; - } + @Override + public Spliterator spliterator() { + if(!HollowListTraversal.isEnabled(dataAccess)) + return super.spliterator(); + if(getClass() != HollowPerfBackedList.class) { + // Java 8's default spliterator calls iterator(), which bypasses a subclass's get(). + return Spliterators.spliterator(super.listIterator(0), size(), Spliterator.ORDERED); + } + HollowOrdinalIterator cursor = dataAccess.ordinalIterator(ordinal); + return cursor instanceof HollowListOrdinalIterator + ? HollowListTraversal.spliterator((HollowListOrdinalIterator)cursor, this::instantiateElement) + : super.spliterator(); + } - @Override - public T next() { - if(!hasNext()) - throw new NoSuchElementException(); - int currentOrdinal = nextOrdinal; - nextOrdinalLoaded = false; - return instantiator.instantiate(elementMaskedTypeIdx | currentOrdinal); - } - }; + private T instantiateElement(int elementOrdinal) { + return instantiator.instantiate(elementMaskedTypeIdx | elementOrdinal); } } diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java index ca5c1a091e..58b1f8dd06 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java @@ -59,7 +59,7 @@ public class HollowReadStateEngine implements HollowStateEngine, HollowDataAcces private final boolean listenToAllPopulatedOrdinals; private final HollowReadConfiguration readConfiguration; private boolean skipTypeShardUpdateWithNoAdditions; - private boolean snapshotCollectionIterators; + private boolean shardCursorIterators; private ArraySegmentRecycler memoryRecycler; private Map headerTags; private Set typesWithDefinedHashCodes = new HashSet(); @@ -265,14 +265,14 @@ public boolean isSkipTypeShardUpdateWithNoAdditions() { } /** - * Experimental: Select snapshot iterators for list, set, and map type read states. + * Experimental. Select shard cursor iterators for collection type read states and Java list traversal. */ - public void setSnapshotCollectionIterators(boolean snapshotCollectionIterators) { - this.snapshotCollectionIterators = snapshotCollectionIterators; + public void setShardCursorIterators(boolean shardCursorIterators) { + this.shardCursorIterators = shardCursorIterators; } - public boolean isSnapshotCollectionIteratorsEnabled() { - return snapshotCollectionIterators; + public boolean isShardCursorIteratorsEnabled() { + return shardCursorIterators; } @Override diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListSnapshotOrdinalIterator.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListSnapshotOrdinalIterator.java index 1f12482e6f..fba4c9c211 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListSnapshotOrdinalIterator.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListSnapshotOrdinalIterator.java @@ -22,8 +22,8 @@ /** * An ordinal iterator over a list record backed by a single validated snapshot of its shard and bounds. *

- * The historical per-element pattern re-reads {@code shardsVolatile} and executes {@code Unsafe.loadFence()} - * twice for every element (the inner loop validating {@code start}/{@code end} plus the trailing + * When on-heap arrays can be recycled, per-element access re-reads {@code shardsVolatile} and executes + * {@code Unsafe.loadFence()} twice for every element (the inner loop validating {@code start}/{@code end} plus the trailing * validation of the element read), i.e. {@code 2N+1} fences for a list of size N including the initial * {@code size()}. This iterator instead validates the bounds once at construction and then executes a single * fence per {@link #next()}, i.e. {@code N+1} fences, while allocating nothing per element. @@ -37,7 +37,7 @@ * When retired on-heap arrays are not recycled, the captured shard remains immutable and strongly reachable. * In that case {@code readWasUnsafe} is a no-op, so the iterator retains its initial snapshot without fences. */ -class HollowListSnapshotOrdinalIterator extends HollowListOrdinalIterator { +final class HollowListSnapshotOrdinalIterator extends HollowListOrdinalIterator { private final HollowListTypeReadState readState; private final int ordinal; @@ -58,6 +58,27 @@ class HollowListSnapshotOrdinalIterator extends HollowListOrdinalIterator { snapshot(); } + private HollowListSnapshotOrdinalIterator(HollowListSnapshotOrdinalIterator source) { + this.readState = source.readState; + this.ordinal = source.ordinal; + this.shardsHolder = source.shardsHolder; + this.shard = source.shard; + this.startElement = source.startElement; + this.endElement = source.endElement; + this.size = source.size; + } + + @Override + public int size() { + return size; + } + + @Override + public HollowListOrdinalIterator copy() { + // Splits retain the captured range, but shard refreshes must remain local to each reader. + return new HollowListSnapshotOrdinalIterator(this); + } + /** * Establish a validated snapshot: a shard whose {@code startElement}/{@code endElement} for this ordinal were * read while the shard was current (hence non-torn). Mirrors the inner validation loop of @@ -85,25 +106,31 @@ private void snapshot() { } @Override - public int next() { - if(index >= size) - return NO_MORE_ORDINALS; + public int getElementOrdinal(int listIndex) { + int elementOrdinal = readElement(listIndex); + if(elementOrdinal == NO_MORE_ORDINALS) + throw new IndexOutOfBoundsException("Index: " + listIndex + ", size: " + size); + return elementOrdinal; + } - int listIndex = index; + private int readElement(int listIndex) { while(true) { - // Bounds were validated in snapshot(), so listIndex < size is an in-bounds, non-throwing read. + if(listIndex < 0 || listIndex >= size) + return NO_MORE_ORDINALS; + // The captured bounds keep the speculative read in bounds even during array recycling. int elementOrdinal = shard.getElementOrdinal(startElement, endElement, listIndex); - - if(!readState.readWasUnsafe(shardsHolder, ordinal, shard)) { - index++; + if(!readState.readWasUnsafe(shardsHolder, ordinal, shard)) return elementOrdinal; - } - - // The snapshot changed mid-read; re-establish it and retry this index. snapshot(); - if(listIndex >= size) - return NO_MORE_ORDINALS; } } + @Override + public int next() { + int elementOrdinal = readElement(index); + if(elementOrdinal != NO_MORE_ORDINALS) + index++; + return elementOrdinal; + } + } diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadState.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadState.java index 1397229468..4b09244b5a 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadState.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadState.java @@ -229,7 +229,7 @@ public int size(int ordinal) { @Override public HollowOrdinalIterator ordinalIterator(int ordinal) { sampler.recordIterator(); - if(stateEngine == null || !stateEngine.isSnapshotCollectionIteratorsEnabled()) + if(stateEngine == null || !stateEngine.isShardCursorIteratorsEnabled()) return new HollowListOrdinalIterator(ordinal, this); return new HollowListSnapshotOrdinalIterator(ordinal, this, sampler); } diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIterator.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIterator.java index 1e73851c7d..c7138d977a 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIterator.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIterator.java @@ -18,16 +18,17 @@ import com.netflix.hollow.api.sampling.HollowMapSampler; import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIteratorImpl; +import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIterator; +import com.netflix.hollow.core.read.iterator.EmptyMapOrdinalIterator; /** * A key/value entry iterator over a map record backed by a single validated snapshot of its shard and bucket * range. *

- * The historical per-bucket pattern re-reads {@code shardsVolatile} and executes {@code Unsafe.loadFence()} - * twice for every bucket probed (via {@code relativeBucket}), plus a {@code size()} at construction — for a map - * whose hash table has M buckets that is {@code 2M+1} fences. This iterator validates the bucket range once at - * construction and then executes a single fence per {@link #next()} (one per surfaced entry), allocating - * nothing. + * When on-heap arrays can be recycled, per-bucket access re-reads {@code shardsVolatile} and executes + * {@code Unsafe.loadFence()} twice for every bucket probed via {@code relativeBucket}, in addition to size + * lookups at construction. This iterator validates the bucket range once at construction and then executes + * a single fence per {@link #next()}, allocating nothing per entry. *

* Correctness mirrors the list/set snapshot iterators: {@code startBucket}/{@code endBucket} are validated * before use, so every relative bucket index in {@code [0, numBuckets)} maps to an in-bounds read from the @@ -52,12 +53,41 @@ final class HollowMapSnapshotEntryOrdinalIterator extends HollowMapEntryOrdinalI private int key = -1; private int value = -1; - HollowMapSnapshotEntryOrdinalIterator( + static HollowMapEntryOrdinalIterator create( int ordinal, HollowMapTypeReadState readState, HollowMapSampler sampler) { + sampler.recordSize(); + HollowMapTypeShardsHolder holder; + HollowMapTypeReadStateShard shard; + long start; + long end; + int size; + do { + holder = readState.shardsVolatile; + shard = holder.shards[ordinal & holder.shardNumberMask]; + int shardOrdinal = ordinal >> shard.shardOrdinalShift; + size = shard.size(shardOrdinal); + if(size == 0) { + start = end = 0; + } else { + start = shard.dataElements.getStartBucket(shardOrdinal); + end = shard.dataElements.getEndBucket(shardOrdinal); + } + } while(readState.readWasUnsafe(holder, ordinal, shard)); + if(size == 0) + return EmptyMapOrdinalIterator.INSTANCE; + // Preserve the size sample formerly recorded by the nonempty iterator's constructor. + sampler.recordSize(); + return new HollowMapSnapshotEntryOrdinalIterator(ordinal, readState, holder, shard, start, end); + } + + private HollowMapSnapshotEntryOrdinalIterator(int ordinal, HollowMapTypeReadState readState, + HollowMapTypeShardsHolder holder, HollowMapTypeReadStateShard shard, long start, long end) { this.readState = readState; this.ordinal = ordinal; - sampler.recordSize(); - snapshot(); + this.shardsHolder = holder; + this.shard = shard; + this.startBucket = start; + this.numBuckets = end - start; } private void snapshot() { diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java index 6c08646931..0c76f5f335 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapTypeReadState.java @@ -367,11 +367,11 @@ public HollowMapEntryOrdinalIterator potentialMatchOrdinalIterator(int ordinal, public HollowMapEntryOrdinalIterator ordinalIterator(int ordinal) { sampler.recordIterator(); + if(stateEngine != null && stateEngine.isShardCursorIteratorsEnabled()) + return HollowMapSnapshotEntryOrdinalIterator.create(ordinal, this, sampler); if(size(ordinal) == 0) return EmptyMapOrdinalIterator.INSTANCE; - if(stateEngine == null || !stateEngine.isSnapshotCollectionIteratorsEnabled()) - return new HollowMapEntryOrdinalIteratorImpl(ordinal, this); - return new HollowMapSnapshotEntryOrdinalIterator(ordinal, this, sampler); + return new HollowMapEntryOrdinalIteratorImpl(ordinal, this); } @Override diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIterator.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIterator.java index 34ec62424f..5736b51b58 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIterator.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIterator.java @@ -18,15 +18,16 @@ import com.netflix.hollow.api.sampling.HollowSetSampler; import com.netflix.hollow.core.read.iterator.HollowSetOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.core.read.iterator.EmptyOrdinalIterator; /** * An ordinal iterator over a set record backed by a single validated snapshot of its shard and bucket range. *

- * The historical per-bucket pattern re-reads {@code shardsVolatile} and executes {@code Unsafe.loadFence()} - * twice for every bucket probed (via {@code relativeBucketValue}), plus a {@code size()} at construction — for a - * set whose hash table has M buckets that is {@code 2M+1} fences. This iterator validates the bucket range once - * at construction and then executes a single fence per {@link #next()} (one per returned element), allocating - * nothing. + * When on-heap arrays can be recycled, per-bucket access re-reads {@code shardsVolatile} and executes + * {@code Unsafe.loadFence()} twice for every bucket probed via {@code relativeBucketValue}, in addition to + * size lookups at construction. This iterator validates the bucket range once at construction and then + * executes a single fence per {@link #next()}, allocating nothing per element. *

* Correctness mirrors the list snapshot iterator: {@code startBucket}/{@code endBucket} are validated before * use, so every relative bucket index in {@code [0, numBuckets)} maps to an in-bounds read from the captured @@ -49,12 +50,41 @@ final class HollowSetSnapshotOrdinalIterator extends HollowSetOrdinalIterator { private int currentBucket = -1; - HollowSetSnapshotOrdinalIterator( + static HollowOrdinalIterator create( int ordinal, HollowSetTypeReadState readState, HollowSetSampler sampler) { + sampler.recordSize(); + HollowSetTypeShardsHolder holder; + HollowSetTypeReadStateShard shard; + long start; + long end; + int size; + do { + holder = readState.shardsVolatile; + shard = holder.shards[ordinal & holder.shardNumberMask]; + int shardOrdinal = ordinal >> shard.shardOrdinalShift; + size = shard.size(shardOrdinal); + if(size == 0) { + start = end = 0; + } else { + start = shard.dataElements.getStartBucket(shardOrdinal); + end = shard.dataElements.getEndBucket(shardOrdinal); + } + } while(readState.readWasUnsafe(holder, ordinal, shard)); + if(size == 0) + return EmptyOrdinalIterator.INSTANCE; + // Preserve the size sample formerly recorded by the nonempty iterator's constructor. + sampler.recordSize(); + return new HollowSetSnapshotOrdinalIterator(ordinal, readState, holder, shard, start, end); + } + + private HollowSetSnapshotOrdinalIterator(int ordinal, HollowSetTypeReadState readState, + HollowSetTypeShardsHolder holder, HollowSetTypeReadStateShard shard, long start, long end) { this.readState = readState; this.ordinal = ordinal; - sampler.recordSize(); - snapshot(); + this.shardsHolder = holder; + this.shard = shard; + this.startBucket = start; + this.numBuckets = end - start; } private void snapshot() { diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java index 943139f565..a2e76e4cda 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java @@ -329,11 +329,11 @@ public HollowOrdinalIterator potentialMatchOrdinalIterator(int ordinal, int hash @Override public HollowOrdinalIterator ordinalIterator(int ordinal) { sampler.recordIterator(); + if(stateEngine != null && stateEngine.isShardCursorIteratorsEnabled()) + return HollowSetSnapshotOrdinalIterator.create(ordinal, this, sampler); if(size(ordinal) == 0) return EmptyOrdinalIterator.INSTANCE; - if(stateEngine == null || !stateEngine.isSnapshotCollectionIteratorsEnabled()) - return new HollowSetOrdinalIterator(ordinal, this); - return new HollowSetSnapshotOrdinalIterator(ordinal, this, sampler); + return new HollowSetOrdinalIterator(ordinal, this); } boolean readWasUnsafe(HollowSetTypeShardsHolder shardsHolder, int ordinal, HollowSetTypeReadStateShard shard) { diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowListOrdinalIterator.java b/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowListOrdinalIterator.java index d5e14bddec..6697a5b4b0 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowListOrdinalIterator.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/iterator/HollowListOrdinalIterator.java @@ -39,12 +39,36 @@ private HollowListOrdinalIterator(int listOrdinal, HollowListTypeDataAccess data this.size = size; } + /** + * Returns the size captured by this cursor, without another data access lookup. + */ + public int size() { + return size; + } + + /** + * Reads an element using this cursor's captured bounds without changing its forward position. + */ + public int getElementOrdinal(int index) { + if(index < 0 || index >= size()) + throw new IndexOutOfBoundsException("Index: " + index + ", size: " + size()); + return dataAccess.getElementOrdinal(listOrdinal, index); + } + + /** + * Copies the captured bounds with an independent traversal and validation state. + * The copy starts at index zero and does not perform another size lookup. + */ + public HollowListOrdinalIterator copy() { + return new HollowListOrdinalIterator(listOrdinal, dataAccess, size); + } + @Override public int next() { - if(currentElement == size) + if(currentElement >= size()) return NO_MORE_ORDINALS; - return dataAccess.getElementOrdinal(listOrdinal, currentElement++); + return getElementOrdinal(currentElement++); } } diff --git a/hollow/src/test/java/com/netflix/hollow/api/consumer/HollowConsumerBuilderTest.java b/hollow/src/test/java/com/netflix/hollow/api/consumer/HollowConsumerBuilderTest.java index 0f0c49fd53..7e2c1aafa5 100644 --- a/hollow/src/test/java/com/netflix/hollow/api/consumer/HollowConsumerBuilderTest.java +++ b/hollow/src/test/java/com/netflix/hollow/api/consumer/HollowConsumerBuilderTest.java @@ -20,7 +20,7 @@ public class HollowConsumerBuilderTest { @Test - public void snapshotCollectionIteratorsAreOptIn() throws IOException { + public void shardCursorIteratorsAreOptIn() throws IOException { TestBlobRetriever blobRetriever = new TestBlobRetriever(); TestHollowConsumer consumer = new TestHollowConsumer.Builder() .withBlobRetriever(blobRetriever) @@ -29,17 +29,22 @@ public void snapshotCollectionIteratorsAreOptIn() throws IOException { .add(new com.netflix.hollow.test.model.Movie(1, "movie", 2026)) .build()); consumer.triggerRefreshTo(1L); - assertFalse(consumer.getStateEngine().isSnapshotCollectionIteratorsEnabled()); + assertFalse(consumer.getStateEngine().isShardCursorIteratorsEnabled()); TestHollowConsumer enabledConsumer = new TestHollowConsumer.Builder() .withBlobRetriever(new TestBlobRetriever()) - .withSnapshotCollectionIterators() + .withShardCursorIterators() .build(); enabledConsumer.addSnapshot(1L, new HollowWriteStateEngineBuilder() .add(new com.netflix.hollow.test.model.Movie(1, "movie", 2026)) .build()); enabledConsumer.triggerRefreshTo(1L); - assertTrue(enabledConsumer.getStateEngine().isSnapshotCollectionIteratorsEnabled()); + assertTrue(enabledConsumer.getStateEngine().isShardCursorIteratorsEnabled()); + enabledConsumer.addSnapshot(2L, new HollowWriteStateEngineBuilder() + .add(new com.netflix.hollow.test.model.Movie(2, "replacement", 2026)) + .build()); + enabledConsumer.triggerRefreshTo(2L); + assertTrue(enabledConsumer.getStateEngine().isShardCursorIteratorsEnabled()); } @Test diff --git a/hollow/src/test/java/com/netflix/hollow/api/objects/HollowListIteratorTest.java b/hollow/src/test/java/com/netflix/hollow/api/objects/HollowListIteratorTest.java index 0dcc2a5065..db4d801d62 100644 --- a/hollow/src/test/java/com/netflix/hollow/api/objects/HollowListIteratorTest.java +++ b/hollow/src/test/java/com/netflix/hollow/api/objects/HollowListIteratorTest.java @@ -2,25 +2,61 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import com.netflix.hollow.api.custom.HollowListTypeAPI; import com.netflix.hollow.api.objects.delegate.HollowListCachedDelegate; import com.netflix.hollow.api.objects.delegate.HollowListDelegate; import com.netflix.hollow.api.objects.delegate.HollowListLookupDelegate; import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; +import com.netflix.hollow.core.read.dataaccess.disabled.HollowListDisabledDataAccess; +import com.netflix.hollow.core.read.dataaccess.proxy.HollowListProxyDataAccess; +import com.netflix.hollow.core.read.dataaccess.proxy.HollowProxyDataAccess; +import com.netflix.hollow.core.read.engine.HollowReadStateEngine; +import com.netflix.hollow.core.read.engine.list.HollowListTypeReadState; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowListOrdinalIterator; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; +import java.util.List; +import java.util.ListIterator; +import java.util.NoSuchElementException; +import java.util.Spliterator; +import java.util.Spliterators; import org.junit.Test; public class HollowListIteratorTest { @Test - public void iteratesUsingOrdinalIterator() { + public void usesGetBasedTraversalWithoutOptIn() { HollowListTypeDataAccess dataAccess = mock(HollowListTypeDataAccess.class); + when(dataAccess.size(7)).thenReturn(2); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); + when(dataAccess.getElementOrdinal(7, 1)).thenReturn(5); + when(dataAccess.ordinalIterator(7)).thenAnswer(invocation -> + new HollowListOrdinalIterator(7, dataAccess)); + HollowList list = list(new HollowListLookupDelegate<>(dataAccess)); + + assertEquals(Integer.valueOf(3), list.iterator().next()); + assertEquals(Integer.valueOf(5), list.listIterator(1).next()); + List values = new ArrayList<>(); + list.spliterator().forEachRemaining(values::add); + assertEquals(Arrays.asList(3, 5), values); + verify(dataAccess, never()).ordinalIterator(7); + } + + @Test + public void iteratesUsingOrdinalIterator() { + HollowListTypeDataAccess dataAccess = enabledDataAccess(); HollowOrdinalIterator ordinalIterator = mock(HollowOrdinalIterator.class); when(dataAccess.ordinalIterator(7)).thenReturn(ordinalIterator); when(ordinalIterator.next()).thenReturn(3, 5, HollowOrdinalIterator.NO_MORE_ORDINALS); @@ -38,7 +74,7 @@ public void iteratesUsingOrdinalIterator() { @Test public void cachedListIteratesCachedOrdinals() { - HollowListTypeDataAccess dataAccess = mock(HollowListTypeDataAccess.class); + HollowListTypeDataAccess dataAccess = enabledDataAccess(); when(dataAccess.size(7)).thenReturn(2); when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); when(dataAccess.getElementOrdinal(7, 1)).thenReturn(5); @@ -52,6 +88,178 @@ public void cachedListIteratesCachedOrdinals() { verify(dataAccess, never()).ordinalIterator(7); } + @Test + public void listIteratorUsesCapturedBoundsInBothDirections() { + HollowListTypeDataAccess dataAccess = enabledDataAccess(); + when(dataAccess.size(7)).thenReturn(2); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); + when(dataAccess.getElementOrdinal(7, 1)).thenReturn(5); + when(dataAccess.ordinalIterator(7)).thenAnswer(invocation -> + new HollowListOrdinalIterator(7, dataAccess)); + HollowList list = list(new HollowListLookupDelegate<>(dataAccess)); + + ListIterator iterator = list.listIterator(1); + clearInvocations(dataAccess); + assertEquals(1, iterator.nextIndex()); + assertEquals(0, iterator.previousIndex()); + assertEquals(Integer.valueOf(3), iterator.previous()); + assertFalse(iterator.hasPrevious()); + assertThatThrownBy(iterator::previous).isInstanceOf(NoSuchElementException.class); + assertEquals(Integer.valueOf(3), iterator.next()); + assertEquals(Integer.valueOf(5), iterator.next()); + assertFalse(iterator.hasNext()); + assertThatThrownBy(iterator::next).isInstanceOf(NoSuchElementException.class); + assertThatThrownBy(iterator::remove).isInstanceOf(UnsupportedOperationException.class); + assertThatThrownBy(() -> iterator.set(9)).isInstanceOf(UnsupportedOperationException.class); + assertThatThrownBy(() -> iterator.add(9)).isInstanceOf(UnsupportedOperationException.class); + verify(dataAccess, never()).size(7); + assertThatThrownBy(() -> list.listIterator(-1)).isInstanceOf(IndexOutOfBoundsException.class); + assertThatThrownBy(() -> list.listIterator(3)).isInstanceOf(IndexOutOfBoundsException.class); + } + + @Test + public void spliteratorSplitsRangesWithoutReadingElements() { + HollowListTypeDataAccess dataAccess = enabledDataAccess(); + when(dataAccess.size(7)).thenReturn(2); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); + when(dataAccess.getElementOrdinal(7, 1)).thenReturn(5); + when(dataAccess.ordinalIterator(7)).thenAnswer(invocation -> + new HollowListOrdinalIterator(7, dataAccess)); + HollowList list = list(new HollowListLookupDelegate<>(dataAccess)); + + Spliterator suffix = list.spliterator(); + clearInvocations(dataAccess); + Spliterator prefix = suffix.trySplit(); + assertEquals(1, prefix.estimateSize()); + assertEquals(1, suffix.estimateSize()); + assertTrue(suffix.hasCharacteristics(Spliterator.ORDERED | Spliterator.SIZED | Spliterator.SUBSIZED)); + verify(dataAccess, never()).getElementOrdinal(7, 0); + verify(dataAccess, never()).getElementOrdinal(7, 1); + verify(dataAccess, never()).size(7); + List values = new ArrayList<>(); + prefix.forEachRemaining(values::add); + suffix.forEachRemaining(values::add); + assertEquals(Arrays.asList(3, 5), values); + assertEquals(0, suffix.estimateSize()); + assertThatThrownBy(() -> suffix.tryAdvance(null)).isInstanceOf(NullPointerException.class); + } + + @Test + public void cachedListTraversalDoesNotReturnToLiveData() { + HollowListTypeDataAccess dataAccess = enabledDataAccess(); + when(dataAccess.size(7)).thenReturn(2); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); + when(dataAccess.getElementOrdinal(7, 1)).thenReturn(5); + HollowList list = list(new HollowListCachedDelegate<>(dataAccess, 7)); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(99); + clearInvocations(dataAccess); + + ListIterator iterator = list.listIterator(2); + assertEquals(Integer.valueOf(5), iterator.previous()); + assertEquals(Integer.valueOf(3), iterator.previous()); + List values = new ArrayList<>(); + Spliterator suffix = list.spliterator(); + suffix.trySplit().forEachRemaining(values::add); + suffix.forEachRemaining(values::add); + assertEquals(Arrays.asList(3, 5), values); + verify(dataAccess, never()).size(7); + verify(dataAccess, never()).getElementOrdinal(7, 0); + verify(dataAccess, never()).ordinalIterator(7); + } + + @Test + public void lookupSubclassRetainsItsGetBasedListIterator() { + HollowListTypeDataAccess dataAccess = enabledDataAccess(); + when(dataAccess.size(7)).thenReturn(1); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); + when(dataAccess.ordinalIterator(7)).thenAnswer(invocation -> + new HollowListOrdinalIterator(7, dataAccess)); + HollowList list = list(new HollowListLookupDelegate(dataAccess) { + @Override + public Integer get(HollowList list, int ordinal, int index) { + return 99; + } + }); + assertEquals(Integer.valueOf(99), list.listIterator().next()); + } + + @Test + public void proxyTraversalContinuesToUseTheProxyAfterSplitting() { + HollowListTypeDataAccess original = enabledDataAccess(); + when(original.size(7)).thenReturn(2); + HollowListProxyDataAccess proxy = new HollowListProxyDataAccess(new HollowProxyDataAccess()); + proxy.setCurrentDataAccess(original); + HollowList list = list(new HollowListLookupDelegate<>(proxy)); + ListIterator iterator = list.listIterator(); + Spliterator suffix = list.spliterator(); + Spliterator prefix = suffix.trySplit(); + + HollowListTypeDataAccess replacement = mock(HollowListTypeDataAccess.class); + when(replacement.getElementOrdinal(7, 0)).thenReturn(9); + when(replacement.getElementOrdinal(7, 1)).thenReturn(11); + proxy.setCurrentDataAccess(replacement); + assertEquals(Integer.valueOf(9), iterator.next()); + List values = new ArrayList<>(); + prefix.forEachRemaining(values::add); + suffix.forEachRemaining(values::add); + assertEquals(Arrays.asList(9, 11), values); + verify(replacement, never()).size(7); + } + + @Test + public void emptyCursorPreservesTraversalContracts() { + HollowListTypeDataAccess dataAccess = enabledDataAccess(); + HollowList list = list(new HollowListCachedDelegate<>(dataAccess, 7)); + clearInvocations(dataAccess); + assertSame(Collections.emptyIterator(), list.iterator()); + ListIterator iterator = list.listIterator(); + assertSame(Collections.emptyListIterator(), iterator); + assertFalse(iterator.hasNext()); + assertFalse(iterator.hasPrevious()); + assertEquals(0, iterator.nextIndex()); + assertEquals(-1, iterator.previousIndex()); + assertThatThrownBy(iterator::next).isInstanceOf(NoSuchElementException.class); + assertThatThrownBy(iterator::previous).isInstanceOf(NoSuchElementException.class); + assertThatThrownBy(iterator::remove).isInstanceOf(IllegalStateException.class); + assertThatThrownBy(() -> iterator.set(9)).isInstanceOf(IllegalStateException.class); + assertThatThrownBy(() -> iterator.add(9)).isInstanceOf(UnsupportedOperationException.class); + assertThatThrownBy(() -> list.listIterator(-1)).isInstanceOf(IndexOutOfBoundsException.class); + assertThatThrownBy(() -> list.listIterator(1)).isInstanceOf(IndexOutOfBoundsException.class); + Spliterator spliterator = list.spliterator(); + assertSame(Spliterators.emptySpliterator(), spliterator); + assertEquals(0, spliterator.estimateSize()); + assertEquals(null, spliterator.trySplit()); + assertFalse(spliterator.tryAdvance(value -> { throw new AssertionError(); })); + assertThatThrownBy(() -> spliterator.tryAdvance(null)).isInstanceOf(NullPointerException.class); + verify(dataAccess, never()).size(7); + verify(dataAccess, never()).getElementOrdinal(7, 0); + } + + @Test + public void cachedTraversalWorksAfterLiveDataAccessIsDisabled() { + HollowListTypeDataAccess dataAccess = enabledDataAccess(); + when(dataAccess.size(7)).thenReturn(2); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); + when(dataAccess.getElementOrdinal(7, 1)).thenReturn(5); + HollowListCachedDelegate delegate = new HollowListCachedDelegate<>(dataAccess, 7); + HollowList list = list(delegate); + HollowListTypeAPI replacement = mock(HollowListTypeAPI.class); + when(replacement.getTypeDataAccess()).thenReturn(HollowListDisabledDataAccess.INSTANCE); + delegate.updateTypeAPI(replacement); + + assertEquals(Integer.valueOf(3), list.iterator().next()); + assertEquals(Integer.valueOf(5), list.listIterator(2).previous()); + assertEquals(Arrays.asList(3, 5), list.stream().collect(java.util.stream.Collectors.toList())); + } + + private HollowListTypeDataAccess enabledDataAccess() { + HollowListTypeReadState type = mock(HollowListTypeReadState.class); + HollowReadStateEngine engine = new HollowReadStateEngine(); + engine.setShardCursorIterators(true); + when(type.getStateEngine()).thenReturn(engine); + return type; + } + private HollowList list(HollowListDelegate delegate) { return new HollowList(delegate, 7) { @Override diff --git a/hollow/src/test/java/com/netflix/hollow/api/perfapi/HollowPerfBackedListTest.java b/hollow/src/test/java/com/netflix/hollow/api/perfapi/HollowPerfBackedListTest.java index d0550d90b3..7902f19b51 100644 --- a/hollow/src/test/java/com/netflix/hollow/api/perfapi/HollowPerfBackedListTest.java +++ b/hollow/src/test/java/com/netflix/hollow/api/perfapi/HollowPerfBackedListTest.java @@ -2,22 +2,103 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.netflix.hollow.core.read.dataaccess.HollowListTypeDataAccess; +import com.netflix.hollow.core.read.engine.HollowReadStateEngine; +import com.netflix.hollow.core.read.engine.list.HollowListTypeReadState; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; +import com.netflix.hollow.core.read.iterator.HollowListOrdinalIterator; +import java.util.Collections; import java.util.Iterator; +import java.util.ListIterator; +import java.util.Spliterator; +import java.util.Spliterators; import org.junit.Test; public class HollowPerfBackedListTest { @Test - public void iteratesUsingOrdinalIterator() { + public void usesGetBasedTraversalWithoutOptIn() { HollowListTypePerfAPI typeAPI = mock(HollowListTypePerfAPI.class); HollowListTypeDataAccess dataAccess = mock(HollowListTypeDataAccess.class); + when(typeAPI.typeAccess()).thenReturn(dataAccess); + when(dataAccess.size(7)).thenReturn(2); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); + when(dataAccess.getElementOrdinal(7, 1)).thenReturn(5); + when(dataAccess.ordinalIterator(7)).thenAnswer(invocation -> + new HollowListOrdinalIterator(7, dataAccess)); + HollowPerfBackedList list = new HollowPerfBackedList<>(typeAPI, 7, ref -> ref); + + assertEquals(Long.valueOf(3), list.iterator().next()); + assertEquals(Long.valueOf(5), list.listIterator(1).next()); + assertEquals(java.util.Arrays.asList(3L, 5L), list.stream().collect(java.util.stream.Collectors.toList())); + verify(dataAccess, never()).ordinalIterator(7); + } + + @Test + public void listIteratorAndSpliteratorUseOrdinalCursor() { + HollowListTypePerfAPI typeAPI = mock(HollowListTypePerfAPI.class); + HollowListTypeDataAccess dataAccess = enabledDataAccess(); + when(typeAPI.typeAccess()).thenReturn(dataAccess); + when(dataAccess.size(7)).thenReturn(2); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); + when(dataAccess.getElementOrdinal(7, 1)).thenReturn(5); + when(dataAccess.ordinalIterator(7)).thenAnswer(invocation -> + new HollowListOrdinalIterator(7, dataAccess)); + HollowPerfBackedList list = new HollowPerfBackedList<>(typeAPI, 7, ref -> ref); + + ListIterator iterator = list.listIterator(1); + clearInvocations(dataAccess); + assertEquals(Long.valueOf(3), iterator.previous()); + assertEquals(Long.valueOf(3), iterator.next()); + assertEquals(Long.valueOf(5), iterator.next()); + assertFalse(iterator.hasNext()); + verify(dataAccess, never()).size(7); + + Spliterator suffix = list.spliterator(); + clearInvocations(dataAccess); + Spliterator prefix = suffix.trySplit(); + assertEquals(1, prefix.estimateSize()); + assertEquals(1, suffix.estimateSize()); + verify(dataAccess, never()).size(7); + verify(dataAccess, never()).getElementOrdinal(7, 0); + verify(dataAccess, never()).getElementOrdinal(7, 1); + assertTrue(prefix.tryAdvance(value -> assertEquals(Long.valueOf(3), value))); + assertTrue(suffix.tryAdvance(value -> assertEquals(Long.valueOf(5), value))); + assertFalse(suffix.tryAdvance(value -> { })); + } + + @Test + public void subclassRetainsItsGetBasedTraversal() { + HollowListTypePerfAPI typeAPI = mock(HollowListTypePerfAPI.class); + HollowListTypeDataAccess dataAccess = enabledDataAccess(); + when(typeAPI.typeAccess()).thenReturn(dataAccess); + when(dataAccess.size(7)).thenReturn(1); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); + when(dataAccess.ordinalIterator(7)).thenAnswer(invocation -> + new HollowListOrdinalIterator(7, dataAccess)); + HollowPerfBackedList list = new HollowPerfBackedList(typeAPI, 7, ref -> ref) { + @Override + public Long get(int index) { + return 99L; + } + }; + assertEquals(Long.valueOf(99), list.listIterator().next()); + assertTrue(list.spliterator().tryAdvance(value -> assertEquals(Long.valueOf(99), value))); + } + + @Test + public void iteratesUsingOrdinalIterator() { + HollowListTypePerfAPI typeAPI = mock(HollowListTypePerfAPI.class); + HollowListTypeDataAccess dataAccess = enabledDataAccess(); HollowOrdinalIterator ordinalIterator = mock(HollowOrdinalIterator.class); when(typeAPI.typeAccess()).thenReturn(dataAccess); when(dataAccess.ordinalIterator(7)).thenReturn(ordinalIterator); @@ -33,4 +114,33 @@ public void iteratesUsingOrdinalIterator() { assertFalse(iterator.hasNext()); verify(dataAccess).ordinalIterator(7); } + + @Test + public void emptyCapturedCursorUsesJdkAdaptersWithoutAnotherSizeLookup() { + HollowListTypePerfAPI typeAPI = mock(HollowListTypePerfAPI.class); + HollowListTypeDataAccess dataAccess = enabledDataAccess(); + when(typeAPI.typeAccess()).thenReturn(dataAccess); + HollowListOrdinalIterator cursor = new HollowListOrdinalIterator(7, dataAccess); + when(dataAccess.ordinalIterator(7)).thenReturn(cursor); + HollowPerfBackedList list = new HollowPerfBackedList<>(typeAPI, 7, ref -> { + throw new AssertionError("Empty traversal must not instantiate an element"); + }); + clearInvocations(dataAccess); + + assertSame(Collections.emptyIterator(), list.iterator()); + assertSame(Collections.emptyListIterator(), list.listIterator()); + assertSame(Spliterators.emptySpliterator(), list.spliterator()); + assertThatThrownBy(() -> list.listIterator(-1)).isInstanceOf(IndexOutOfBoundsException.class); + assertThatThrownBy(() -> list.listIterator(1)).isInstanceOf(IndexOutOfBoundsException.class); + verify(dataAccess, never()).size(7); + verify(dataAccess, never()).getElementOrdinal(7, 0); + } + + private HollowListTypeDataAccess enabledDataAccess() { + HollowListTypeReadState type = mock(HollowListTypeReadState.class); + HollowReadStateEngine engine = new HollowReadStateEngine(); + engine.setShardCursorIterators(true); + when(type.getStateEngine()).thenReturn(engine); + return type; + } } diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTraversalSnapshotTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTraversalSnapshotTest.java new file mode 100644 index 0000000000..679f14176b --- /dev/null +++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTraversalSnapshotTest.java @@ -0,0 +1,185 @@ +package com.netflix.hollow.core.read.engine.list; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertSame; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.netflix.hollow.api.objects.HollowList; +import com.netflix.hollow.api.objects.delegate.HollowListLookupDelegate; +import com.netflix.hollow.api.perfapi.HollowListTypePerfAPI; +import com.netflix.hollow.api.perfapi.HollowPerfBackedList; +import com.netflix.hollow.core.memory.pool.WastefulRecycler; +import com.netflix.hollow.core.read.engine.HollowReadStateEngine; +import com.netflix.hollow.core.read.engine.HollowTypeReshardingStrategy; +import com.netflix.hollow.core.util.StateEngineRoundTripper; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.ListIterator; +import java.util.Spliterator; +import java.util.Spliterators; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import org.junit.Test; + +public class HollowListTraversalSnapshotTest extends AbstractHollowListTypeDataElementsSplitJoinTest { + + @Test + public void defaultJavaTraversalDoesNotSelectOrdinalIterators() throws Exception { + HollowListTypeReadState type = spy(populateTypeStateWith(new int[][] {{0, 1, 2}})); + for (List list : lists(type)) { + assertEquals(0, list.iterator().next().intValue()); + assertEquals(1, list.listIterator(1).next().intValue()); + assertEquals(Arrays.asList(0, 1, 2), list.stream().map(Number::intValue).collect(Collectors.toList())); + } + verify(type, never()).ordinalIterator(0); + } + + @Test + public void emptyShardCursorsUseJdkAdapters() throws Exception { + HollowListTypeReadState type = populateTypeStateWith(new int[][] {{}}); + readStateEngine.setShardCursorIterators(true); + for (List list : lists(type)) { + assertSame(Collections.emptyIterator(), list.iterator()); + assertSame(Collections.emptyListIterator(), list.listIterator()); + assertSame(Spliterators.emptySpliterator(), list.spliterator()); + } + } + + @Test + public void listIteratorRetainsImmutableSnapshotInBothDirections() throws Exception { + HollowListTypeReadState type = immutableState(); + List> iterators = new ArrayList<>(); + for (List list : lists(type)) + iterators.add(list.listIterator(1)); + refresh(); + for (ListIterator iterator : iterators) { + assertEquals(0, iterator.previous().intValue()); + assertEquals(0, iterator.next().intValue()); + assertEquals(1, iterator.next().intValue()); + assertEquals(2, iterator.next().intValue()); + assertEquals(2, iterator.previous().intValue()); + } + } + + @Test + public void splitsRetainTheOriginalImmutableSnapshot() throws Exception { + HollowListTypeReadState type = immutableState(); + List> suffixes = new ArrayList<>(); + List> prefixes = new ArrayList<>(); + for (List list : lists(type)) { + Spliterator suffix = list.spliterator(); + suffixes.add(suffix); + prefixes.add(suffix.trySplit()); + } + refresh(); + for (int i = 0; i < suffixes.size(); i++) { + List values = new ArrayList<>(); + prefixes.get(i).forEachRemaining(value -> values.add(value.intValue())); + Spliterator tail = suffixes.get(i); + tail.trySplit().forEachRemaining(value -> values.add(value.intValue())); + tail.forEachRemaining(value -> values.add(value.intValue())); + assertEquals(Arrays.asList(0, 1, 2), values); + } + } + + @Test + public void parallelTraversalSurvivesRecycledResharding() throws Exception { + HollowListTypeReadState type = populateTypeStateWith(generateListContents(128)); + readStateEngine.setShardCursorIterators(true); + for (List list : lists(type, 127)) { + Spliterator suffix = list.spliterator(); + Spliterator prefix = suffix.trySplit(); + HollowTypeReshardingStrategy.getInstance(type).reshard(type, type.numShards(), 2); + List values = new ArrayList<>(); + prefix.forEachRemaining(value -> values.add(value.intValue())); + suffix.forEachRemaining(value -> values.add(value.intValue())); + assertEquals(128, values.size()); + for (int i = 0; i < values.size(); i++) + assertEquals(i, values.get(i).intValue()); + assertEquals(values, list.parallelStream().map(Number::intValue).collect(Collectors.toList())); + HollowTypeReshardingStrategy.getInstance(type).reshard(type, type.numShards(), 1); + } + } + + @Test(timeout = 60_000) + public void parallelSplitsValidateIndependentlyDuringResharding() throws Exception { + HollowListTypeReadState type = populateTypeStateWith(generateListContents(128)); + readStateEngine.setShardCursorIterators(true); + List expected = IntStream.range(0, 128).boxed().collect(Collectors.toList()); + CountDownLatch start = new CountDownLatch(1); + AtomicReference failure = new AtomicReference<>(); + Thread writer = new Thread(() -> { + try { + start.await(); + for (int i = 0; i < 100; i++) { + int count = type.numShards(); + HollowTypeReshardingStrategy.getInstance(type).reshard(type, count, count < 4 ? count * 2 : 1); + } + } catch (Throwable t) { + failure.set(t); + } + }, "list-split-resharder"); + writer.start(); + start.countDown(); + try { + for (int i = 0; i < 20; i++) { + for (List list : lists(type, 127)) + assertEquals(expected, list.parallelStream().map(Number::intValue).collect(Collectors.toList())); + } + } finally { + writer.join(30_000); + } + assertTrue("resharder did not finish", !writer.isAlive()); + if (failure.get() != null) + throw new AssertionError(failure.get()); + } + + private HollowListTypeReadState immutableState() throws Exception { + populateWriteStateEngine(3); + populateWriteStateEngineWithListRecords(new int[][] {{0, 1, 2}}); + readStateEngine = new HollowReadStateEngine(WastefulRecycler.DEFAULT_INSTANCE); + readStateEngine.setShardCursorIterators(true); + StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); + return (HollowListTypeReadState) readStateEngine.getTypeState("TestList"); + } + + private void refresh() throws Exception { + populateWriteStateEngine(writeStateEngine, schema, 3); + populateWriteStateEngineWithListRecords(new int[][] {{2}}); + roundTripDelta(); + // Retire the ghost and reuse its ordinal so live get() can no longer reproduce the snapshot. + populateWriteStateEngine(writeStateEngine, schema, 3); + populateWriteStateEngineWithListRecords(new int[][] {{1}}); + roundTripDelta(); + } + + private List> lists(HollowListTypeReadState type) { + return lists(type, 0); + } + + private List> lists(HollowListTypeReadState type, int ordinal) { + HollowList generic = new HollowList(new HollowListLookupDelegate<>(type), ordinal) { + @Override + public Integer instantiateElement(int elementOrdinal) { + return elementOrdinal; + } + + @Override + public boolean equalsElement(int elementOrdinal, Object testObject) { + return Integer.valueOf(elementOrdinal).equals(testObject); + } + }; + HollowListTypePerfAPI api = mock(HollowListTypePerfAPI.class); + when(api.typeAccess()).thenReturn(type); + return Arrays.asList(generic, new HollowPerfBackedList<>(api, ordinal, ref -> ref)); + } +} diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java index 55a156242a..4fc8dac6c4 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java @@ -40,7 +40,7 @@ public void usesCompatibleSnapshotIteratorWhenEnabled() throws Exception { populateWriteStateEngineWithListRecords(new int[][] {{0, 1, 2}}); readStateEngine = new HollowReadStateEngine(WastefulRecycler.DEFAULT_INSTANCE); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); HollowListTypeReadState typeState = @@ -57,7 +57,7 @@ public void iteratorRetainsImmutableShardAcrossRefresh() throws Exception { populateWriteStateEngineWithListRecords(new int[][] {{0, 1, 2}}); readStateEngine = new HollowReadStateEngine(WastefulRecycler.DEFAULT_INSTANCE); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); HollowListTypeReadState typeState = @@ -79,7 +79,7 @@ public void iteratorRefreshesRecycledShardAcrossRefresh() throws Exception { populateWriteStateEngine(3); populateWriteStateEngineWithListRecords(new int[][] {{0, 1, 2}, {0}}); roundTripSnapshot(); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); HollowListTypeReadState typeState = (HollowListTypeReadState) readStateEngine.getTypeState("TestList"); @@ -95,6 +95,28 @@ public void iteratorRefreshesRecycledShardAcrossRefresh() throws Exception { assertEquals(HollowOrdinalIterator.NO_MORE_ORDINALS, iterator.next()); } + @Test + public void recycledIteratorExhaustsWhenOrdinalIsReusedByShorterList() throws Exception { + populateWriteStateEngine(3); + populateWriteStateEngineWithListRecords(new int[][] {{0, 1, 2}}); + roundTripSnapshot(); + readStateEngine.setShardCursorIterators(true); + HollowListTypeReadState typeState = (HollowListTypeReadState) readStateEngine.getTypeState("TestList"); + HollowOrdinalIterator iterator = typeState.ordinalIterator(0); + assertEquals(0, iterator.next()); + + populateWriteStateEngine(writeStateEngine, schema, 3); + populateWriteStateEngineWithListRecords(new int[][] {{2}}); + roundTripDelta(); + populateWriteStateEngine(writeStateEngine, schema, 3); + populateWriteStateEngineWithListRecords(new int[][] {{1}}); + roundTripDelta(); + assertEquals(1, typeState.size(0)); + assertEquals(1, typeState.getElementOrdinal(0, 0)); + assertEquals(HollowOrdinalIterator.NO_MORE_ORDINALS, iterator.next()); + assertEquals(HollowOrdinalIterator.NO_MORE_ORDINALS, iterator.next()); + } + @Test public void testApproximateHoleCostWithShards() throws Exception { // Setup with 2 shards diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIteratorTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIteratorTest.java index 8bf12e580c..6c2c127e1a 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIteratorTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/map/HollowMapSnapshotEntryOrdinalIteratorTest.java @@ -1,8 +1,20 @@ package com.netflix.hollow.core.read.engine.map; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.junit.Assert.fail; +import com.netflix.hollow.api.sampling.EnabledSamplingDirector; +import com.netflix.hollow.api.sampling.SampleResult; +import com.netflix.hollow.core.read.iterator.EmptyMapOrdinalIterator; import com.netflix.hollow.core.read.engine.HollowTypeReshardingStrategy; import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIterator; import com.netflix.hollow.core.read.iterator.HollowMapEntryOrdinalIteratorImpl; @@ -44,7 +56,7 @@ public void iteratesMapEntries() throws IOException { int numRecords = 200; int[][][] mapContents = generateMapContents(numRecords); HollowMapTypeReadState readState = populateTypeStateWith(mapContents); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); HollowMapEntryOrdinalIterator iterator = readState.ordinalIterator(0); assertTrue(iterator instanceof HollowMapSnapshotEntryOrdinalIterator); assertTrue(iterator instanceof HollowMapEntryOrdinalIteratorImpl); @@ -56,12 +68,37 @@ public void iteratesMapEntries() throws IOException { assertTrue(sawEntries); } + @Test + public void capturesSizeAndBoundsTogetherAndPreservesSampling() throws IOException { + HollowMapTypeReadState readState = spy(populateTypeStateWith( + new int[][][] { {}, {{0, 1}} })); + readStateEngine.setShardCursorIterators(true); + readState.setSamplingDirector(new EnabledSamplingDirector()); + HollowMapEntryOrdinalIterator empty = readState.ordinalIterator(0); + assertSame(EmptyMapOrdinalIterator.INSTANCE, empty); + assertFalse(empty.next()); + HollowMapEntryOrdinalIterator nonempty = readState.ordinalIterator(1); + verify(readState, times(1)).readWasUnsafe(any(), eq(0), any()); + verify(readState, times(1)).readWasUnsafe(any(), eq(1), any()); + verify(readState, never()).size(0); + verify(readState, never()).size(1); + assertTrue(nonempty.next()); + assertEquals(0, nonempty.getKey()); + assertEquals(1, nonempty.getValue()); + for (SampleResult result : readState.getSampler().getSampleResults()) { + if (result.getIdentifier().endsWith(".size()")) + assertEquals(3, result.getNumSamples()); + if (result.getIdentifier().endsWith(".iterator()")) + assertEquals(2, result.getNumSamples()); + } + } + @Test(timeout = 60_000) public void iterateConcurrentlyWhileResharding() throws Exception { int numRecords = 500; int[][][] mapContents = generateMapContents(numRecords); final HollowMapTypeReadState readState = populateTypeStateWith(mapContents); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); final HollowTypeReshardingStrategy reshardingStrategy = HollowTypeReshardingStrategy.getInstance(readState); final int numReaders = 6; diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIteratorTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIteratorTest.java index 8d53318b95..d16e0b5606 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIteratorTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/set/HollowSetSnapshotOrdinalIteratorTest.java @@ -1,8 +1,19 @@ package com.netflix.hollow.core.read.engine.set; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.junit.Assert.fail; +import com.netflix.hollow.api.sampling.EnabledSamplingDirector; +import com.netflix.hollow.api.sampling.SampleResult; +import com.netflix.hollow.core.read.iterator.EmptyOrdinalIterator; import com.netflix.hollow.core.read.engine.HollowTypeReshardingStrategy; import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator; import com.netflix.hollow.core.read.iterator.HollowSetOrdinalIterator; @@ -56,7 +67,7 @@ public void iteratesEmptyAndSingleAndMultiElementSets() throws IOException { int numRecords = 50; int[][] setContents = generateSetContents(numRecords); HollowSetTypeReadState readState = populateTypeStateWith(setContents); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); HollowOrdinalIterator iterator = readState.ordinalIterator(0); assertTrue(iterator instanceof HollowSetSnapshotOrdinalIterator); assertTrue(iterator instanceof HollowSetOrdinalIterator); @@ -73,12 +84,34 @@ public void iteratesEmptyAndSingleAndMultiElementSets() throws IOException { assertTrue(sawLarge); } + @Test + public void capturesSizeAndBoundsTogetherAndPreservesSampling() throws IOException { + HollowSetTypeReadState readState = spy(populateTypeStateWith(new int[][] { {}, {0} })); + readStateEngine.setShardCursorIterators(true); + readState.setSamplingDirector(new EnabledSamplingDirector()); + HollowOrdinalIterator empty = readState.ordinalIterator(0); + assertSame(EmptyOrdinalIterator.INSTANCE, empty); + assertEquals(HollowOrdinalIterator.NO_MORE_ORDINALS, empty.next()); + HollowOrdinalIterator nonempty = readState.ordinalIterator(1); + verify(readState, times(1)).readWasUnsafe(any(), eq(0), any()); + verify(readState, times(1)).readWasUnsafe(any(), eq(1), any()); + verify(readState, never()).size(0); + verify(readState, never()).size(1); + assertEquals(0, nonempty.next()); + for (SampleResult result : readState.getSampler().getSampleResults()) { + if (result.getIdentifier().endsWith(".size()")) + assertEquals(3, result.getNumSamples()); + if (result.getIdentifier().endsWith(".iterator()")) + assertEquals(2, result.getNumSamples()); + } + } + @Test(timeout = 60_000) public void iterateConcurrentlyWhileResharding() throws Exception { int numRecords = 500; int[][] setContents = generateSetContents(numRecords); final HollowSetTypeReadState readState = populateTypeStateWith(setContents); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); final HollowTypeReshardingStrategy reshardingStrategy = HollowTypeReshardingStrategy.getInstance(readState); final int numReaders = 6; diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/iterator/HollowListSnapshotOrdinalIteratorTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/iterator/HollowListSnapshotOrdinalIteratorTest.java index 95318129eb..7514c53816 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/read/iterator/HollowListSnapshotOrdinalIteratorTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/read/iterator/HollowListSnapshotOrdinalIteratorTest.java @@ -55,7 +55,7 @@ public void iteratesEmptyAndSingleAndMultiElementLists() throws IOException { int numRecords = 50; int[][] listContents = generateListContents(numRecords); HollowListTypeReadState readState = populateTypeStateWith(listContents); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); boolean sawSizeOne = false; boolean sawLarge = false; @@ -72,7 +72,7 @@ public void iteratesEmptyAndSingleAndMultiElementLists() throws IOException { @Test public void iteratorExhaustsCleanlyAndRepeatsNoMoreOrdinals() throws IOException { HollowListTypeReadState readState = populateTypeStateWith(generateListContents(10)); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); HollowOrdinalIterator iter = readState.ordinalIterator(0); int count = 0; while (iter.next() != HollowOrdinalIterator.NO_MORE_ORDINALS) { @@ -87,7 +87,7 @@ public void iteratorExhaustsCleanlyAndRepeatsNoMoreOrdinals() throws IOException @Test public void iteratorMatchesPerElementReads() throws IOException { HollowListTypeReadState readState = populateTypeStateWith(generateListContents(100)); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); for (int ordinal = 0; ordinal <= readState.maxOrdinal(); ordinal++) { int size = readState.size(ordinal); IntList iterated = drain(readState, ordinal); @@ -104,7 +104,7 @@ public void iterateConcurrentlyWhileResharding() throws Exception { int numRecords = 1000; int[][] listContents = generateListContents(numRecords); final HollowListTypeReadState readState = populateTypeStateWith(listContents); - readStateEngine.setSnapshotCollectionIterators(true); + readStateEngine.setShardCursorIterators(true); final HollowTypeReshardingStrategy reshardingStrategy = HollowTypeReshardingStrategy.getInstance(readState); final int numReaders = 6; From 34e3eb6f8205863f950af3e0bd89d9d86fc72b14 Mon Sep 17 00:00:00 2001 From: Danny Thomas Date: Mon, 5 Oct 2026 15:24:21 +1100 Subject: [PATCH 4/6] Check ordinal reuse with both iterator recycler modes --- .../core/read/engine/list/HollowListTypeReadStateTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java index 4fc8dac6c4..bd160658cc 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java @@ -96,7 +96,7 @@ public void iteratorRefreshesRecycledShardAcrossRefresh() throws Exception { } @Test - public void recycledIteratorExhaustsWhenOrdinalIsReusedByShorterList() throws Exception { + public void iteratorHandlesOrdinalReusedByShorterList() throws Exception { populateWriteStateEngine(3); populateWriteStateEngineWithListRecords(new int[][] {{0, 1, 2}}); roundTripSnapshot(); @@ -113,6 +113,11 @@ public void recycledIteratorExhaustsWhenOrdinalIsReusedByShorterList() throws Ex roundTripDelta(); assertEquals(1, typeState.size(0)); assertEquals(1, typeState.getElementOrdinal(0, 0)); + if(!recycling) { + // Immutable cursors retain the original list even when its ordinal is reused. + assertEquals(1, iterator.next()); + assertEquals(2, iterator.next()); + } assertEquals(HollowOrdinalIterator.NO_MORE_ORDINALS, iterator.next()); assertEquals(HollowOrdinalIterator.NO_MORE_ORDINALS, iterator.next()); } From 370f6a3743e2ecd8940a666b57fe92d8f4613ba4 Mon Sep 17 00:00:00 2001 From: Danny Thomas Date: Tue, 6 Oct 2026 12:24:14 +1100 Subject: [PATCH 5/6] Preserve get-based traversal for custom list implementations 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. --- .../hollow/api/objects/HollowList.java | 4 +- .../api/perfapi/HollowPerfBackedList.java | 9 +-- .../api/objects/HollowListIteratorTest.java | 56 +++++++++++++++++-- .../api/perfapi/HollowPerfBackedListTest.java | 11 ++++ 4 files changed, 67 insertions(+), 13 deletions(-) diff --git a/hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java b/hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java index 1c5edd4cc1..1dafa84238 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java +++ b/hollow/src/main/java/com/netflix/hollow/api/objects/HollowList.java @@ -59,7 +59,9 @@ public final T get(int index) { public Iterator iterator() { if(!HollowListTraversal.isEnabled(delegate.getTypeDataAccess())) return super.iterator(); - return HollowListTraversal.iterator(delegate.iterator(ordinal), this::instantiateElement); + HollowListOrdinalIterator cursor = delegate.ordinalCursor(ordinal); + return cursor == null ? super.iterator() + : HollowListTraversal.iterator(cursor, this::instantiateElement); } @Override diff --git a/hollow/src/main/java/com/netflix/hollow/api/perfapi/HollowPerfBackedList.java b/hollow/src/main/java/com/netflix/hollow/api/perfapi/HollowPerfBackedList.java index 397aa1cbd1..44fd82f735 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/perfapi/HollowPerfBackedList.java +++ b/hollow/src/main/java/com/netflix/hollow/api/perfapi/HollowPerfBackedList.java @@ -24,7 +24,6 @@ import java.util.Iterator; import java.util.ListIterator; import java.util.Spliterator; -import java.util.Spliterators; import java.util.RandomAccess; public class HollowPerfBackedList extends AbstractList implements RandomAccess { @@ -54,7 +53,7 @@ public int size() { @Override public Iterator iterator() { - if(!HollowListTraversal.isEnabled(dataAccess)) + if(getClass() != HollowPerfBackedList.class || !HollowListTraversal.isEnabled(dataAccess)) return super.iterator(); return HollowListTraversal.iterator(dataAccess.ordinalIterator(ordinal), this::instantiateElement); } @@ -72,12 +71,8 @@ public ListIterator listIterator(int index) { @Override public Spliterator spliterator() { - if(!HollowListTraversal.isEnabled(dataAccess)) + if(getClass() != HollowPerfBackedList.class || !HollowListTraversal.isEnabled(dataAccess)) return super.spliterator(); - if(getClass() != HollowPerfBackedList.class) { - // Java 8's default spliterator calls iterator(), which bypasses a subclass's get(). - return Spliterators.spliterator(super.listIterator(0), size(), Spliterator.ORDERED); - } HollowOrdinalIterator cursor = dataAccess.ordinalIterator(ordinal); return cursor instanceof HollowListOrdinalIterator ? HollowListTraversal.spliterator((HollowListOrdinalIterator)cursor, this::instantiateElement) diff --git a/hollow/src/test/java/com/netflix/hollow/api/objects/HollowListIteratorTest.java b/hollow/src/test/java/com/netflix/hollow/api/objects/HollowListIteratorTest.java index db4d801d62..03e4b51a8c 100644 --- a/hollow/src/test/java/com/netflix/hollow/api/objects/HollowListIteratorTest.java +++ b/hollow/src/test/java/com/netflix/hollow/api/objects/HollowListIteratorTest.java @@ -55,11 +55,13 @@ public void usesGetBasedTraversalWithoutOptIn() { } @Test - public void iteratesUsingOrdinalIterator() { + public void iteratesUsingOrdinalCursor() { HollowListTypeDataAccess dataAccess = enabledDataAccess(); - HollowOrdinalIterator ordinalIterator = mock(HollowOrdinalIterator.class); + HollowListOrdinalIterator ordinalIterator = mock(HollowListOrdinalIterator.class); when(dataAccess.ordinalIterator(7)).thenReturn(ordinalIterator); - when(ordinalIterator.next()).thenReturn(3, 5, HollowOrdinalIterator.NO_MORE_ORDINALS); + when(ordinalIterator.size()).thenReturn(2); + when(ordinalIterator.getElementOrdinal(0)).thenReturn(3); + when(ordinalIterator.getElementOrdinal(1)).thenReturn(5); HollowList list = list(new HollowListLookupDelegate<>(dataAccess)); @@ -168,7 +170,7 @@ public void cachedListTraversalDoesNotReturnToLiveData() { } @Test - public void lookupSubclassRetainsItsGetBasedListIterator() { + public void lookupSubclassRetainsItsGetBasedTraversal() { HollowListTypeDataAccess dataAccess = enabledDataAccess(); when(dataAccess.size(7)).thenReturn(1); when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); @@ -180,7 +182,38 @@ public Integer get(HollowList list, int ordinal, int index) { return 99; } }); - assertEquals(Integer.valueOf(99), list.listIterator().next()); + assertTraversalMatchesGet(list, Collections.singletonList(99)); + verify(dataAccess, never()).ordinalIterator(7); + } + + @Test + public void cachedSubclassRetainsItsGetBasedTraversal() { + HollowListTypeDataAccess dataAccess = enabledDataAccess(); + when(dataAccess.size(7)).thenReturn(1); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); + HollowList list = list(new HollowListCachedDelegate(dataAccess, 7) { + @Override + public Integer get(HollowList list, int ordinal, int index) { + return 99; + } + }); + + assertTraversalMatchesGet(list, Collections.singletonList(99)); + verify(dataAccess, never()).ordinalIterator(7); + } + + @Test + public void retainsGetBasedTraversalWithoutIndexedCursor() { + HollowListTypeDataAccess dataAccess = enabledDataAccess(); + when(dataAccess.size(7)).thenReturn(1); + when(dataAccess.getElementOrdinal(7, 0)).thenReturn(3); + HollowOrdinalIterator ordinalIterator = mock(HollowOrdinalIterator.class); + when(ordinalIterator.next()).thenReturn(99, HollowOrdinalIterator.NO_MORE_ORDINALS); + when(dataAccess.ordinalIterator(7)).thenReturn(ordinalIterator); + HollowList list = list(new HollowListLookupDelegate<>(dataAccess)); + + assertTraversalMatchesGet(list, Collections.singletonList(3)); + verify(ordinalIterator, never()).next(); } @Test @@ -252,6 +285,19 @@ public void cachedTraversalWorksAfterLiveDataAccessIsDisabled() { assertEquals(Arrays.asList(3, 5), list.stream().collect(java.util.stream.Collectors.toList())); } + private void assertTraversalMatchesGet(List list, List expected) { + assertEquals(expected.get(0), list.get(0)); + assertEquals(expected.get(0), list.listIterator().next()); + assertEquals(expected.get(0), list.iterator().next()); + assertEquals(expected, list.stream().collect(java.util.stream.Collectors.toList())); + assertEquals(expected, list.parallelStream().collect(java.util.stream.Collectors.toList())); + assertEquals(expected, new ArrayList<>(list)); + assertEquals(expected, list); + assertEquals(list, expected); + assertEquals(expected.hashCode(), list.hashCode()); + assertEquals(expected.toString(), list.toString()); + } + private HollowListTypeDataAccess enabledDataAccess() { HollowListTypeReadState type = mock(HollowListTypeReadState.class); HollowReadStateEngine engine = new HollowReadStateEngine(); diff --git a/hollow/src/test/java/com/netflix/hollow/api/perfapi/HollowPerfBackedListTest.java b/hollow/src/test/java/com/netflix/hollow/api/perfapi/HollowPerfBackedListTest.java index 7902f19b51..30027eaa8f 100644 --- a/hollow/src/test/java/com/netflix/hollow/api/perfapi/HollowPerfBackedListTest.java +++ b/hollow/src/test/java/com/netflix/hollow/api/perfapi/HollowPerfBackedListTest.java @@ -91,8 +91,19 @@ public Long get(int index) { return 99L; } }; + java.util.List expected = Collections.singletonList(99L); + assertEquals(Long.valueOf(99), list.get(0)); assertEquals(Long.valueOf(99), list.listIterator().next()); + assertEquals(Long.valueOf(99), list.iterator().next()); assertTrue(list.spliterator().tryAdvance(value -> assertEquals(Long.valueOf(99), value))); + assertEquals(expected, list.stream().collect(java.util.stream.Collectors.toList())); + assertEquals(expected, list.parallelStream().collect(java.util.stream.Collectors.toList())); + assertEquals(expected, new java.util.ArrayList<>(list)); + assertEquals(expected, list); + assertEquals(list, expected); + assertEquals(expected.hashCode(), list.hashCode()); + assertEquals(expected.toString(), list.toString()); + verify(dataAccess, never()).ordinalIterator(7); } @Test From d8beb2ece1bb2844c9121cc6cfce42100e171267 Mon Sep 17 00:00:00 2001 From: Danny Thomas Date: Wed, 7 Oct 2026 11:43:48 +1100 Subject: [PATCH 6/6] Expose shard cursors through experimental feature opt-ins --- .../api/client/HollowClientUpdater.java | 5 +-- .../hollow/api/consumer/HollowConsumer.java | 2 +- .../core/read/engine/ExperimentalFeature.java | 5 ++- .../read/engine/HollowReadStateEngine.java | 4 ++- ...ollowConsumerExperimentalFeaturesTest.java | 31 ++++++++++++++++++- .../list/HollowListTraversalSnapshotTest.java | 6 +++- .../list/HollowListTypeReadStateTest.java | 5 +-- 7 files changed, 49 insertions(+), 9 deletions(-) diff --git a/hollow/src/main/java/com/netflix/hollow/api/client/HollowClientUpdater.java b/hollow/src/main/java/com/netflix/hollow/api/client/HollowClientUpdater.java index e26aa19a16..0540eadd24 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/client/HollowClientUpdater.java +++ b/hollow/src/main/java/com/netflix/hollow/api/client/HollowClientUpdater.java @@ -66,7 +66,7 @@ public class HollowClientUpdater { private boolean skipTypeShardUpdateWithNoAdditions; private ExperimentalFeature[] experimentalFeatures = new ExperimentalFeature[0]; private MemoryRecyclingMode memoryRecyclingMode = MemoryRecyclingMode.AUTO; - private boolean shardCursorIterators; + private Boolean shardCursorIterators; private TypeFilter filter; @@ -367,7 +367,8 @@ private HollowReadStateEngine newStateEngine() { stateEngine = new HollowReadStateEngine(hashCodeFinder, true, new HollowReadConfiguration(memoryMode, memoryRecyclingMode.createRecycler(), experimentalFeatures)); } - stateEngine.setShardCursorIterators(shardCursorIterators); + if(shardCursorIterators != null) + stateEngine.setShardCursorIterators(shardCursorIterators); return stateEngine; } diff --git a/hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java b/hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java index e4bdc12caf..aeebd092e5 100644 --- a/hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java +++ b/hollow/src/main/java/com/netflix/hollow/api/consumer/HollowConsumer.java @@ -1488,7 +1488,7 @@ public B withSkipTypeShardUpdateWithNoAdditions() { */ public B withShardCursorIterators() { this.shardCursorIterators = true; - return (B)this; + return withExperimentalFeatures(ExperimentalFeature.SHARD_CURSOR_ITERATORS); } @Deprecated diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/ExperimentalFeature.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/ExperimentalFeature.java index 2e86afac37..1328e0028c 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/ExperimentalFeature.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/ExperimentalFeature.java @@ -17,5 +17,8 @@ public enum ExperimentalFeature { SHARD_READ_FAST_PATHS, /** Direct decoding and equality checks against immutable on-heap byte segments. */ - DIRECT_SEGMENT_STRING_READS + DIRECT_SEGMENT_STRING_READS, + + /** Capture collection shard bounds once for ordinal cursors and Java list traversal. */ + SHARD_CURSOR_ITERATORS } diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java index 58b1f8dd06..a6e5d1f2eb 100644 --- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java +++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java @@ -106,6 +106,7 @@ public HollowReadStateEngine(HollowObjectHashCodeFinder hashCodeFinder, boolean this.listeners = new HashMap>(); this.hashCodeFinder = hashCodeFinder; this.memoryRecycler = readConfiguration.getMemoryRecycler(); + this.shardCursorIterators = readConfiguration.isExperimentalFeatureEnabled(ExperimentalFeature.SHARD_CURSOR_ITERATORS); this.listenToAllPopulatedOrdinals = listenToAllPopulatedOrdinals; } @@ -242,7 +243,8 @@ public HollowReadConfiguration getReadConfiguration() { } public boolean isExperimentalFeatureEnabled(ExperimentalFeature feature) { - return readConfiguration.isExperimentalFeatureEnabled(feature); + return feature == ExperimentalFeature.SHARD_CURSOR_ITERATORS ? shardCursorIterators + : readConfiguration.isExperimentalFeatureEnabled(feature); } public ArraySegmentRecycler getMemoryRecycler() { diff --git a/hollow/src/test/java/com/netflix/hollow/api/consumer/HollowConsumerExperimentalFeaturesTest.java b/hollow/src/test/java/com/netflix/hollow/api/consumer/HollowConsumerExperimentalFeaturesTest.java index 5dd4fb50c3..e9e3a7d721 100644 --- a/hollow/src/test/java/com/netflix/hollow/api/consumer/HollowConsumerExperimentalFeaturesTest.java +++ b/hollow/src/test/java/com/netflix/hollow/api/consumer/HollowConsumerExperimentalFeaturesTest.java @@ -25,7 +25,8 @@ public class HollowConsumerExperimentalFeaturesTest { public void featuresDefaultToOff() throws IOException { TestHollowConsumer consumer = builder().build(); loadSnapshot(consumer, 1L); - assertFalse(consumer.getStateEngine().isExperimentalFeatureEnabled(SHARD_READ_FAST_PATHS)); + for(ExperimentalFeature feature : ExperimentalFeature.values()) + assertFalse(consumer.getStateEngine().isExperimentalFeatureEnabled(feature)); } @Test @@ -103,6 +104,34 @@ public void stringAndShardOptInsAreIndependent() throws IOException { assertTrue(both.getStateEngine().isExperimentalFeatureEnabled(SHARD_READ_FAST_PATHS)); } + @Test + public void allFeatureCombinationsSurviveReplacementSnapshots() throws IOException { + ExperimentalFeature[] features = ExperimentalFeature.values(); + for(int mask = 0; mask < (1 << features.length); mask++) { + TestHollowConsumer.Builder builder = builder(); + for(int i = 0; i < features.length; i++) { + if((mask & (1 << i)) != 0) + builder.withExperimentalFeatures(features[i]); + } + TestHollowConsumer consumer = builder.build(); + loadSnapshot(consumer, 1L); + loadSnapshot(consumer, 2L); + for(int i = 0; i < features.length; i++) + assertEquals((mask & (1 << i)) != 0, consumer.getStateEngine().isExperimentalFeatureEnabled(features[i])); + assertEquals((mask & (1 << ExperimentalFeature.SHARD_CURSOR_ITERATORS.ordinal())) != 0, + consumer.getStateEngine().isShardCursorIteratorsEnabled()); + } + } + + @Test + public void iteratorConvenienceMethodOnlyOptsInToIterators() throws IOException { + TestHollowConsumer consumer = builder().withShardCursorIterators().build(); + loadSnapshot(consumer, 1L); + assertTrue(consumer.getStateEngine().isExperimentalFeatureEnabled(ExperimentalFeature.SHARD_CURSOR_ITERATORS)); + assertFalse(consumer.getStateEngine().isExperimentalFeatureEnabled(SHARD_READ_FAST_PATHS)); + assertFalse(consumer.getStateEngine().isExperimentalFeatureEnabled(DIRECT_SEGMENT_STRING_READS)); + } + @Test(expected = NullPointerException.class) public void rejectsNullFeature() { builder().withExperimentalFeatures((ExperimentalFeature)null); diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTraversalSnapshotTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTraversalSnapshotTest.java index 679f14176b..3d49ccbf39 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTraversalSnapshotTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTraversalSnapshotTest.java @@ -14,6 +14,9 @@ import com.netflix.hollow.api.perfapi.HollowListTypePerfAPI; import com.netflix.hollow.api.perfapi.HollowPerfBackedList; import com.netflix.hollow.core.memory.pool.WastefulRecycler; +import com.netflix.hollow.core.memory.MemoryMode; +import com.netflix.hollow.core.read.engine.ExperimentalFeature; +import com.netflix.hollow.core.read.engine.HollowReadConfiguration; import com.netflix.hollow.core.read.engine.HollowReadStateEngine; import com.netflix.hollow.core.read.engine.HollowTypeReshardingStrategy; import com.netflix.hollow.core.util.StateEngineRoundTripper; @@ -146,7 +149,8 @@ public void parallelSplitsValidateIndependentlyDuringResharding() throws Excepti private HollowListTypeReadState immutableState() throws Exception { populateWriteStateEngine(3); populateWriteStateEngineWithListRecords(new int[][] {{0, 1, 2}}); - readStateEngine = new HollowReadStateEngine(WastefulRecycler.DEFAULT_INSTANCE); + readStateEngine = new HollowReadStateEngine(new HollowReadConfiguration(MemoryMode.ON_HEAP, + WastefulRecycler.DEFAULT_INSTANCE, ExperimentalFeature.SHARD_READ_FAST_PATHS)); readStateEngine.setShardCursorIterators(true); StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, null); return (HollowListTypeReadState) readStateEngine.getTypeState("TestList"); diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java index bd160658cc..aa0f436cf9 100644 --- a/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java +++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeReadStateTest.java @@ -113,8 +113,9 @@ public void iteratorHandlesOrdinalReusedByShorterList() throws Exception { roundTripDelta(); assertEquals(1, typeState.size(0)); assertEquals(1, typeState.getElementOrdinal(0, 0)); - if(!recycling) { - // Immutable cursors retain the original list even when its ordinal is reused. + if(!recycling && shardReadFastPaths) { + // Validation elision lets immutable cursors retain the list when its ordinal is reused. + // Without that opt-in, validation refreshes the cursor to the shorter live list. assertEquals(1, iterator.next()); assertEquals(2, iterator.next()); }