From 7fa6ad4314c9d1a2d3d42637025e370e5879e35b Mon Sep 17 00:00:00 2001 From: leventov Date: Fri, 15 Sep 2017 18:57:04 -0500 Subject: [PATCH 1/4] Remove IndexedInts.iterator() --- .../DistinctCountAggregator.java | 5 +- .../DistinctCountBufferAggregator.java | 5 +- ...alityAggregatorColumnSelectorStrategy.java | 6 +- .../query/groupby/GroupByQueryEngine.java | 3 +- .../CompressedVSizeIndexedSupplier.java | 8 --- .../io/druid/segment/IntIteratorUtils.java | 35 +---------- .../segment/StringDimensionMergerV9.java | 22 +++++-- .../segment/data/ArrayBasedIndexedInts.java | 8 --- .../data/BitmapCompressedIndexedInts.java | 8 --- .../data/CompressedIntsIndexedSupplier.java | 7 --- .../CompressedVSizeIntsIndexedSupplier.java | 7 --- .../druid/segment/data/EmptyIndexedInts.java | 8 --- .../io/druid/segment/data/IndexedInts.java | 15 ++++- .../segment/data/IndexedIntsIterator.java | 61 ------------------- .../druid/segment/data/RangeIndexedInts.java | 8 --- .../druid/segment/data/SingleIndexedInt.java | 8 --- .../druid/segment/data/VSizeIndexedInts.java | 7 --- .../druid/segment/data/ZeroIndexedInts.java | 8 --- .../incremental/IncrementalIndexAdapter.java | 8 --- .../CardinalityAggregatorTest.java | 8 --- .../ConstantDimensionSelectorTest.java | 7 --- .../io/druid/segment/IndexMergerTestBase.java | 5 +- .../CompressedVSizeIndexedSupplierTest.java | 7 +-- .../IncrementalIndexStorageAdapterTest.java | 16 ++--- 24 files changed, 53 insertions(+), 227 deletions(-) delete mode 100644 processing/src/main/java/io/druid/segment/data/IndexedIntsIterator.java diff --git a/extensions-contrib/distinctcount/src/main/java/io/druid/query/aggregation/distinctcount/DistinctCountAggregator.java b/extensions-contrib/distinctcount/src/main/java/io/druid/query/aggregation/distinctcount/DistinctCountAggregator.java index 13ab3fc2bba3..b6b55d24607a 100644 --- a/extensions-contrib/distinctcount/src/main/java/io/druid/query/aggregation/distinctcount/DistinctCountAggregator.java +++ b/extensions-contrib/distinctcount/src/main/java/io/druid/query/aggregation/distinctcount/DistinctCountAggregator.java @@ -22,6 +22,7 @@ import io.druid.collections.bitmap.MutableBitmap; import io.druid.query.aggregation.Aggregator; import io.druid.segment.DimensionSelector; +import io.druid.segment.data.IndexedInts; public class DistinctCountAggregator implements Aggregator { @@ -41,7 +42,9 @@ public DistinctCountAggregator( @Override public void aggregate() { - for (final Integer index : selector.getRow()) { + IndexedInts row = selector.getRow(); + for (int i = 0; i < row.size(); i++) { + int index = row.get(i); mutableBitmap.add(index); } } diff --git a/extensions-contrib/distinctcount/src/main/java/io/druid/query/aggregation/distinctcount/DistinctCountBufferAggregator.java b/extensions-contrib/distinctcount/src/main/java/io/druid/query/aggregation/distinctcount/DistinctCountBufferAggregator.java index 5c21597177b1..db61ea34b841 100644 --- a/extensions-contrib/distinctcount/src/main/java/io/druid/query/aggregation/distinctcount/DistinctCountBufferAggregator.java +++ b/extensions-contrib/distinctcount/src/main/java/io/druid/query/aggregation/distinctcount/DistinctCountBufferAggregator.java @@ -24,6 +24,7 @@ import io.druid.query.aggregation.BufferAggregator; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import io.druid.segment.DimensionSelector; +import io.druid.segment.data.IndexedInts; import it.unimi.dsi.fastutil.ints.Int2ObjectMap; import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap; @@ -51,7 +52,9 @@ public void init(ByteBuffer buf, int position) public void aggregate(ByteBuffer buf, int position) { MutableBitmap mutableBitmap = getMutableBitmap(buf, position); - for (final Integer index : selector.getRow()) { + IndexedInts row = selector.getRow(); + for (int i = 0; i < row.size(); i++) { + int index = row.get(i); mutableBitmap.add(index); } buf.putLong(position, mutableBitmap.size()); diff --git a/processing/src/main/java/io/druid/query/aggregation/cardinality/types/StringCardinalityAggregatorColumnSelectorStrategy.java b/processing/src/main/java/io/druid/query/aggregation/cardinality/types/StringCardinalityAggregatorColumnSelectorStrategy.java index fb78d6c2453f..71661abf91b9 100644 --- a/processing/src/main/java/io/druid/query/aggregation/cardinality/types/StringCardinalityAggregatorColumnSelectorStrategy.java +++ b/processing/src/main/java/io/druid/query/aggregation/cardinality/types/StringCardinalityAggregatorColumnSelectorStrategy.java @@ -24,7 +24,6 @@ import io.druid.query.aggregation.cardinality.CardinalityAggregator; import io.druid.segment.DimensionSelector; import io.druid.segment.data.IndexedInts; -import it.unimi.dsi.fastutil.ints.IntIterator; import java.util.Arrays; @@ -62,8 +61,9 @@ public void hashRow(DimensionSelector dimSelector, Hasher hasher) @Override public void hashValues(DimensionSelector dimSelector, HyperLogLogCollector collector) { - for (IntIterator rowIt = dimSelector.getRow().iterator(); rowIt.hasNext(); ) { - int index = rowIt.nextInt(); + IndexedInts row = dimSelector.getRow(); + for (int i = 0; i < row.size(); i++) { + int index = row.get(i); final String value = dimSelector.lookupName(index); collector.add(CardinalityAggregator.hashFn.hashUnencodedChars(nullToSpecial(value)).asBytes()); } diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java index 298bee8b96b3..8191d00b916f 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryEngine.java @@ -196,8 +196,9 @@ private List updateValues( newKey.putInt(MISSING_VALUE); unaggregatedBuffers = updateValues(newKey, dims.subList(1, dims.size())); } else { - for (Integer dimValue : row) { + for (int i = 0; i < row.size(); i++) { ByteBuffer newKey = key.duplicate(); + int dimValue = row.get(i); newKey.putInt(dimValue); unaggregatedBuffers = updateValues(newKey, dims.subList(1, dims.size())); } diff --git a/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java b/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java index e8c7e2e95d1a..8e1079cf63f9 100644 --- a/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java +++ b/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java @@ -25,11 +25,9 @@ import io.druid.segment.data.CompressedObjectStrategy; import io.druid.segment.data.CompressedVSizeIntsIndexedSupplier; import io.druid.segment.data.IndexedInts; -import io.druid.segment.data.IndexedIntsIterator; import io.druid.segment.data.IndexedIterable; import io.druid.segment.data.IndexedMultivalue; import io.druid.segment.data.WritableSupplier; -import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.IOException; import java.nio.ByteBuffer; @@ -206,12 +204,6 @@ public void close() throws IOException // no-op } - @Override - public IntIterator iterator() - { - return new IndexedIntsIterator(this); - } - @Override public void inspectRuntimeShape(RuntimeShapeInspector inspector) { diff --git a/processing/src/main/java/io/druid/segment/IntIteratorUtils.java b/processing/src/main/java/io/druid/segment/IntIteratorUtils.java index 98a56d02da1a..7739d5020aee 100644 --- a/processing/src/main/java/io/druid/segment/IntIteratorUtils.java +++ b/processing/src/main/java/io/druid/segment/IntIteratorUtils.java @@ -43,7 +43,7 @@ public static int skip(IntIterator it, int n) } int skipped = 0; while (skipped < n && it.hasNext()) { - it.next(); + it.nextInt(); skipped++; } return skipped; @@ -164,39 +164,6 @@ public int skip(int n) } } - public static IntIterator fromRoaringBitmapIntIterator(org.roaringbitmap.IntIterator iterator) - { - return new RoaringBitmapDelegatingIntIterator(iterator); - } - - private static class RoaringBitmapDelegatingIntIterator extends AbstractIntIterator - { - private final org.roaringbitmap.IntIterator delegate; - - private RoaringBitmapDelegatingIntIterator(org.roaringbitmap.IntIterator delegate) - { - this.delegate = delegate; - } - - @Override - public boolean hasNext() - { - return delegate.hasNext(); - } - - @Override - public int nextInt() - { - return delegate.next(); - } - - @Override - public int skip(int n) - { - return IntIteratorUtils.skip(this, n); - } - } - public static IntList toIntList(IntIterator iterator) { final IntList integers = new IntArrayList(); diff --git a/processing/src/main/java/io/druid/segment/StringDimensionMergerV9.java b/processing/src/main/java/io/druid/segment/StringDimensionMergerV9.java index 887620c36ac6..a5499647af20 100644 --- a/processing/src/main/java/io/druid/segment/StringDimensionMergerV9.java +++ b/processing/src/main/java/io/druid/segment/StringDimensionMergerV9.java @@ -31,6 +31,7 @@ import io.druid.collections.spatial.RTree; import io.druid.collections.spatial.split.LinearGutmanSplitStrategy; import io.druid.java.util.common.ByteBufferUtils; +import io.druid.java.util.common.IAE; import io.druid.java.util.common.ISE; import io.druid.java.util.common.StringUtils; import io.druid.java.util.common.io.Closer; @@ -565,25 +566,38 @@ public int get(int index) @Override public IntIterator iterator() { - final IntIterator baseIterator = baseIndex.iterator(); return new AbstractIntIterator() { + final int limit = baseIndex.size(); + int i = 0; + @Override public boolean hasNext() { - return baseIterator.hasNext(); + return i < limit; } @Override public int nextInt() { - return conversionBuffer.get(baseIterator.nextInt()); + return conversionBuffer.get(baseIndex.get(i++)); } @Override public int skip(int n) { - return IntIteratorUtils.skip(baseIterator, n); + if (n < 0) { + throw new IAE("n: " + n); + } + // overflow-aware code + int remaining = limit - i; + if (remaining <= n) { + i = n; + return remaining; + } else { + i += n; + return n; + } } }; } diff --git a/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java b/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java index 4c3302b1c9f5..9caca269c29a 100644 --- a/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java @@ -22,8 +22,6 @@ import io.druid.java.util.common.IAE; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import it.unimi.dsi.fastutil.ints.IntArrays; -import it.unimi.dsi.fastutil.ints.IntIterator; -import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -76,12 +74,6 @@ public int get(int index) return expansion[index]; } - @Override - public IntIterator iterator() - { - return IntIterators.wrap(expansion, 0, size); - } - @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/BitmapCompressedIndexedInts.java b/processing/src/main/java/io/druid/segment/data/BitmapCompressedIndexedInts.java index fabbbfcd82c7..6665396b1d02 100644 --- a/processing/src/main/java/io/druid/segment/data/BitmapCompressedIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/BitmapCompressedIndexedInts.java @@ -22,8 +22,6 @@ import com.google.common.collect.Ordering; import io.druid.collections.bitmap.ImmutableBitmap; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import io.druid.segment.IntIteratorUtils; -import it.unimi.dsi.fastutil.ints.IntIterator; import javax.annotation.Nullable; import java.io.IOException; @@ -82,12 +80,6 @@ public ImmutableBitmap getImmutableBitmap() return immutableBitmap; } - @Override - public IntIterator iterator() - { - return IntIteratorUtils.fromRoaringBitmapIntIterator(immutableBitmap.iterator()); - } - @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/CompressedIntsIndexedSupplier.java b/processing/src/main/java/io/druid/segment/data/CompressedIntsIndexedSupplier.java index 88affd110c4e..7292c78e47b8 100644 --- a/processing/src/main/java/io/druid/segment/data/CompressedIntsIndexedSupplier.java +++ b/processing/src/main/java/io/druid/segment/data/CompressedIntsIndexedSupplier.java @@ -29,7 +29,6 @@ import io.druid.java.util.common.io.smoosh.SmooshedFileMapper; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import io.druid.segment.CompressedPools; -import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.IOException; import java.nio.ByteBuffer; @@ -311,12 +310,6 @@ public int get(int index) return buffer.get(buffer.position() + bufferIndex); } - @Override - public IntIterator iterator() - { - return new IndexedIntsIterator(this); - } - protected void loadBuffer(int bufferNum) { CloseQuietly.close(holder); diff --git a/processing/src/main/java/io/druid/segment/data/CompressedVSizeIntsIndexedSupplier.java b/processing/src/main/java/io/druid/segment/data/CompressedVSizeIntsIndexedSupplier.java index c9a3c82d6f8c..9a4eabf79795 100644 --- a/processing/src/main/java/io/druid/segment/data/CompressedVSizeIntsIndexedSupplier.java +++ b/processing/src/main/java/io/druid/segment/data/CompressedVSizeIntsIndexedSupplier.java @@ -31,7 +31,6 @@ import io.druid.java.util.common.io.smoosh.SmooshedFileMapper; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import io.druid.segment.CompressedPools; -import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.IOException; import java.nio.ByteBuffer; @@ -374,12 +373,6 @@ protected int _get(final int index) buffer.getInt(pos) & littleEndianMask; } - @Override - public IntIterator iterator() - { - return new IndexedIntsIterator(this); - } - protected void loadBuffer(int bufferNum) { CloseQuietly.close(holder); diff --git a/processing/src/main/java/io/druid/segment/data/EmptyIndexedInts.java b/processing/src/main/java/io/druid/segment/data/EmptyIndexedInts.java index ae493acbca12..0bd0724c020a 100644 --- a/processing/src/main/java/io/druid/segment/data/EmptyIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/EmptyIndexedInts.java @@ -20,8 +20,6 @@ package io.druid.segment.data; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import it.unimi.dsi.fastutil.ints.IntIterator; -import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -47,12 +45,6 @@ public int get(int index) throw new UnsupportedOperationException(); } - @Override - public IntIterator iterator() - { - return IntIterators.EMPTY_ITERATOR; - } - @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/IndexedInts.java b/processing/src/main/java/io/druid/segment/data/IndexedInts.java index 2fc65b24e0c1..8374f662f078 100644 --- a/processing/src/main/java/io/druid/segment/data/IndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/IndexedInts.java @@ -21,17 +21,28 @@ import io.druid.query.monomorphicprocessing.CalledFromHotLoop; import io.druid.query.monomorphicprocessing.HotLoopCallee; -import it.unimi.dsi.fastutil.ints.IntIterable; import java.io.Closeable; +import java.util.function.IntConsumer; /** * Get a int an index (array or list lookup abstraction without boxing). + * + * Doesn't extend {@link Iterable} (or {@link it.unimi.dsi.fastutil.ints.IntIterable} to avoid accidential boxing + * for-each iteration. */ -public interface IndexedInts extends IntIterable, Closeable, HotLoopCallee +public interface IndexedInts extends Closeable, HotLoopCallee { @CalledFromHotLoop int size(); @CalledFromHotLoop int get(int index); + + default void forEach(IntConsumer action) + { + int size = size(); + for (int i = 0; i < size; i++) { + action.accept(get(i)); + } + } } diff --git a/processing/src/main/java/io/druid/segment/data/IndexedIntsIterator.java b/processing/src/main/java/io/druid/segment/data/IndexedIntsIterator.java deleted file mode 100644 index 267c7fde50cb..000000000000 --- a/processing/src/main/java/io/druid/segment/data/IndexedIntsIterator.java +++ /dev/null @@ -1,61 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets licenses this file - * to you 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 io.druid.segment.data; - - -import io.druid.segment.IntIteratorUtils; -import it.unimi.dsi.fastutil.ints.AbstractIntIterator; - -/** - */ -public class IndexedIntsIterator extends AbstractIntIterator -{ - private final IndexedInts baseInts; - private final int size; - - private int currIndex = 0; - - public IndexedIntsIterator( - IndexedInts baseInts - ) - { - this.baseInts = baseInts; - - size = baseInts.size(); - } - - @Override - public boolean hasNext() - { - return currIndex < size; - } - - @Override - public int nextInt() - { - return baseInts.get(currIndex++); - } - - @Override - public int skip(int n) - { - return IntIteratorUtils.skip(this, n); - } -} diff --git a/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java b/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java index e5e85a70787e..c02118afa807 100644 --- a/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java @@ -21,8 +21,6 @@ import com.google.common.base.Preconditions; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import it.unimi.dsi.fastutil.ints.IntIterator; -import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -72,12 +70,6 @@ public int get(int index) return index; } - @Override - public IntIterator iterator() - { - return IntIterators.fromTo(0, size); - } - @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java b/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java index d3d8427f1ab1..c8ed042ce004 100644 --- a/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java +++ b/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java @@ -20,8 +20,6 @@ package io.druid.segment.data; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import it.unimi.dsi.fastutil.ints.IntIterator; -import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -49,12 +47,6 @@ public int get(int i) return value; } - @Override - public IntIterator iterator() - { - return IntIterators.singleton(value); - } - @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/VSizeIndexedInts.java b/processing/src/main/java/io/druid/segment/data/VSizeIndexedInts.java index 46d37cd72c10..421264cd1041 100644 --- a/processing/src/main/java/io/druid/segment/data/VSizeIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/VSizeIndexedInts.java @@ -23,7 +23,6 @@ import com.google.common.primitives.Ints; import io.druid.java.util.common.IAE; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.IOException; import java.nio.ByteBuffer; @@ -177,12 +176,6 @@ public long getSerializedSize() return 1 + 1 + 4 + buffer.remaining(); } - @Override - public IntIterator iterator() - { - return new IndexedIntsIterator(this); - } - public void writeToChannel(WritableByteChannel channel) throws IOException { channel.write(ByteBuffer.wrap(new byte[]{VERSION, (byte) numBytes})); diff --git a/processing/src/main/java/io/druid/segment/data/ZeroIndexedInts.java b/processing/src/main/java/io/druid/segment/data/ZeroIndexedInts.java index 7e361a0e7313..a2a16fbcae8e 100644 --- a/processing/src/main/java/io/druid/segment/data/ZeroIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/ZeroIndexedInts.java @@ -20,8 +20,6 @@ package io.druid.segment.data; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import it.unimi.dsi.fastutil.ints.IntIterator; -import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -56,12 +54,6 @@ public int get(int index) return 0; } - @Override - public IntIterator iterator() - { - return IntIterators.singleton(0); - } - @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexAdapter.java b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexAdapter.java index f0de26bf4e02..523241123dee 100644 --- a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexAdapter.java +++ b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexAdapter.java @@ -29,7 +29,6 @@ import io.druid.segment.DimensionHandler; import io.druid.segment.DimensionIndexer; import io.druid.segment.IndexableAdapter; -import io.druid.segment.IntIteratorUtils; import io.druid.segment.Metadata; import io.druid.segment.Rowboat; import io.druid.segment.column.ColumnCapabilities; @@ -37,7 +36,6 @@ import io.druid.segment.data.Indexed; import io.druid.segment.data.IndexedInts; import io.druid.segment.data.ListIndexed; -import it.unimi.dsi.fastutil.ints.IntIterator; import org.joda.time.Interval; import java.io.IOException; @@ -285,12 +283,6 @@ public int get(int index) throw new UnsupportedOperationException("Not supported."); } - @Override - public IntIterator iterator() - { - return IntIteratorUtils.fromRoaringBitmapIntIterator(bitmapIndex.iterator()); - } - @Override public void close() throws IOException { diff --git a/processing/src/test/java/io/druid/query/aggregation/cardinality/CardinalityAggregatorTest.java b/processing/src/test/java/io/druid/query/aggregation/cardinality/CardinalityAggregatorTest.java index 362f14285a2a..444eb83541cc 100644 --- a/processing/src/test/java/io/druid/query/aggregation/cardinality/CardinalityAggregatorTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/cardinality/CardinalityAggregatorTest.java @@ -48,8 +48,6 @@ import io.druid.segment.DimensionSelectorUtils; import io.druid.segment.IdLookup; import io.druid.segment.data.IndexedInts; -import it.unimi.dsi.fastutil.ints.IntIterator; -import it.unimi.dsi.fastutil.ints.IntIterators; import org.junit.Assert; import org.junit.Test; @@ -142,12 +140,6 @@ public int get(int i) return column.get(p)[i]; } - @Override - public IntIterator iterator() - { - return IntIterators.asIntIterator(Iterators.forArray(column.get(p))); - } - @Override public void close() throws IOException { diff --git a/processing/src/test/java/io/druid/segment/ConstantDimensionSelectorTest.java b/processing/src/test/java/io/druid/segment/ConstantDimensionSelectorTest.java index 344c4938c8f4..abac95cf0159 100644 --- a/processing/src/test/java/io/druid/segment/ConstantDimensionSelectorTest.java +++ b/processing/src/test/java/io/druid/segment/ConstantDimensionSelectorTest.java @@ -25,8 +25,6 @@ import org.junit.Assert; import org.junit.Test; -import java.util.Iterator; - public class ConstantDimensionSelectorTest { private final DimensionSelector NULL_SELECTOR = DimensionSelectorUtils.constantSelector(null); @@ -46,11 +44,6 @@ public void testGetRow() throws Exception IndexedInts row = NULL_SELECTOR.getRow(); Assert.assertEquals(1, row.size()); Assert.assertEquals(0, row.get(0)); - - Iterator iter = row.iterator(); - Assert.assertEquals(true, iter.hasNext()); - Assert.assertEquals(0, iter.next().intValue()); - Assert.assertEquals(false, iter.hasNext()); } @Test diff --git a/processing/src/test/java/io/druid/segment/IndexMergerTestBase.java b/processing/src/test/java/io/druid/segment/IndexMergerTestBase.java index f5196d062a52..15b0cc417e84 100644 --- a/processing/src/test/java/io/druid/segment/IndexMergerTestBase.java +++ b/processing/src/test/java/io/druid/segment/IndexMergerTestBase.java @@ -1523,9 +1523,8 @@ public void testNoRollupMergeWithDuplicateRow() throws Exception private void checkBitmapIndex(ArrayList expected, IndexedInts real) { Assert.assertEquals(expected.size(), real.size()); - int i = 0; - for (Object index : real) { - Assert.assertEquals(expected.get(i++), index); + for (int i = 0; i < real.size(); i++) { + Assert.assertEquals(expected.get(i), (Integer) real.get(i)); } } diff --git a/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedSupplierTest.java b/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedSupplierTest.java index dd9554da4857..914247c8497c 100644 --- a/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedSupplierTest.java +++ b/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedSupplierTest.java @@ -115,11 +115,8 @@ public void testIterators() final IndexedInts vSizeIndexedInts = iterator.next(); Assert.assertEquals(ints.length, vSizeIndexedInts.size()); - Iterator valsIterator = vSizeIndexedInts.iterator(); - int j = 0; - while (valsIterator.hasNext()) { - Assert.assertEquals((Integer) ints[j], valsIterator.next()); - j++; + for (int i = 0; i < vSizeIndexedInts.size(); i++) { + Assert.assertEquals(ints[i], vSizeIndexedInts.get(i)); } row++; } diff --git a/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java b/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java index d3cf377e4421..f35ce01fffb6 100644 --- a/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java +++ b/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java @@ -459,9 +459,7 @@ public Object apply(Cursor cursor) // and then, cursoring continues in the other thread while (!cursor.isDone()) { IndexedInts row = dimSelector.getRow(); - for (int i : row) { - Assert.assertTrue(i < cardinality); - } + row.forEach(i -> Assert.assertTrue(i < cardinality)); cursor.advance(); rowNumInCursor++; } @@ -593,17 +591,11 @@ public Object apply(Cursor cursor) // and then, cursoring continues in the other thread while (!cursor.isDone()) { IndexedInts rowA = dimSelector1A.getRow(); - for (int i : rowA) { - Assert.assertTrue(i < cardinalityA); - } + rowA.forEach(i -> Assert.assertTrue(i < cardinalityA)); IndexedInts rowB = dimSelector1B.getRow(); - for (int i : rowB) { - Assert.assertTrue(i < cardinalityA); - } + rowB.forEach(i -> Assert.assertTrue(i < cardinalityA)); IndexedInts rowC = dimSelector1C.getRow(); - for (int i : rowC) { - Assert.assertTrue(i < cardinalityA); - } + rowC.forEach(i -> Assert.assertTrue(i < cardinalityA)); IndexedInts rowD = dimSelector2D.getRow(); // no null id, so should get empty dims array Assert.assertEquals(0, rowD.size()); From d15489be18b01518f738ce22355b96a6e1bb681f Mon Sep 17 00:00:00 2001 From: leventov Date: Sat, 16 Sep 2017 13:24:10 -0500 Subject: [PATCH 2/4] Retain IndexedInts.iterator(), but don't extend Iterable --- .../CompressedVSizeIndexedSupplier.java | 8 +++ .../io/druid/segment/IntIteratorUtils.java | 33 ++++++++++ .../segment/StringDimensionMergerV9.java | 22 ++----- .../segment/data/ArrayBasedIndexedInts.java | 8 +++ .../data/BitmapCompressedIndexedInts.java | 8 +++ .../data/CompressedIntsIndexedSupplier.java | 7 +++ .../CompressedVSizeIntsIndexedSupplier.java | 7 +++ .../druid/segment/data/EmptyIndexedInts.java | 8 +++ .../io/druid/segment/data/IndexedInts.java | 7 ++- .../segment/data/IndexedIntsIterator.java | 61 +++++++++++++++++++ .../druid/segment/data/RangeIndexedInts.java | 8 +++ .../druid/segment/data/SingleIndexedInt.java | 8 +++ .../druid/segment/data/VSizeIndexedInts.java | 7 +++ .../druid/segment/data/ZeroIndexedInts.java | 8 +++ .../incremental/IncrementalIndexAdapter.java | 8 +++ .../CardinalityAggregatorTest.java | 8 +++ .../ConstantDimensionSelectorTest.java | 7 +++ .../io/druid/segment/IndexMergerTestBase.java | 7 ++- .../CompressedVSizeIndexedSupplierTest.java | 7 ++- 19 files changed, 213 insertions(+), 24 deletions(-) create mode 100644 processing/src/main/java/io/druid/segment/data/IndexedIntsIterator.java diff --git a/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java b/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java index 8e1079cf63f9..e8c7e2e95d1a 100644 --- a/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java +++ b/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java @@ -25,9 +25,11 @@ import io.druid.segment.data.CompressedObjectStrategy; import io.druid.segment.data.CompressedVSizeIntsIndexedSupplier; import io.druid.segment.data.IndexedInts; +import io.druid.segment.data.IndexedIntsIterator; import io.druid.segment.data.IndexedIterable; import io.druid.segment.data.IndexedMultivalue; import io.druid.segment.data.WritableSupplier; +import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.IOException; import java.nio.ByteBuffer; @@ -204,6 +206,12 @@ public void close() throws IOException // no-op } + @Override + public IntIterator iterator() + { + return new IndexedIntsIterator(this); + } + @Override public void inspectRuntimeShape(RuntimeShapeInspector inspector) { diff --git a/processing/src/main/java/io/druid/segment/IntIteratorUtils.java b/processing/src/main/java/io/druid/segment/IntIteratorUtils.java index 7739d5020aee..8e5232336146 100644 --- a/processing/src/main/java/io/druid/segment/IntIteratorUtils.java +++ b/processing/src/main/java/io/druid/segment/IntIteratorUtils.java @@ -164,6 +164,39 @@ public int skip(int n) } } + public static IntIterator fromRoaringBitmapIntIterator(org.roaringbitmap.IntIterator iterator) + { + return new RoaringBitmapDelegatingIntIterator(iterator); + } + + private static class RoaringBitmapDelegatingIntIterator extends AbstractIntIterator + { + private final org.roaringbitmap.IntIterator delegate; + + private RoaringBitmapDelegatingIntIterator(org.roaringbitmap.IntIterator delegate) + { + this.delegate = delegate; + } + + @Override + public boolean hasNext() + { + return delegate.hasNext(); + } + + @Override + public int nextInt() + { + return delegate.next(); + } + + @Override + public int skip(int n) + { + return IntIteratorUtils.skip(this, n); + } + } + public static IntList toIntList(IntIterator iterator) { final IntList integers = new IntArrayList(); diff --git a/processing/src/main/java/io/druid/segment/StringDimensionMergerV9.java b/processing/src/main/java/io/druid/segment/StringDimensionMergerV9.java index a5499647af20..887620c36ac6 100644 --- a/processing/src/main/java/io/druid/segment/StringDimensionMergerV9.java +++ b/processing/src/main/java/io/druid/segment/StringDimensionMergerV9.java @@ -31,7 +31,6 @@ import io.druid.collections.spatial.RTree; import io.druid.collections.spatial.split.LinearGutmanSplitStrategy; import io.druid.java.util.common.ByteBufferUtils; -import io.druid.java.util.common.IAE; import io.druid.java.util.common.ISE; import io.druid.java.util.common.StringUtils; import io.druid.java.util.common.io.Closer; @@ -566,38 +565,25 @@ public int get(int index) @Override public IntIterator iterator() { + final IntIterator baseIterator = baseIndex.iterator(); return new AbstractIntIterator() { - final int limit = baseIndex.size(); - int i = 0; - @Override public boolean hasNext() { - return i < limit; + return baseIterator.hasNext(); } @Override public int nextInt() { - return conversionBuffer.get(baseIndex.get(i++)); + return conversionBuffer.get(baseIterator.nextInt()); } @Override public int skip(int n) { - if (n < 0) { - throw new IAE("n: " + n); - } - // overflow-aware code - int remaining = limit - i; - if (remaining <= n) { - i = n; - return remaining; - } else { - i += n; - return n; - } + return IntIteratorUtils.skip(baseIterator, n); } }; } diff --git a/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java b/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java index 9caca269c29a..4c3302b1c9f5 100644 --- a/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java @@ -22,6 +22,8 @@ import io.druid.java.util.common.IAE; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import it.unimi.dsi.fastutil.ints.IntArrays; +import it.unimi.dsi.fastutil.ints.IntIterator; +import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -74,6 +76,12 @@ public int get(int index) return expansion[index]; } + @Override + public IntIterator iterator() + { + return IntIterators.wrap(expansion, 0, size); + } + @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/BitmapCompressedIndexedInts.java b/processing/src/main/java/io/druid/segment/data/BitmapCompressedIndexedInts.java index 6665396b1d02..fabbbfcd82c7 100644 --- a/processing/src/main/java/io/druid/segment/data/BitmapCompressedIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/BitmapCompressedIndexedInts.java @@ -22,6 +22,8 @@ import com.google.common.collect.Ordering; import io.druid.collections.bitmap.ImmutableBitmap; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; +import io.druid.segment.IntIteratorUtils; +import it.unimi.dsi.fastutil.ints.IntIterator; import javax.annotation.Nullable; import java.io.IOException; @@ -80,6 +82,12 @@ public ImmutableBitmap getImmutableBitmap() return immutableBitmap; } + @Override + public IntIterator iterator() + { + return IntIteratorUtils.fromRoaringBitmapIntIterator(immutableBitmap.iterator()); + } + @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/CompressedIntsIndexedSupplier.java b/processing/src/main/java/io/druid/segment/data/CompressedIntsIndexedSupplier.java index 7292c78e47b8..88affd110c4e 100644 --- a/processing/src/main/java/io/druid/segment/data/CompressedIntsIndexedSupplier.java +++ b/processing/src/main/java/io/druid/segment/data/CompressedIntsIndexedSupplier.java @@ -29,6 +29,7 @@ import io.druid.java.util.common.io.smoosh.SmooshedFileMapper; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import io.druid.segment.CompressedPools; +import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.IOException; import java.nio.ByteBuffer; @@ -310,6 +311,12 @@ public int get(int index) return buffer.get(buffer.position() + bufferIndex); } + @Override + public IntIterator iterator() + { + return new IndexedIntsIterator(this); + } + protected void loadBuffer(int bufferNum) { CloseQuietly.close(holder); diff --git a/processing/src/main/java/io/druid/segment/data/CompressedVSizeIntsIndexedSupplier.java b/processing/src/main/java/io/druid/segment/data/CompressedVSizeIntsIndexedSupplier.java index 9a4eabf79795..c9a3c82d6f8c 100644 --- a/processing/src/main/java/io/druid/segment/data/CompressedVSizeIntsIndexedSupplier.java +++ b/processing/src/main/java/io/druid/segment/data/CompressedVSizeIntsIndexedSupplier.java @@ -31,6 +31,7 @@ import io.druid.java.util.common.io.smoosh.SmooshedFileMapper; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import io.druid.segment.CompressedPools; +import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.IOException; import java.nio.ByteBuffer; @@ -373,6 +374,12 @@ protected int _get(final int index) buffer.getInt(pos) & littleEndianMask; } + @Override + public IntIterator iterator() + { + return new IndexedIntsIterator(this); + } + protected void loadBuffer(int bufferNum) { CloseQuietly.close(holder); diff --git a/processing/src/main/java/io/druid/segment/data/EmptyIndexedInts.java b/processing/src/main/java/io/druid/segment/data/EmptyIndexedInts.java index 0bd0724c020a..ae493acbca12 100644 --- a/processing/src/main/java/io/druid/segment/data/EmptyIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/EmptyIndexedInts.java @@ -20,6 +20,8 @@ package io.druid.segment.data; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; +import it.unimi.dsi.fastutil.ints.IntIterator; +import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -45,6 +47,12 @@ public int get(int index) throw new UnsupportedOperationException(); } + @Override + public IntIterator iterator() + { + return IntIterators.EMPTY_ITERATOR; + } + @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/IndexedInts.java b/processing/src/main/java/io/druid/segment/data/IndexedInts.java index 8374f662f078..cb81136e1276 100644 --- a/processing/src/main/java/io/druid/segment/data/IndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/IndexedInts.java @@ -21,6 +21,7 @@ import io.druid.query.monomorphicprocessing.CalledFromHotLoop; import io.druid.query.monomorphicprocessing.HotLoopCallee; +import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.Closeable; import java.util.function.IntConsumer; @@ -28,8 +29,8 @@ /** * Get a int an index (array or list lookup abstraction without boxing). * - * Doesn't extend {@link Iterable} (or {@link it.unimi.dsi.fastutil.ints.IntIterable} to avoid accidential boxing - * for-each iteration. + * Doesn't extend {@link Iterable} (or {@link it.unimi.dsi.fastutil.ints.IntIterable} to avoid accidential + * for-each iteration with boxing. */ public interface IndexedInts extends Closeable, HotLoopCallee { @@ -38,6 +39,8 @@ public interface IndexedInts extends Closeable, HotLoopCallee @CalledFromHotLoop int get(int index); + IntIterator iterator(); + default void forEach(IntConsumer action) { int size = size(); diff --git a/processing/src/main/java/io/druid/segment/data/IndexedIntsIterator.java b/processing/src/main/java/io/druid/segment/data/IndexedIntsIterator.java new file mode 100644 index 000000000000..267c7fde50cb --- /dev/null +++ b/processing/src/main/java/io/druid/segment/data/IndexedIntsIterator.java @@ -0,0 +1,61 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you 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 io.druid.segment.data; + + +import io.druid.segment.IntIteratorUtils; +import it.unimi.dsi.fastutil.ints.AbstractIntIterator; + +/** + */ +public class IndexedIntsIterator extends AbstractIntIterator +{ + private final IndexedInts baseInts; + private final int size; + + private int currIndex = 0; + + public IndexedIntsIterator( + IndexedInts baseInts + ) + { + this.baseInts = baseInts; + + size = baseInts.size(); + } + + @Override + public boolean hasNext() + { + return currIndex < size; + } + + @Override + public int nextInt() + { + return baseInts.get(currIndex++); + } + + @Override + public int skip(int n) + { + return IntIteratorUtils.skip(this, n); + } +} diff --git a/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java b/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java index c02118afa807..e5e85a70787e 100644 --- a/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java @@ -21,6 +21,8 @@ import com.google.common.base.Preconditions; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; +import it.unimi.dsi.fastutil.ints.IntIterator; +import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -70,6 +72,12 @@ public int get(int index) return index; } + @Override + public IntIterator iterator() + { + return IntIterators.fromTo(0, size); + } + @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java b/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java index c8ed042ce004..d3d8427f1ab1 100644 --- a/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java +++ b/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java @@ -20,6 +20,8 @@ package io.druid.segment.data; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; +import it.unimi.dsi.fastutil.ints.IntIterator; +import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -47,6 +49,12 @@ public int get(int i) return value; } + @Override + public IntIterator iterator() + { + return IntIterators.singleton(value); + } + @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/VSizeIndexedInts.java b/processing/src/main/java/io/druid/segment/data/VSizeIndexedInts.java index 421264cd1041..46d37cd72c10 100644 --- a/processing/src/main/java/io/druid/segment/data/VSizeIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/VSizeIndexedInts.java @@ -23,6 +23,7 @@ import com.google.common.primitives.Ints; import io.druid.java.util.common.IAE; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; +import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.IOException; import java.nio.ByteBuffer; @@ -176,6 +177,12 @@ public long getSerializedSize() return 1 + 1 + 4 + buffer.remaining(); } + @Override + public IntIterator iterator() + { + return new IndexedIntsIterator(this); + } + public void writeToChannel(WritableByteChannel channel) throws IOException { channel.write(ByteBuffer.wrap(new byte[]{VERSION, (byte) numBytes})); diff --git a/processing/src/main/java/io/druid/segment/data/ZeroIndexedInts.java b/processing/src/main/java/io/druid/segment/data/ZeroIndexedInts.java index a2a16fbcae8e..7e361a0e7313 100644 --- a/processing/src/main/java/io/druid/segment/data/ZeroIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/ZeroIndexedInts.java @@ -20,6 +20,8 @@ package io.druid.segment.data; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; +import it.unimi.dsi.fastutil.ints.IntIterator; +import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -54,6 +56,12 @@ public int get(int index) return 0; } + @Override + public IntIterator iterator() + { + return IntIterators.singleton(0); + } + @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexAdapter.java b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexAdapter.java index 523241123dee..f0de26bf4e02 100644 --- a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexAdapter.java +++ b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexAdapter.java @@ -29,6 +29,7 @@ import io.druid.segment.DimensionHandler; import io.druid.segment.DimensionIndexer; import io.druid.segment.IndexableAdapter; +import io.druid.segment.IntIteratorUtils; import io.druid.segment.Metadata; import io.druid.segment.Rowboat; import io.druid.segment.column.ColumnCapabilities; @@ -36,6 +37,7 @@ import io.druid.segment.data.Indexed; import io.druid.segment.data.IndexedInts; import io.druid.segment.data.ListIndexed; +import it.unimi.dsi.fastutil.ints.IntIterator; import org.joda.time.Interval; import java.io.IOException; @@ -283,6 +285,12 @@ public int get(int index) throw new UnsupportedOperationException("Not supported."); } + @Override + public IntIterator iterator() + { + return IntIteratorUtils.fromRoaringBitmapIntIterator(bitmapIndex.iterator()); + } + @Override public void close() throws IOException { diff --git a/processing/src/test/java/io/druid/query/aggregation/cardinality/CardinalityAggregatorTest.java b/processing/src/test/java/io/druid/query/aggregation/cardinality/CardinalityAggregatorTest.java index 444eb83541cc..362f14285a2a 100644 --- a/processing/src/test/java/io/druid/query/aggregation/cardinality/CardinalityAggregatorTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/cardinality/CardinalityAggregatorTest.java @@ -48,6 +48,8 @@ import io.druid.segment.DimensionSelectorUtils; import io.druid.segment.IdLookup; import io.druid.segment.data.IndexedInts; +import it.unimi.dsi.fastutil.ints.IntIterator; +import it.unimi.dsi.fastutil.ints.IntIterators; import org.junit.Assert; import org.junit.Test; @@ -140,6 +142,12 @@ public int get(int i) return column.get(p)[i]; } + @Override + public IntIterator iterator() + { + return IntIterators.asIntIterator(Iterators.forArray(column.get(p))); + } + @Override public void close() throws IOException { diff --git a/processing/src/test/java/io/druid/segment/ConstantDimensionSelectorTest.java b/processing/src/test/java/io/druid/segment/ConstantDimensionSelectorTest.java index abac95cf0159..344c4938c8f4 100644 --- a/processing/src/test/java/io/druid/segment/ConstantDimensionSelectorTest.java +++ b/processing/src/test/java/io/druid/segment/ConstantDimensionSelectorTest.java @@ -25,6 +25,8 @@ import org.junit.Assert; import org.junit.Test; +import java.util.Iterator; + public class ConstantDimensionSelectorTest { private final DimensionSelector NULL_SELECTOR = DimensionSelectorUtils.constantSelector(null); @@ -44,6 +46,11 @@ public void testGetRow() throws Exception IndexedInts row = NULL_SELECTOR.getRow(); Assert.assertEquals(1, row.size()); Assert.assertEquals(0, row.get(0)); + + Iterator iter = row.iterator(); + Assert.assertEquals(true, iter.hasNext()); + Assert.assertEquals(0, iter.next().intValue()); + Assert.assertEquals(false, iter.hasNext()); } @Test diff --git a/processing/src/test/java/io/druid/segment/IndexMergerTestBase.java b/processing/src/test/java/io/druid/segment/IndexMergerTestBase.java index 15b0cc417e84..77627890b444 100644 --- a/processing/src/test/java/io/druid/segment/IndexMergerTestBase.java +++ b/processing/src/test/java/io/druid/segment/IndexMergerTestBase.java @@ -57,6 +57,7 @@ import io.druid.segment.incremental.IncrementalIndexAdapter; import io.druid.segment.incremental.IncrementalIndexSchema; import io.druid.segment.incremental.IndexSizeExceededException; +import it.unimi.dsi.fastutil.ints.IntIterator; import org.joda.time.Interval; import org.junit.Assert; import org.junit.Rule; @@ -1523,8 +1524,10 @@ public void testNoRollupMergeWithDuplicateRow() throws Exception private void checkBitmapIndex(ArrayList expected, IndexedInts real) { Assert.assertEquals(expected.size(), real.size()); - for (int i = 0; i < real.size(); i++) { - Assert.assertEquals(expected.get(i), (Integer) real.get(i)); + int i = 0; + for (IntIterator iterator = real.iterator(); iterator.hasNext(); ) { + int index = iterator.nextInt(); + Assert.assertEquals(expected.get(i++), (Integer) index); } } diff --git a/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedSupplierTest.java b/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedSupplierTest.java index 914247c8497c..dd9554da4857 100644 --- a/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedSupplierTest.java +++ b/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedSupplierTest.java @@ -115,8 +115,11 @@ public void testIterators() final IndexedInts vSizeIndexedInts = iterator.next(); Assert.assertEquals(ints.length, vSizeIndexedInts.size()); - for (int i = 0; i < vSizeIndexedInts.size(); i++) { - Assert.assertEquals(ints[i], vSizeIndexedInts.get(i)); + Iterator valsIterator = vSizeIndexedInts.iterator(); + int j = 0; + while (valsIterator.hasNext()) { + Assert.assertEquals((Integer) ints[j], valsIterator.next()); + j++; } row++; } From 9a809e378c5039da236e29c20d5306e8d42783e2 Mon Sep 17 00:00:00 2001 From: leventov Date: Sat, 16 Sep 2017 21:01:47 -0500 Subject: [PATCH 3/4] Add BitmapValues --- .../CompressedVSizeIndexedSupplier.java | 8 -- .../io/druid/segment/IndexableAdapter.java | 4 +- .../QueryableIndexIndexableAdapter.java | 49 +++++---- .../segment/RowboatFilteringIndexAdapter.java | 6 +- .../segment/StringDimensionMergerV9.java | 33 +++--- .../segment/data/ArrayBasedIndexedInts.java | 8 -- .../data/BitmapCompressedIndexedInts.java | 101 ------------------ .../io/druid/segment/data/BitmapValues.java | 48 +++++++++ .../data/CompressedIntsIndexedSupplier.java | 7 -- .../CompressedVSizeIntsIndexedSupplier.java | 7 -- .../druid/segment/data/EmptyIndexedInts.java | 8 -- ...erator.java => ImmutableBitmapValues.java} | 33 ++---- .../io/druid/segment/data/IndexedInts.java | 3 - .../druid/segment/data/RangeIndexedInts.java | 8 -- .../druid/segment/data/SingleIndexedInt.java | 8 -- .../druid/segment/data/VSizeIndexedInts.java | 7 -- .../druid/segment/data/ZeroIndexedInts.java | 8 -- .../incremental/IncrementalIndexAdapter.java | 40 ++----- .../CardinalityAggregatorTest.java | 8 -- .../ConstantDimensionSelectorTest.java | 7 -- .../io/druid/segment/IndexMergerTestBase.java | 4 +- .../QueryableIndexIndexableAdapterTest.java | 4 +- .../CompressedVSizeIndexedSupplierTest.java | 7 +- .../IncrementalIndexAdapterTest.java | 2 +- 24 files changed, 117 insertions(+), 301 deletions(-) delete mode 100644 processing/src/main/java/io/druid/segment/data/BitmapCompressedIndexedInts.java create mode 100644 processing/src/main/java/io/druid/segment/data/BitmapValues.java rename processing/src/main/java/io/druid/segment/data/{IndexedIntsIterator.java => ImmutableBitmapValues.java} (63%) diff --git a/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java b/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java index e8c7e2e95d1a..8e1079cf63f9 100644 --- a/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java +++ b/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java @@ -25,11 +25,9 @@ import io.druid.segment.data.CompressedObjectStrategy; import io.druid.segment.data.CompressedVSizeIntsIndexedSupplier; import io.druid.segment.data.IndexedInts; -import io.druid.segment.data.IndexedIntsIterator; import io.druid.segment.data.IndexedIterable; import io.druid.segment.data.IndexedMultivalue; import io.druid.segment.data.WritableSupplier; -import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.IOException; import java.nio.ByteBuffer; @@ -206,12 +204,6 @@ public void close() throws IOException // no-op } - @Override - public IntIterator iterator() - { - return new IndexedIntsIterator(this); - } - @Override public void inspectRuntimeShape(RuntimeShapeInspector inspector) { diff --git a/processing/src/main/java/io/druid/segment/IndexableAdapter.java b/processing/src/main/java/io/druid/segment/IndexableAdapter.java index cc34913d8525..56c026ba2b9f 100644 --- a/processing/src/main/java/io/druid/segment/IndexableAdapter.java +++ b/processing/src/main/java/io/druid/segment/IndexableAdapter.java @@ -20,8 +20,8 @@ package io.druid.segment; import io.druid.segment.column.ColumnCapabilities; +import io.druid.segment.data.BitmapValues; import io.druid.segment.data.Indexed; -import io.druid.segment.data.IndexedInts; import org.joda.time.Interval; import java.util.Map; @@ -43,7 +43,7 @@ public interface IndexableAdapter Iterable getRows(); - IndexedInts getBitmapIndex(String dimension, int dictId); + BitmapValues getBitmapValues(String dimension, int dictId); String getMetricType(String metric); diff --git a/processing/src/main/java/io/druid/segment/QueryableIndexIndexableAdapter.java b/processing/src/main/java/io/druid/segment/QueryableIndexIndexableAdapter.java index 951929faf8dc..386910371fd7 100644 --- a/processing/src/main/java/io/druid/segment/QueryableIndexIndexableAdapter.java +++ b/processing/src/main/java/io/druid/segment/QueryableIndexIndexableAdapter.java @@ -39,10 +39,9 @@ import io.druid.segment.column.IndexedFloatsGenericColumn; import io.druid.segment.column.IndexedLongsGenericColumn; import io.druid.segment.column.ValueType; -import io.druid.segment.data.BitmapCompressedIndexedInts; -import io.druid.segment.data.EmptyIndexedInts; +import io.druid.segment.data.ImmutableBitmapValues; +import io.druid.segment.data.BitmapValues; import io.druid.segment.data.Indexed; -import io.druid.segment.data.IndexedInts; import io.druid.segment.data.IndexedIterable; import io.druid.segment.data.ListIndexed; import org.joda.time.Interval; @@ -300,23 +299,6 @@ public void remove() }; } - @VisibleForTesting - IndexedInts getBitmapIndex(String dimension, String value) - { - final Column column = input.getColumn(dimension); - - if (column == null) { - return EmptyIndexedInts.EMPTY_INDEXED_INTS; - } - - final BitmapIndex bitmaps = column.getBitmapIndex(); - if (bitmaps == null) { - return EmptyIndexedInts.EMPTY_INDEXED_INTS; - } - - return new BitmapCompressedIndexedInts(bitmaps.getBitmap(bitmaps.getIndex(value))); - } - @Override public String getMetricType(String metric) { @@ -347,25 +329,42 @@ public ColumnCapabilities getCapabilities(String column) } @Override - public IndexedInts getBitmapIndex(String dimension, int dictId) + public BitmapValues getBitmapValues(String dimension, int dictId) { final Column column = input.getColumn(dimension); if (column == null) { - return EmptyIndexedInts.EMPTY_INDEXED_INTS; + return BitmapValues.EMPTY; } final BitmapIndex bitmaps = column.getBitmapIndex(); if (bitmaps == null) { - return EmptyIndexedInts.EMPTY_INDEXED_INTS; + return BitmapValues.EMPTY; } if (dictId >= 0) { - return new BitmapCompressedIndexedInts(bitmaps.getBitmap(dictId)); + return new ImmutableBitmapValues(bitmaps.getBitmap(dictId)); } else { - return EmptyIndexedInts.EMPTY_INDEXED_INTS; + return BitmapValues.EMPTY; } } + @VisibleForTesting + BitmapValues getBitmapIndex(String dimension, String value) + { + final Column column = input.getColumn(dimension); + + if (column == null) { + return BitmapValues.EMPTY; + } + + final BitmapIndex bitmaps = column.getBitmapIndex(); + if (bitmaps == null) { + return BitmapValues.EMPTY; + } + + return new ImmutableBitmapValues(bitmaps.getBitmap(bitmaps.getIndex(value))); + } + @Override public Metadata getMetadata() { diff --git a/processing/src/main/java/io/druid/segment/RowboatFilteringIndexAdapter.java b/processing/src/main/java/io/druid/segment/RowboatFilteringIndexAdapter.java index e9976bf6fde1..590f48b3d926 100644 --- a/processing/src/main/java/io/druid/segment/RowboatFilteringIndexAdapter.java +++ b/processing/src/main/java/io/druid/segment/RowboatFilteringIndexAdapter.java @@ -22,8 +22,8 @@ import com.google.common.base.Predicate; import com.google.common.collect.Iterables; import io.druid.segment.column.ColumnCapabilities; +import io.druid.segment.data.BitmapValues; import io.druid.segment.data.Indexed; -import io.druid.segment.data.IndexedInts; import org.joda.time.Interval; import java.util.Map; @@ -90,9 +90,9 @@ public ColumnCapabilities getCapabilities(String column) } @Override - public IndexedInts getBitmapIndex(String dimension, int dictId) + public BitmapValues getBitmapValues(String dimension, int dictId) { - return baseAdapter.getBitmapIndex(dimension, dictId); + return baseAdapter.getBitmapValues(dimension, dictId); } @Override diff --git a/processing/src/main/java/io/druid/segment/StringDimensionMergerV9.java b/processing/src/main/java/io/druid/segment/StringDimensionMergerV9.java index 887620c36ac6..93ed1a0da8ba 100644 --- a/processing/src/main/java/io/druid/segment/StringDimensionMergerV9.java +++ b/processing/src/main/java/io/druid/segment/StringDimensionMergerV9.java @@ -40,6 +40,7 @@ import io.druid.segment.column.ValueType; import io.druid.segment.data.ArrayIndexed; import io.druid.segment.data.BitmapSerdeFactory; +import io.druid.segment.data.BitmapValues; import io.druid.segment.data.ByteBufferWriter; import io.druid.segment.data.CompressedObjectStrategy; import io.druid.segment.data.CompressedVSizeIndexedV3Writer; @@ -48,7 +49,6 @@ import io.druid.segment.data.GenericIndexedWriter; import io.druid.segment.data.IOPeon; import io.druid.segment.data.Indexed; -import io.druid.segment.data.IndexedInts; import io.druid.segment.data.IndexedIntsWriter; import io.druid.segment.data.IndexedRTree; import io.druid.segment.data.VSizeIndexedIntsWriter; @@ -58,6 +58,7 @@ import it.unimi.dsi.fastutil.ints.IntIterable; import it.unimi.dsi.fastutil.ints.IntIterator; +import javax.annotation.Nonnull; import java.io.Closeable; import java.io.File; import java.io.FileOutputStream; @@ -370,13 +371,14 @@ static void mergeBitmaps( GenericIndexedWriter bitmapWriter ) throws IOException { - List convertedInvertedIndexesToMerge = Lists.newArrayListWithCapacity(adapters.size()); + List convertedInvertedIndexesToMerge = Lists.newArrayListWithCapacity(adapters.size()); for (int j = 0; j < adapters.size(); ++j) { int seekedDictId = dictIdSeeker[j].seek(dictId); if (seekedDictId != IndexSeeker.NOT_EXIST) { convertedInvertedIndexesToMerge.add( - new ConvertingIndexedInts( - adapters.get(j).getBitmapIndex(dimensionName, seekedDictId), segmentRowNumConversions.get(j) + new ConvertingBitmapValues( + adapters.get(j).getBitmapValues(dimensionName, seekedDictId), + segmentRowNumConversions.get(j) ) ); } @@ -384,7 +386,7 @@ static void mergeBitmaps( MutableBitmap mergedIndexes = bmpFactory.makeEmptyMutableBitmap(); List convertedInvertedIndexesIterators = new ArrayList<>(convertedInvertedIndexesToMerge.size()); - for (ConvertingIndexedInts convertedInvertedIndexes : convertedInvertedIndexesToMerge) { + for (ConvertingBitmapValues convertedInvertedIndexes : convertedInvertedIndexesToMerge) { convertedInvertedIndexesIterators.add(convertedInvertedIndexes.iterator()); } @@ -538,34 +540,27 @@ public int seek(int dictId) } } - public static class ConvertingIndexedInts implements IntIterable + public static class ConvertingBitmapValues implements IntIterable { - private final IndexedInts baseIndex; + private final BitmapValues baseValues; private final IntBuffer conversionBuffer; - public ConvertingIndexedInts( - IndexedInts baseIndex, - IntBuffer conversionBuffer - ) + ConvertingBitmapValues(BitmapValues baseValues, IntBuffer conversionBuffer) { - this.baseIndex = baseIndex; + this.baseValues = baseValues; this.conversionBuffer = conversionBuffer; } public int size() { - return baseIndex.size(); - } - - public int get(int index) - { - return conversionBuffer.get(baseIndex.get(index)); + return baseValues.size(); } + @Nonnull @Override public IntIterator iterator() { - final IntIterator baseIterator = baseIndex.iterator(); + final IntIterator baseIterator = baseValues.iterator(); return new AbstractIntIterator() { @Override diff --git a/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java b/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java index 4c3302b1c9f5..9caca269c29a 100644 --- a/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java @@ -22,8 +22,6 @@ import io.druid.java.util.common.IAE; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import it.unimi.dsi.fastutil.ints.IntArrays; -import it.unimi.dsi.fastutil.ints.IntIterator; -import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -76,12 +74,6 @@ public int get(int index) return expansion[index]; } - @Override - public IntIterator iterator() - { - return IntIterators.wrap(expansion, 0, size); - } - @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/BitmapCompressedIndexedInts.java b/processing/src/main/java/io/druid/segment/data/BitmapCompressedIndexedInts.java deleted file mode 100644 index fabbbfcd82c7..000000000000 --- a/processing/src/main/java/io/druid/segment/data/BitmapCompressedIndexedInts.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets licenses this file - * to you 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 io.druid.segment.data; - -import com.google.common.collect.Ordering; -import io.druid.collections.bitmap.ImmutableBitmap; -import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import io.druid.segment.IntIteratorUtils; -import it.unimi.dsi.fastutil.ints.IntIterator; - -import javax.annotation.Nullable; -import java.io.IOException; - -/** - */ -public class BitmapCompressedIndexedInts implements IndexedInts, Comparable -{ - private static final Ordering COMPARATOR = new Ordering() - { - @Override - public int compare( - ImmutableBitmap set, ImmutableBitmap set1 - ) - { - if (set.size() == 0 && set1.size() == 0) { - return 0; - } - if (set.size() == 0) { - return -1; - } - if (set1.size() == 0) { - return 1; - } - return set.compareTo(set1); - } - }.nullsFirst(); - - private final ImmutableBitmap immutableBitmap; - - public BitmapCompressedIndexedInts(ImmutableBitmap immutableBitmap) - { - this.immutableBitmap = immutableBitmap; - } - - @Override - public int compareTo(@Nullable ImmutableBitmap otherBitmap) - { - return COMPARATOR.compare(immutableBitmap, otherBitmap); - } - - @Override - public int size() - { - return immutableBitmap.size(); - } - - @Override - public int get(int index) - { - throw new UnsupportedOperationException("This is really slow, so it's just not supported."); - } - - public ImmutableBitmap getImmutableBitmap() - { - return immutableBitmap; - } - - @Override - public IntIterator iterator() - { - return IntIteratorUtils.fromRoaringBitmapIntIterator(immutableBitmap.iterator()); - } - - @Override - public void close() throws IOException - { - } - - @Override - public void inspectRuntimeShape(RuntimeShapeInspector inspector) - { - inspector.visit("immutableBitmap", immutableBitmap); - } -} diff --git a/processing/src/main/java/io/druid/segment/data/BitmapValues.java b/processing/src/main/java/io/druid/segment/data/BitmapValues.java new file mode 100644 index 000000000000..9d2528b91c5f --- /dev/null +++ b/processing/src/main/java/io/druid/segment/data/BitmapValues.java @@ -0,0 +1,48 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you 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 io.druid.segment.data; + +import it.unimi.dsi.fastutil.ints.IntIterator; +import it.unimi.dsi.fastutil.ints.IntIterators; + +/** + * Doesn't extend {@link it.unimi.dsi.fastutil.ints.IntIterable} to avoid accidential for-each iteration with boxing. + */ +public interface BitmapValues +{ + BitmapValues EMPTY = new BitmapValues() + { + @Override + public int size() + { + return 0; + } + + @Override + public IntIterator iterator() + { + return IntIterators.EMPTY_ITERATOR; + } + }; + + int size(); + + IntIterator iterator(); +} diff --git a/processing/src/main/java/io/druid/segment/data/CompressedIntsIndexedSupplier.java b/processing/src/main/java/io/druid/segment/data/CompressedIntsIndexedSupplier.java index 88affd110c4e..7292c78e47b8 100644 --- a/processing/src/main/java/io/druid/segment/data/CompressedIntsIndexedSupplier.java +++ b/processing/src/main/java/io/druid/segment/data/CompressedIntsIndexedSupplier.java @@ -29,7 +29,6 @@ import io.druid.java.util.common.io.smoosh.SmooshedFileMapper; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import io.druid.segment.CompressedPools; -import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.IOException; import java.nio.ByteBuffer; @@ -311,12 +310,6 @@ public int get(int index) return buffer.get(buffer.position() + bufferIndex); } - @Override - public IntIterator iterator() - { - return new IndexedIntsIterator(this); - } - protected void loadBuffer(int bufferNum) { CloseQuietly.close(holder); diff --git a/processing/src/main/java/io/druid/segment/data/CompressedVSizeIntsIndexedSupplier.java b/processing/src/main/java/io/druid/segment/data/CompressedVSizeIntsIndexedSupplier.java index c9a3c82d6f8c..9a4eabf79795 100644 --- a/processing/src/main/java/io/druid/segment/data/CompressedVSizeIntsIndexedSupplier.java +++ b/processing/src/main/java/io/druid/segment/data/CompressedVSizeIntsIndexedSupplier.java @@ -31,7 +31,6 @@ import io.druid.java.util.common.io.smoosh.SmooshedFileMapper; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import io.druid.segment.CompressedPools; -import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.IOException; import java.nio.ByteBuffer; @@ -374,12 +373,6 @@ protected int _get(final int index) buffer.getInt(pos) & littleEndianMask; } - @Override - public IntIterator iterator() - { - return new IndexedIntsIterator(this); - } - protected void loadBuffer(int bufferNum) { CloseQuietly.close(holder); diff --git a/processing/src/main/java/io/druid/segment/data/EmptyIndexedInts.java b/processing/src/main/java/io/druid/segment/data/EmptyIndexedInts.java index ae493acbca12..0bd0724c020a 100644 --- a/processing/src/main/java/io/druid/segment/data/EmptyIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/EmptyIndexedInts.java @@ -20,8 +20,6 @@ package io.druid.segment.data; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import it.unimi.dsi.fastutil.ints.IntIterator; -import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -47,12 +45,6 @@ public int get(int index) throw new UnsupportedOperationException(); } - @Override - public IntIterator iterator() - { - return IntIterators.EMPTY_ITERATOR; - } - @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/IndexedIntsIterator.java b/processing/src/main/java/io/druid/segment/data/ImmutableBitmapValues.java similarity index 63% rename from processing/src/main/java/io/druid/segment/data/IndexedIntsIterator.java rename to processing/src/main/java/io/druid/segment/data/ImmutableBitmapValues.java index 267c7fde50cb..456ce877a582 100644 --- a/processing/src/main/java/io/druid/segment/data/IndexedIntsIterator.java +++ b/processing/src/main/java/io/druid/segment/data/ImmutableBitmapValues.java @@ -19,43 +19,30 @@ package io.druid.segment.data; - +import io.druid.collections.bitmap.ImmutableBitmap; import io.druid.segment.IntIteratorUtils; -import it.unimi.dsi.fastutil.ints.AbstractIntIterator; +import it.unimi.dsi.fastutil.ints.IntIterator; /** */ -public class IndexedIntsIterator extends AbstractIntIterator +public class ImmutableBitmapValues implements BitmapValues { - private final IndexedInts baseInts; - private final int size; - - private int currIndex = 0; - - public IndexedIntsIterator( - IndexedInts baseInts - ) - { - this.baseInts = baseInts; + private final ImmutableBitmap immutableBitmap; - size = baseInts.size(); - } - - @Override - public boolean hasNext() + public ImmutableBitmapValues(ImmutableBitmap immutableBitmap) { - return currIndex < size; + this.immutableBitmap = immutableBitmap; } @Override - public int nextInt() + public int size() { - return baseInts.get(currIndex++); + return immutableBitmap.size(); } @Override - public int skip(int n) + public IntIterator iterator() { - return IntIteratorUtils.skip(this, n); + return IntIteratorUtils.fromRoaringBitmapIntIterator(immutableBitmap.iterator()); } } diff --git a/processing/src/main/java/io/druid/segment/data/IndexedInts.java b/processing/src/main/java/io/druid/segment/data/IndexedInts.java index cb81136e1276..f94b5eca4b40 100644 --- a/processing/src/main/java/io/druid/segment/data/IndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/IndexedInts.java @@ -21,7 +21,6 @@ import io.druid.query.monomorphicprocessing.CalledFromHotLoop; import io.druid.query.monomorphicprocessing.HotLoopCallee; -import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.Closeable; import java.util.function.IntConsumer; @@ -39,8 +38,6 @@ public interface IndexedInts extends Closeable, HotLoopCallee @CalledFromHotLoop int get(int index); - IntIterator iterator(); - default void forEach(IntConsumer action) { int size = size(); diff --git a/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java b/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java index e5e85a70787e..c02118afa807 100644 --- a/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java @@ -21,8 +21,6 @@ import com.google.common.base.Preconditions; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import it.unimi.dsi.fastutil.ints.IntIterator; -import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -72,12 +70,6 @@ public int get(int index) return index; } - @Override - public IntIterator iterator() - { - return IntIterators.fromTo(0, size); - } - @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java b/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java index d3d8427f1ab1..c8ed042ce004 100644 --- a/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java +++ b/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java @@ -20,8 +20,6 @@ package io.druid.segment.data; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import it.unimi.dsi.fastutil.ints.IntIterator; -import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -49,12 +47,6 @@ public int get(int i) return value; } - @Override - public IntIterator iterator() - { - return IntIterators.singleton(value); - } - @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/data/VSizeIndexedInts.java b/processing/src/main/java/io/druid/segment/data/VSizeIndexedInts.java index 46d37cd72c10..421264cd1041 100644 --- a/processing/src/main/java/io/druid/segment/data/VSizeIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/VSizeIndexedInts.java @@ -23,7 +23,6 @@ import com.google.common.primitives.Ints; import io.druid.java.util.common.IAE; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import it.unimi.dsi.fastutil.ints.IntIterator; import java.io.IOException; import java.nio.ByteBuffer; @@ -177,12 +176,6 @@ public long getSerializedSize() return 1 + 1 + 4 + buffer.remaining(); } - @Override - public IntIterator iterator() - { - return new IndexedIntsIterator(this); - } - public void writeToChannel(WritableByteChannel channel) throws IOException { channel.write(ByteBuffer.wrap(new byte[]{VERSION, (byte) numBytes})); diff --git a/processing/src/main/java/io/druid/segment/data/ZeroIndexedInts.java b/processing/src/main/java/io/druid/segment/data/ZeroIndexedInts.java index 7e361a0e7313..a2a16fbcae8e 100644 --- a/processing/src/main/java/io/druid/segment/data/ZeroIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/ZeroIndexedInts.java @@ -20,8 +20,6 @@ package io.druid.segment.data; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import it.unimi.dsi.fastutil.ints.IntIterator; -import it.unimi.dsi.fastutil.ints.IntIterators; import java.io.IOException; @@ -56,12 +54,6 @@ public int get(int index) return 0; } - @Override - public IntIterator iterator() - { - return IntIterators.singleton(0); - } - @Override public void close() throws IOException { diff --git a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexAdapter.java b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexAdapter.java index f0de26bf4e02..0c50ee4efbea 100644 --- a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexAdapter.java +++ b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexAdapter.java @@ -25,7 +25,6 @@ import io.druid.collections.bitmap.BitmapFactory; import io.druid.collections.bitmap.MutableBitmap; import io.druid.java.util.common.logger.Logger; -import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import io.druid.segment.DimensionHandler; import io.druid.segment.DimensionIndexer; import io.druid.segment.IndexableAdapter; @@ -33,14 +32,12 @@ import io.druid.segment.Metadata; import io.druid.segment.Rowboat; import io.druid.segment.column.ColumnCapabilities; -import io.druid.segment.data.EmptyIndexedInts; +import io.druid.segment.data.BitmapValues; import io.druid.segment.data.Indexed; -import io.druid.segment.data.IndexedInts; import io.druid.segment.data.ListIndexed; import it.unimi.dsi.fastutil.ints.IntIterator; import org.joda.time.Interval; -import java.io.IOException; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -223,31 +220,31 @@ public Rowboat apply(IncrementalIndex.TimeAndDims timeAndDims) } @Override - public IndexedInts getBitmapIndex(String dimension, int index) + public BitmapValues getBitmapValues(String dimension, int index) { DimensionAccessor accessor = accessors.get(dimension); if (accessor == null) { - return EmptyIndexedInts.EMPTY_INDEXED_INTS; + return BitmapValues.EMPTY; } ColumnCapabilities capabilities = accessor.dimensionDesc.getCapabilities(); DimensionIndexer indexer = accessor.dimensionDesc.getIndexer(); if (!capabilities.hasBitmapIndexes()) { - return EmptyIndexedInts.EMPTY_INDEXED_INTS; + return BitmapValues.EMPTY; } final int id = (Integer) indexer.getUnsortedEncodedValueFromSorted(index); if (id < 0 || id >= indexer.getCardinality()) { - return EmptyIndexedInts.EMPTY_INDEXED_INTS; + return BitmapValues.EMPTY; } MutableBitmap bitmapIndex = accessor.invertedIndexes[id]; if (bitmapIndex == null) { - return EmptyIndexedInts.EMPTY_INDEXED_INTS; + return BitmapValues.EMPTY; } - return new BitmapIndexedInts(bitmapIndex); + return new MutableBitmapValues(bitmapIndex); } @Override @@ -262,12 +259,11 @@ public ColumnCapabilities getCapabilities(String column) return index.getCapabilities(column); } - static class BitmapIndexedInts implements IndexedInts + static class MutableBitmapValues implements BitmapValues { - private final MutableBitmap bitmapIndex; - BitmapIndexedInts(MutableBitmap bitmapIndex) + MutableBitmapValues(MutableBitmap bitmapIndex) { this.bitmapIndex = bitmapIndex; } @@ -278,29 +274,11 @@ public int size() return bitmapIndex.size(); } - @Override - public int get(int index) - { - // Slow for concise bitmaps, but is fast with roaring bitmaps, so it's just not supported. - throw new UnsupportedOperationException("Not supported."); - } - @Override public IntIterator iterator() { return IntIteratorUtils.fromRoaringBitmapIntIterator(bitmapIndex.iterator()); } - - @Override - public void close() throws IOException - { - } - - @Override - public void inspectRuntimeShape(RuntimeShapeInspector inspector) - { - inspector.visit("bitmapIndex", bitmapIndex); - } } @Override diff --git a/processing/src/test/java/io/druid/query/aggregation/cardinality/CardinalityAggregatorTest.java b/processing/src/test/java/io/druid/query/aggregation/cardinality/CardinalityAggregatorTest.java index 362f14285a2a..444eb83541cc 100644 --- a/processing/src/test/java/io/druid/query/aggregation/cardinality/CardinalityAggregatorTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/cardinality/CardinalityAggregatorTest.java @@ -48,8 +48,6 @@ import io.druid.segment.DimensionSelectorUtils; import io.druid.segment.IdLookup; import io.druid.segment.data.IndexedInts; -import it.unimi.dsi.fastutil.ints.IntIterator; -import it.unimi.dsi.fastutil.ints.IntIterators; import org.junit.Assert; import org.junit.Test; @@ -142,12 +140,6 @@ public int get(int i) return column.get(p)[i]; } - @Override - public IntIterator iterator() - { - return IntIterators.asIntIterator(Iterators.forArray(column.get(p))); - } - @Override public void close() throws IOException { diff --git a/processing/src/test/java/io/druid/segment/ConstantDimensionSelectorTest.java b/processing/src/test/java/io/druid/segment/ConstantDimensionSelectorTest.java index 344c4938c8f4..abac95cf0159 100644 --- a/processing/src/test/java/io/druid/segment/ConstantDimensionSelectorTest.java +++ b/processing/src/test/java/io/druid/segment/ConstantDimensionSelectorTest.java @@ -25,8 +25,6 @@ import org.junit.Assert; import org.junit.Test; -import java.util.Iterator; - public class ConstantDimensionSelectorTest { private final DimensionSelector NULL_SELECTOR = DimensionSelectorUtils.constantSelector(null); @@ -46,11 +44,6 @@ public void testGetRow() throws Exception IndexedInts row = NULL_SELECTOR.getRow(); Assert.assertEquals(1, row.size()); Assert.assertEquals(0, row.get(0)); - - Iterator iter = row.iterator(); - Assert.assertEquals(true, iter.hasNext()); - Assert.assertEquals(0, iter.next().intValue()); - Assert.assertEquals(false, iter.hasNext()); } @Test diff --git a/processing/src/test/java/io/druid/segment/IndexMergerTestBase.java b/processing/src/test/java/io/druid/segment/IndexMergerTestBase.java index 77627890b444..6b9825a0a476 100644 --- a/processing/src/test/java/io/druid/segment/IndexMergerTestBase.java +++ b/processing/src/test/java/io/druid/segment/IndexMergerTestBase.java @@ -49,10 +49,10 @@ import io.druid.segment.column.DictionaryEncodedColumn; import io.druid.segment.column.SimpleDictionaryEncodedColumn; import io.druid.segment.data.BitmapSerdeFactory; +import io.druid.segment.data.BitmapValues; import io.druid.segment.data.CompressedObjectStrategy; import io.druid.segment.data.CompressionFactory; import io.druid.segment.data.IncrementalIndexTest; -import io.druid.segment.data.IndexedInts; import io.druid.segment.incremental.IncrementalIndex; import io.druid.segment.incremental.IncrementalIndexAdapter; import io.druid.segment.incremental.IncrementalIndexSchema; @@ -1521,7 +1521,7 @@ public void testNoRollupMergeWithDuplicateRow() throws Exception checkBitmapIndex(Lists.newArrayList(3), adapter.getBitmapIndex("d9", "921")); } - private void checkBitmapIndex(ArrayList expected, IndexedInts real) + private void checkBitmapIndex(ArrayList expected, BitmapValues real) { Assert.assertEquals(expected.size(), real.size()); int i = 0; diff --git a/processing/src/test/java/io/druid/segment/QueryableIndexIndexableAdapterTest.java b/processing/src/test/java/io/druid/segment/QueryableIndexIndexableAdapterTest.java index a08cb561cd61..a531269ca8cd 100644 --- a/processing/src/test/java/io/druid/segment/QueryableIndexIndexableAdapterTest.java +++ b/processing/src/test/java/io/druid/segment/QueryableIndexIndexableAdapterTest.java @@ -69,9 +69,9 @@ public void testGetBitmapIndex() throws Exception IndexableAdapter adapter = new QueryableIndexIndexableAdapter(index); String dimension = "dim1"; //null is added to all dimensions with value - IndexedInts indexedInts = adapter.getBitmapIndex(dimension, 0); + IndexedInts indexedInts = adapter.getBitmapValues(dimension, 0); for (int i = 0; i < adapter.getDimValueLookup(dimension).size(); i++) { - indexedInts = adapter.getBitmapIndex(dimension, i); + indexedInts = adapter.getBitmapValues(dimension, i); Assert.assertEquals(1, indexedInts.size()); } } diff --git a/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedSupplierTest.java b/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedSupplierTest.java index dd9554da4857..914247c8497c 100644 --- a/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedSupplierTest.java +++ b/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedSupplierTest.java @@ -115,11 +115,8 @@ public void testIterators() final IndexedInts vSizeIndexedInts = iterator.next(); Assert.assertEquals(ints.length, vSizeIndexedInts.size()); - Iterator valsIterator = vSizeIndexedInts.iterator(); - int j = 0; - while (valsIterator.hasNext()) { - Assert.assertEquals((Integer) ints[j], valsIterator.next()); - j++; + for (int i = 0; i < vSizeIndexedInts.size(); i++) { + Assert.assertEquals(ints[i], vSizeIndexedInts.get(i)); } row++; } diff --git a/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexAdapterTest.java b/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexAdapterTest.java index 2d06530a0a36..0fe5a015a568 100644 --- a/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexAdapterTest.java +++ b/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexAdapterTest.java @@ -55,7 +55,7 @@ public void testGetBitmapIndex() throws Exception ); String dimension = "dim1"; for (int i = 0; i < adapter.getDimValueLookup(dimension).size(); i++) { - IndexedInts indexedInts = adapter.getBitmapIndex(dimension, i); + IndexedInts indexedInts = adapter.getBitmapValues(dimension, i); Assert.assertEquals(1, indexedInts.size()); } } From 19525d4bc2d19b150c7d28f977c2aad97f53857c Mon Sep 17 00:00:00 2001 From: leventov Date: Sat, 16 Sep 2017 21:09:45 -0500 Subject: [PATCH 4/4] Fix tests --- .../druid/segment/QueryableIndexIndexableAdapterTest.java | 8 ++++---- .../segment/incremental/IncrementalIndexAdapterTest.java | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/processing/src/test/java/io/druid/segment/QueryableIndexIndexableAdapterTest.java b/processing/src/test/java/io/druid/segment/QueryableIndexIndexableAdapterTest.java index a531269ca8cd..2f6a1202e036 100644 --- a/processing/src/test/java/io/druid/segment/QueryableIndexIndexableAdapterTest.java +++ b/processing/src/test/java/io/druid/segment/QueryableIndexIndexableAdapterTest.java @@ -19,11 +19,11 @@ package io.druid.segment; +import io.druid.segment.data.BitmapValues; import io.druid.segment.data.CompressedObjectStrategy; import io.druid.segment.data.CompressionFactory; import io.druid.segment.data.ConciseBitmapSerdeFactory; import io.druid.segment.data.IncrementalIndexTest; -import io.druid.segment.data.IndexedInts; import io.druid.segment.incremental.IncrementalIndex; import org.junit.Assert; import org.junit.Rule; @@ -69,10 +69,10 @@ public void testGetBitmapIndex() throws Exception IndexableAdapter adapter = new QueryableIndexIndexableAdapter(index); String dimension = "dim1"; //null is added to all dimensions with value - IndexedInts indexedInts = adapter.getBitmapValues(dimension, 0); + BitmapValues bitmapValues = adapter.getBitmapValues(dimension, 0); for (int i = 0; i < adapter.getDimValueLookup(dimension).size(); i++) { - indexedInts = adapter.getBitmapValues(dimension, i); - Assert.assertEquals(1, indexedInts.size()); + bitmapValues = adapter.getBitmapValues(dimension, i); + Assert.assertEquals(1, bitmapValues.size()); } } } diff --git a/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexAdapterTest.java b/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexAdapterTest.java index 0fe5a015a568..619a5e3e8ae7 100644 --- a/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexAdapterTest.java +++ b/processing/src/test/java/io/druid/segment/incremental/IncrementalIndexAdapterTest.java @@ -22,11 +22,11 @@ import io.druid.segment.IndexSpec; import io.druid.segment.IndexableAdapter; import io.druid.segment.Rowboat; +import io.druid.segment.data.BitmapValues; import io.druid.segment.data.CompressedObjectStrategy; import io.druid.segment.data.CompressionFactory; import io.druid.segment.data.ConciseBitmapSerdeFactory; import io.druid.segment.data.IncrementalIndexTest; -import io.druid.segment.data.IndexedInts; import org.junit.Assert; import org.junit.Test; @@ -55,8 +55,8 @@ public void testGetBitmapIndex() throws Exception ); String dimension = "dim1"; for (int i = 0; i < adapter.getDimValueLookup(dimension).size(); i++) { - IndexedInts indexedInts = adapter.getBitmapValues(dimension, i); - Assert.assertEquals(1, indexedInts.size()); + BitmapValues bitmapValues = adapter.getBitmapValues(dimension, i); + Assert.assertEquals(1, bitmapValues.size()); } }