gianm 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_r308939367
##########
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:
Thanks for clarifying. I just read the javadocs for Union and HllSketch and
the situation is more clear. I adjusted the comment to the following,
```java
/**
* Used by {@link #init(ByteBuffer, int)}. We initialize by copying a
prebuilt empty Union image.
* {@link HllSketchBuildBufferAggregator} does something similar, but
different enough that we don't share code. The
* "build" flavor uses {@link HllSketch} objects and the "merge" flavor
uses {@link Union} objects.
*/
```
----------------------------------------------------------------
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]
