AlexanderSaydakov commented on a change in pull request #8194: HllSketch
Merge/Build BufferAggregators: Speed up init with prebuilt sketch.
URL: https://github.com/apache/incubator-druid/pull/8194#discussion_r308900477
##########
File path:
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchBuildBufferAggregator.java
##########
@@ -53,6 +55,14 @@
private final IdentityHashMap<ByteBuffer, Int2ObjectMap<HllSketch>>
sketchCache = new IdentityHashMap<>();
private final Striped<ReadWriteLock> stripedLock =
Striped.readWriteLock(NUM_STRIPES);
+ /**
+ * Used by {@link #init(ByteBuffer, int)}. We initialize by copying a
prebuilt empty sketch object.
+ * {@link HllSketchMergeBufferAggregator} does something similar, but we
don't share code to create emptySketch
+ * because it is not exactly the same (the "build" flavor initializes based
on tgtHllType, but the "merge" flavor
+ * always initializes using HLL_8).
+ */
Review comment:
This is not so. HllSketchMergeBufferAggregator initializes HllUnion objects
as opposed to HllSketch objects here. And HllUnion constructor does not even
have the target type parameter (it is always in HLL_8 mode, but it is an
implementation detail). The target type is used when the final result is
obtained from the union in get().
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
