Add several missing inspectRuntimeShape() calls (#6893)

* Add several missing inspectRuntimeShape() calls

* Add lgK to runtime shapes
This commit is contained in:
Roman Leventov 2019-02-01 11:04:26 +07:00 committed by Jonathan Wei
parent 4e426327bb
commit f7df5fedcc
8 changed files with 61 additions and 24 deletions

View File

@ -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);
}
}
}

View File

@ -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);
}
}

View File

@ -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

View File

@ -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);
}
}

View File

@ -144,18 +144,17 @@ public class FixedBucketsHistogramAggregatorFactory extends AggregatorFactory
combined.combineHistogram(other);
}
@Override
public Class<FixedBucketsHistogram> classOfObject()
{
return FixedBucketsHistogram.class;
}
@Nullable
@Override
public FixedBucketsHistogram getObject()
{
return combined;
}
@Override
public Class<FixedBucketsHistogram> classOfObject()
{
return FixedBucketsHistogram.class;
}
};
}

View File

@ -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.
}
}

View File

@ -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);
}
}

View File

@ -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);
}