Apache
/
druid
mirror of https://github.com/apache/druid.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Don't use HistoricalXxxPrototypes in PooledTopNAlgorithm when Cursor's offset is FilteredOffset (#5133) 

* Don't use HistoricalXxxPrototypes in PooledTopNAlgorithm when Cursor's offset is FilteredOffset, because it doens't support cloning

* Add test
This commit is contained in:
Roman Leventov 2017-12-01 22:04:22 -03:00 committed by Gian Merlino
parent aafd0373be
commit aacc57131b
8 changed files with 127 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
processing/src/main/java/io/druid/query/topn/Generic1AggPooledTopNScannerPrototype.java
View File

@ -28,6 +28,18 @@ import java.nio.ByteBuffer;
public final class Generic1AggPooledTopNScannerPrototype implements Generic1AggPooledTopNScanner
{
/**
* Any changes to this method should be coordinated with {@link TopNUtils}, {@link
* PooledTopNAlgorithm#computeSpecializedScanAndAggregateImplementations} and downstream methods.
*
* It should be checked with a tool like https://github.com/AdoptOpenJDK/jitwatch that C2 compiler output for this
* method doesn't have any method calls in the while loop, i. e. all method calls are inlined. To be able to see
* assembly of this method in JITWatch and other similar tools, {@link
* PooledTopNAlgorithm#specializeGeneric1AggPooledTopN} should be turned off. Note that in this case the benchmark
* should be "naturally monomorphic", i. e. execute this method always with the same runtime shape.
*
* If the while loop contains not inlined method calls, it should be considered as a performance bug.
*/
@Override
public long scanAndAggregate(
DimensionSelector dimensionSelector,

12
processing/src/main/java/io/druid/query/topn/Generic2AggPooledTopNScannerPrototype.java
View File

@ -28,6 +28,18 @@ import java.nio.ByteBuffer;
public final class Generic2AggPooledTopNScannerPrototype implements Generic2AggPooledTopNScanner
{
/**
* Any changes to this method should be coordinated with {@link TopNUtils}, {@link
* PooledTopNAlgorithm#computeSpecializedScanAndAggregateImplementations} and downstream methods.
*
* It should be checked with a tool like https://github.com/AdoptOpenJDK/jitwatch that C2 compiler output for this
* method doesn't have any method calls in the while loop, i. e. all method calls are inlined. To be able to see
* assembly of this method in JITWatch and other similar tools, {@link
* PooledTopNAlgorithm#specializeGeneric2AggPooledTopN} should be turned off. Note that in this case the benchmark
* should be "naturally monomorphic", i. e. execute this method always with the same runtime shape.
*
* If the while loop contains not inlined method calls, it should be considered as a performance bug.
*/
@Override
public long scanAndAggregate(
DimensionSelector dimensionSelector,

12
processing/src/main/java/io/druid/query/topn/Historical1SimpleDoubleAggPooledTopNScannerPrototype.java
View File

@ -35,6 +35,18 @@ public class Historical1SimpleDoubleAggPooledTopNScannerPrototype
SimpleDoubleBufferAggregator
>
{
/**
* Any changes to this method should be coordinated with {@link TopNUtils}, {@link
* PooledTopNAlgorithm#computeSpecializedScanAndAggregateImplementations} and downstream methods.
*
* It should be checked with a tool like https://github.com/AdoptOpenJDK/jitwatch that C2 compiler output for this
* method doesn't have any method calls in the while loop, i. e. all method calls are inlined. To be able to see
* assembly of this method in JITWatch and other similar tools, {@link
* PooledTopNAlgorithm#specializeHistorical1SimpleDoubleAggPooledTopN} should be turned off. Note that in this case
* the benchmark should be "naturally monomorphic", i. e. execute this method always with the same runtime shape.
*
* If the while loop contains not inlined method calls, it should be considered as a performance bug.
*/
@Override
public long scanAndAggregate(
HistoricalDimensionSelector dimensionSelector,

13
processing/src/main/java/io/druid/query/topn/HistoricalSingleValueDimSelector1SimpleDoubleAggPooledTopNScannerPrototype.java
View File

@ -34,6 +34,19 @@ public class HistoricalSingleValueDimSelector1SimpleDoubleAggPooledTopNScannerPr
SimpleDoubleBufferAggregator
>
{
/**
* Any changes to this method should be coordinated with {@link TopNUtils}, {@link
* PooledTopNAlgorithm#computeSpecializedScanAndAggregateImplementations} and downstream methods.
*
* It should be checked with a tool like https://github.com/AdoptOpenJDK/jitwatch that C2 compiler output for this
* method doesn't have any method calls in the while loop, i. e. all method calls are inlined. To be able to see
* assembly of this method in JITWatch and other similar tools, {@link
* PooledTopNAlgorithm#specializeHistoricalSingleValueDimSelector1SimpleDoubleAggPooledTopN} should be turned off.
* Note that in this case the benchmark should be "naturally monomorphic", i. e. execute this method always with the
* same runtime shape.
*
* If the while loop contains not inlined method calls, it should be considered as a performance bug.
*/
@Override
public long scanAndAggregate(
SingleValueHistoricalDimensionSelector dimensionSelector,

53
processing/src/main/java/io/druid/query/topn/PooledTopNAlgorithm.java
View File

@ -36,6 +36,7 @@ import io.druid.query.monomorphicprocessing.StringRuntimeShape;
import io.druid.segment.Capabilities;
import io.druid.segment.Cursor;
import io.druid.segment.DimensionSelector;
import io.druid.segment.FilteredOffset;
import io.druid.segment.column.ValueType;
import io.druid.segment.data.IndexedInts;
import io.druid.segment.data.Offset;
@ -128,17 +129,21 @@ public class PooledTopNAlgorithm
if (theAggregators.length == 1) {
BufferAggregator aggregator = theAggregators[0];
final Cursor cursor = params.getCursor();
if (cursor instanceof HistoricalCursor && aggregator instanceof SimpleDoubleBufferAggregator) {
if (params.getDimSelector() instanceof SingleValueHistoricalDimensionSelector &&
((SimpleDoubleBufferAggregator) aggregator).getSelector() instanceof HistoricalColumnSelector) {
return scanAndAggregateHistorical1SimpleDoubleAgg(
params,
positions,
(SimpleDoubleBufferAggregator) aggregator,
(HistoricalCursor) cursor,
defaultHistoricalSingleValueDimSelector1SimpleDoubleAggScanner
);
}
if (cursor instanceof HistoricalCursor &&
// FilteredOffset.clone() is not supported. This condition should be removed if
// HistoricalSingleValueDimSelector1SimpleDoubleAggPooledTopNScannerPrototype
// doesn't clone offset anymore.
!(((HistoricalCursor) cursor).getOffset() instanceof FilteredOffset) &&
aggregator instanceof SimpleDoubleBufferAggregator &&
params.getDimSelector() instanceof SingleValueHistoricalDimensionSelector &&
((SimpleDoubleBufferAggregator) aggregator).getSelector() instanceof HistoricalColumnSelector) {
return scanAndAggregateHistorical1SimpleDoubleAgg(
params,
positions,
(SimpleDoubleBufferAggregator) aggregator,
(HistoricalCursor) cursor,
defaultHistoricalSingleValueDimSelector1SimpleDoubleAggScanner
);
}
}
return -1;
@ -149,17 +154,21 @@ public class PooledTopNAlgorithm
if (theAggregators.length == 1) {
BufferAggregator aggregator = theAggregators[0];
final Cursor cursor = params.getCursor();
if (cursor instanceof HistoricalCursor && aggregator instanceof SimpleDoubleBufferAggregator) {
if (params.getDimSelector() instanceof HistoricalDimensionSelector &&
((SimpleDoubleBufferAggregator) aggregator).getSelector() instanceof HistoricalColumnSelector) {
return scanAndAggregateHistorical1SimpleDoubleAgg(
params,
positions,
(SimpleDoubleBufferAggregator) aggregator,
(HistoricalCursor) cursor,
defaultHistorical1SimpleDoubleAggScanner
);
}
if (cursor instanceof HistoricalCursor &&
// FilteredOffset.clone() is not supported. This condition should be removed if
// Historical1SimpleDoubleAggPooledTopNScannerPrototype
// doesn't clone offset anymore.
!(((HistoricalCursor) cursor).getOffset() instanceof FilteredOffset) &&
aggregator instanceof SimpleDoubleBufferAggregator &&
params.getDimSelector() instanceof HistoricalDimensionSelector &&
((SimpleDoubleBufferAggregator) aggregator).getSelector() instanceof HistoricalColumnSelector) {
return scanAndAggregateHistorical1SimpleDoubleAgg(
params,
positions,
(SimpleDoubleBufferAggregator) aggregator,
(HistoricalCursor) cursor,
defaultHistorical1SimpleDoubleAggScanner
);
}
}
return -1;

4
processing/src/main/java/io/druid/query/topn/TopNUtils.java
View File

@ -35,6 +35,10 @@ final class TopNUtils
* Casting to the specific Offset subtype helps Hotspot JIT (OpenJDK 8) to generate better assembly. It shouldn't be
* so, because the Offset subtype is still always the same (otherwise cast wouldn't be possible), so JIT should
* generate equivalent code. In OpenJDK 9 Hotspot could be improved and this "casting hack" is not needed anymore.
*
* TODO check if offset.clone() is also necessary for generating better assembly, or it could be removed.
*
* See also javadoc comments to methods, where this method is used.
*/
static Object copyOffset(HistoricalCursor cursor)
{

12
processing/src/main/java/io/druid/segment/FilteredOffset.java
View File

@ -110,10 +110,20 @@ public final class FilteredOffset extends Offset
return baseOffset.getBaseReadableOffset();
}
/**
* clone() is not supported by FilteredOffset because it's not possible to clone {@link #filterMatcher}, and
* while re-creating filterMatcher could be not very cheap for some implementations of {@link Filter}. Although this
* approach could be investigated.
*
* If clone is made possible for FilteredOffset, some improvements could become possible in {@link
* io.druid.query.topn.PooledTopNAlgorithm#computeSpecializedScanAndAggregateImplementations}.
*
* See also https://github.com/druid-io/druid/issues/5132.
*/
@Override
public Offset clone()
{
throw new UnsupportedOperationException("FilteredOffset should not be cloned");
throw new UnsupportedOperationException("FilteredOffset could not be cloned");
}
@Override

32
processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java
View File

@ -5605,4 +5605,36 @@ public class TopNQueryRunnerTest
);
assertExpectedResults(expectedResults, query);
}
/**
* Regression test for https://github.com/druid-io/druid/issues/5132
*/
@Test
public void testTopNWithNonBitmapFilter()
{
TopNQuery query = new TopNQueryBuilder()
.dataSource(QueryRunnerTestHelper.dataSource)
.granularity(QueryRunnerTestHelper.allGran)
.filters(new BoundDimFilter(
Column.TIME_COLUMN_NAME,
"0",
String.valueOf(Long.MAX_VALUE),
true,
true,
false,
null,
StringComparators.NUMERIC
))
.dimension(QueryRunnerTestHelper.marketDimension)
.metric("count")
.threshold(4)
.intervals(QueryRunnerTestHelper.firstToThird)
.aggregators(
Collections.singletonList(new DoubleSumAggregatorFactory("count", "qualityDouble"))
)
.build();
// Don't check results, just the fact that the query could complete
Assert.assertNotNull(Sequences.toList(runWithMerge(query), new ArrayList<>()));
}
}