Akshat-Jain opened a new issue, #17693:
URL: https://github.com/apache/druid/issues/17693

   ### Description
   
   Currently, we have some benchmarks which are present outside of 
`druid-benchmarks` module:
   - `TopNQueryRunnerBenchmark.java`
   - `ApproximateHistogramErrorBenchmark.java`
   - `CardinalityAggregatorBenchmark.java`
   - `JavaScriptAggregatorBenchmark.java`
   - `TopNBinaryFnBenchmark.java`
   - `MemcachedCacheBenchmark.java`
   - `HyperLogLogCollectorBenchmark.java`
   - `BitmapCreationBenchmark.java`
   - `CostBalancerStrategyBenchmark.java`
   
   These should be moved to the `druid-benchmarks` module.
   
   I spent some time in attempting to move them, the diff for a couple of which 
can be seen 
[here](https://github.com/Akshat-Jain/druid/compare/master...Akshat-Jain:druid:akshat/move-benchmarks-to-benchmark-module?expand=1).
 But it's not trivial to move them, so I'm sharing my findings and the diff as 
a reference point, in case anyone would like to take it forward.
   
   - `TopNQueryRunnerBenchmark.java`: This has been moved to the 
`druid-benchmarks` module in the diff, but it doesn't work because of 
`StupidPool` leaks. But the same problem exists in the existing 
`TopNQueryRunnerBenchmark.java`.
   - `ApproximateHistogramErrorBenchmark.java`: This has been moved to the 
`druid-benchmarks` module in the diff. But the existing 
`ApproximateHistogramErrorBenchmark.java` was printing/reporting the number of 
errors, which doesn't seem possible to do with JMH.
     - To add a custom column in JMH benchmark results, I tried using 
`AuxCounters`, but JMH doesn't output the value in AuxCounters class without 
aggregating them over the run of the benchmark (across all iterations). So this 
doesn't work.
     - `AuxCounters` doesn't support `SingleShot` mode, which is what has been 
done in the diff for now. Instead of adding it in the benchmark results, we are 
using `SingleShot` mode, and printing the number of errors, since JMH can't 
report it as an output metric. So the benchmark results themselves don't really 
add much of a value IMO.
   - The following benchmarks error out with `java.lang.NoSuchMethodError: 
'void com.google.common.io.Closeables.closeQuietly(java.io.Closeable)'`:
     - `CardinalityAggregatorBenchmark.java`
     - `JavaScriptAggregatorBenchmark.java`
     - `TopNBinaryFnBenchmark.java`
     - `MemcachedCacheBenchmark.java`
     - `HyperLogLogCollectorBenchmark.java`
   - `BitmapCreationBenchmark.java`: It is ignored with `@Ignore`.
   - `CostBalancerStrategyBenchmark.java`: We have 2 files of the same name
     - First one is ignored with `@Ignore`.
     - Second one is already in `druid-benchmarks` module. Not sure if we can 
remove the first one directly, it is marked with `@Ignore` anyway.
   
   For each of the above, we should ideally try to analyze the value they add 
(since most of them are non-functional currently - they either error out or are 
marked with `@Ignore`). If they add value, then they should be moved to the 
`druid-benchmarks` module.


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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to