You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by jo...@apache.org on 2019/02/01 04:04:31 UTC
[incubator-druid] branch master updated: Add several missing
inspectRuntimeShape() calls (#6893)
This is an automated email from the ASF dual-hosted git repository.
jonwei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-druid.git
The following commit(s) were added to refs/heads/master by this push:
new f7df5fe Add several missing inspectRuntimeShape() calls (#6893)
f7df5fe is described below
commit f7df5fedcc452a8ba4d8e26a560a1ae52a0e85a5
Author: Roman Leventov <le...@gmail.com>
AuthorDate: Fri Feb 1 11:04:26 2019 +0700
Add several missing inspectRuntimeShape() calls (#6893)
* Add several missing inspectRuntimeShape() calls
* Add lgK to runtime shapes
---
.../druid/benchmark/ExpressionAggregationBenchmark.java | 9 +++++++++
.../hll/HllSketchBuildBufferAggregator.java | 16 +++++++++++++---
.../datasketches/hll/HllSketchMergeAggregator.java | 4 ++--
.../hll/HllSketchMergeBufferAggregator.java | 17 ++++++++++++++---
.../FixedBucketsHistogramAggregatorFactory.java | 9 ++++-----
.../druid/query/aggregation/AggregateCombiner.java | 5 +++--
.../query/aggregation/NullableBufferAggregator.java | 17 ++++++++++++-----
.../monomorphicprocessing/RuntimeShapeInspector.java | 4 ++--
8 files changed, 59 insertions(+), 22 deletions(-)
diff --git a/benchmarks/src/main/java/org/apache/druid/benchmark/ExpressionAggregationBenchmark.java b/benchmarks/src/main/java/org/apache/druid/benchmark/ExpressionAggregationBenchmark.java
index ed56d74..69b8f80 100644
--- a/benchmarks/src/main/java/org/apache/druid/benchmark/ExpressionAggregationBenchmark.java
+++ b/benchmarks/src/main/java/org/apache/druid/benchmark/ExpressionAggregationBenchmark.java
@@ -32,6 +32,7 @@ import org.apache.druid.query.aggregation.BufferAggregator;
import org.apache.druid.query.aggregation.DoubleSumAggregatorFactory;
import org.apache.druid.query.aggregation.JavaScriptAggregatorFactory;
import org.apache.druid.query.expression.TestExprMacroTable;
+import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector;
import org.apache.druid.segment.BaseFloatColumnValueSelector;
import org.apache.druid.segment.ColumnSelectorFactory;
import org.apache.druid.segment.Cursor;
@@ -240,10 +241,18 @@ public class ExpressionAggregationBenchmark
{
throw new UnsupportedOperationException();
}
+
@Override
public void close()
{
+ // nothing to close
+ }
+ @Override
+ public void inspectRuntimeShape(RuntimeShapeInspector inspector)
+ {
+ inspector.visit("xSelector", xSelector);
+ inspector.visit("ySelector", ySelector);
}
}
}
diff --git a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchBuildBufferAggregator.java b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchBuildBufferAggregator.java
index a170502..0ec525e 100644
--- a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchBuildBufferAggregator.java
+++ b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchBuildBufferAggregator.java
@@ -26,6 +26,7 @@ import com.yahoo.sketches.hll.TgtHllType;
import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
import org.apache.druid.query.aggregation.BufferAggregator;
+import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector;
import org.apache.druid.segment.ColumnValueSelector;
import java.nio.ByteBuffer;
@@ -42,7 +43,7 @@ import java.util.concurrent.locks.ReadWriteLock;
public class HllSketchBuildBufferAggregator implements BufferAggregator
{
- // for locking per buffer position (power of 2 to make index computation faster)
+ /** for locking per buffer position (power of 2 to make index computation faster) */
private static final int NUM_STRIPES = 64;
private final ColumnValueSelector<Object> selector;
@@ -73,7 +74,7 @@ public class HllSketchBuildBufferAggregator implements BufferAggregator
putSketchIntoCache(buf, position, new HllSketch(lgK, tgtHllType, mem));
}
- /*
+ /**
* This method uses locks because it can be used during indexing,
* and Druid can call aggregate() and get() concurrently
* See https://github.com/druid-io/druid/pull/3956
@@ -96,7 +97,7 @@ public class HllSketchBuildBufferAggregator implements BufferAggregator
}
}
- /*
+ /**
* This method uses locks because it can be used during indexing,
* and Druid can call aggregate() and get() concurrently
* See https://github.com/druid-io/druid/pull/3956
@@ -181,4 +182,13 @@ public class HllSketchBuildBufferAggregator implements BufferAggregator
return hashCode ^ (hashCode >>> 7) ^ (hashCode >>> 4);
}
+ @Override
+ public void inspectRuntimeShape(RuntimeShapeInspector inspector)
+ {
+ inspector.visit("selector", selector);
+ // lgK should be inspected because different execution paths exist in HllSketch.update() that is called from
+ // @CalledFromHotLoop-annotated aggregate() depending on the lgK.
+ // See https://github.com/apache/incubator-druid/pull/6893#discussion_r250726028
+ inspector.visit("lgK", lgK);
+ }
}
diff --git a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchMergeAggregator.java b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchMergeAggregator.java
index 44f1157..14538f3 100644
--- a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchMergeAggregator.java
+++ b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchMergeAggregator.java
@@ -48,7 +48,7 @@ public class HllSketchMergeAggregator implements Aggregator
this.union = new Union(lgK);
}
- /*
+ /**
* This method is synchronized because it can be used during indexing,
* and Druid can call aggregate() and get() concurrently.
* See https://github.com/druid-io/druid/pull/3956
@@ -65,7 +65,7 @@ public class HllSketchMergeAggregator implements Aggregator
}
}
- /*
+ /**
* This method is synchronized because it can be used during indexing,
* and Druid can call aggregate() and get() concurrently.
* See https://github.com/druid-io/druid/pull/3956
diff --git a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchMergeBufferAggregator.java b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchMergeBufferAggregator.java
index 65b273b..eec9ef2 100644
--- a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchMergeBufferAggregator.java
+++ b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchMergeBufferAggregator.java
@@ -25,6 +25,7 @@ import com.yahoo.sketches.hll.HllSketch;
import com.yahoo.sketches.hll.TgtHllType;
import com.yahoo.sketches.hll.Union;
import org.apache.druid.query.aggregation.BufferAggregator;
+import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector;
import org.apache.druid.segment.ColumnValueSelector;
import java.nio.ByteBuffer;
@@ -40,7 +41,7 @@ import java.util.concurrent.locks.ReadWriteLock;
public class HllSketchMergeBufferAggregator implements BufferAggregator
{
- // for locking per buffer position (power of 2 to make index computation faster)
+ /** for locking per buffer position (power of 2 to make index computation faster) */
private static final int NUM_STRIPES = 64;
private final ColumnValueSelector<HllSketch> selector;
@@ -73,7 +74,7 @@ public class HllSketchMergeBufferAggregator implements BufferAggregator
new Union(lgK, mem);
}
- /*
+ /**
* This method uses locks because it can be used during indexing,
* and Druid can call aggregate() and get() concurrently
* See https://github.com/druid-io/druid/pull/3956
@@ -97,7 +98,7 @@ public class HllSketchMergeBufferAggregator implements BufferAggregator
}
}
- /*
+ /**
* This method uses locks because it can be used during indexing,
* and Druid can call aggregate() and get() concurrently
* See https://github.com/druid-io/druid/pull/3956
@@ -120,6 +121,7 @@ public class HllSketchMergeBufferAggregator implements BufferAggregator
@Override
public void close()
{
+ // nothing to close
}
@Override
@@ -134,4 +136,13 @@ public class HllSketchMergeBufferAggregator implements BufferAggregator
throw new UnsupportedOperationException("Not implemented");
}
+ @Override
+ public void inspectRuntimeShape(RuntimeShapeInspector inspector)
+ {
+ inspector.visit("selector", selector);
+ // lgK should be inspected because different execution paths exist in Union.update() that is called from
+ // @CalledFromHotLoop-annotated aggregate() depending on the lgK.
+ // See https://github.com/apache/incubator-druid/pull/6893#discussion_r250726028
+ inspector.visit("lgK", lgK);
+ }
}
diff --git a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramAggregatorFactory.java b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramAggregatorFactory.java
index de30747..7ddcc9f 100644
--- a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramAggregatorFactory.java
+++ b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramAggregatorFactory.java
@@ -145,16 +145,15 @@ public class FixedBucketsHistogramAggregatorFactory extends AggregatorFactory
}
@Override
- public Class<FixedBucketsHistogram> classOfObject()
+ public FixedBucketsHistogram getObject()
{
- return FixedBucketsHistogram.class;
+ return combined;
}
- @Nullable
@Override
- public FixedBucketsHistogram getObject()
+ public Class<FixedBucketsHistogram> classOfObject()
{
- return combined;
+ return FixedBucketsHistogram.class;
}
};
}
diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/AggregateCombiner.java b/processing/src/main/java/org/apache/druid/query/aggregation/AggregateCombiner.java
index 0a6c542..d29ef3e 100644
--- a/processing/src/main/java/org/apache/druid/query/aggregation/AggregateCombiner.java
+++ b/processing/src/main/java/org/apache/druid/query/aggregation/AggregateCombiner.java
@@ -51,7 +51,6 @@ public interface AggregateCombiner<T> extends ColumnValueSelector<T>
* org.apache.druid.segment.ObjectColumnSelector#getObject()} must not be modified, and must not become a subject for
* modification during subsequent {@link #fold} calls.
*/
- @SuppressWarnings("unused") // Going to be used when https://github.com/apache/incubator-druid/projects/2 is complete
void reset(ColumnValueSelector selector);
/**
@@ -80,6 +79,8 @@ public interface AggregateCombiner<T> extends ColumnValueSelector<T>
@Override
default void inspectRuntimeShape(RuntimeShapeInspector inspector)
{
- // Usually AggregateCombiner has nothing to inspect
+ // Usually AggregateCombiners have nothing to inspect, because their getLong/getDouble/getFloat (the methods
+ // annotated @CalledFromHotLoop in AggregateCombiner) is a plain getter of a field, so there is no source for
+ // branching and otherwise non-monomorphic runtime profile.
}
}
diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/NullableBufferAggregator.java b/processing/src/main/java/org/apache/druid/query/aggregation/NullableBufferAggregator.java
index 48de4df..e176a32 100644
--- a/processing/src/main/java/org/apache/druid/query/aggregation/NullableBufferAggregator.java
+++ b/processing/src/main/java/org/apache/druid/query/aggregation/NullableBufferAggregator.java
@@ -21,6 +21,7 @@ package org.apache.druid.query.aggregation;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.guice.annotations.PublicApi;
+import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector;
import org.apache.druid.segment.BaseNullableColumnValueSelector;
import javax.annotation.Nullable;
@@ -39,14 +40,13 @@ import java.nio.ByteBuffer;
@PublicApi
public final class NullableBufferAggregator implements BufferAggregator
{
-
private final BufferAggregator delegate;
- private final BaseNullableColumnValueSelector selector;
+ private final BaseNullableColumnValueSelector nullSelector;
- public NullableBufferAggregator(BufferAggregator delegate, BaseNullableColumnValueSelector selector)
+ public NullableBufferAggregator(BufferAggregator delegate, BaseNullableColumnValueSelector nullSelector)
{
this.delegate = delegate;
- this.selector = selector;
+ this.nullSelector = nullSelector;
}
@Override
@@ -59,7 +59,7 @@ public final class NullableBufferAggregator implements BufferAggregator
@Override
public void aggregate(ByteBuffer buf, int position)
{
- boolean isNotNull = !selector.isNull();
+ boolean isNotNull = !nullSelector.isNull();
if (isNotNull) {
if (buf.get(position) == NullHandling.IS_NULL_BYTE) {
buf.put(position, NullHandling.IS_NOT_NULL_BYTE);
@@ -116,4 +116,11 @@ public final class NullableBufferAggregator implements BufferAggregator
{
delegate.close();
}
+
+ @Override
+ public void inspectRuntimeShape(RuntimeShapeInspector inspector)
+ {
+ inspector.visit("delegate", delegate);
+ inspector.visit("nullSelector", nullSelector);
+ }
}
diff --git a/processing/src/main/java/org/apache/druid/query/monomorphicprocessing/RuntimeShapeInspector.java b/processing/src/main/java/org/apache/druid/query/monomorphicprocessing/RuntimeShapeInspector.java
index 5dffef2..0e27f30 100644
--- a/processing/src/main/java/org/apache/druid/query/monomorphicprocessing/RuntimeShapeInspector.java
+++ b/processing/src/main/java/org/apache/druid/query/monomorphicprocessing/RuntimeShapeInspector.java
@@ -37,8 +37,8 @@ public interface RuntimeShapeInspector
/**
* To be called from {@link HotLoopCallee#inspectRuntimeShape(RuntimeShapeInspector)} with something, that is
* important to ensure monomorphism and predictable branch taking in hot loops, but doesn't apply to other visit()
- * methods in RuntimeShapeInspector. For example, {@link org.apache.druid.segment.BitmapOffset#inspectRuntimeShape} reports
- * bitmap population via this method, to ensure predictable branch taking inside Bitmap's iterators.
+ * methods in RuntimeShapeInspector. For example, {@link org.apache.druid.segment.BitmapOffset#inspectRuntimeShape}
+ * reports bitmap population via this method, to ensure predictable branch taking inside Bitmap's iterators.
*/
void visit(String key, String runtimeShape);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org