Optimize CostBalancerStrategy

[nishantmonu51]

Optimizations to Cost balancer -

1. Avoid creating thread pool on each chooseBestServer call
2. Use custom impl for finding gapMillis between two segments rather than using Joda impl which is slow.

These changes speeds up computing segment costs fast by 2X.

Benchmark results -
Before (with shared thread pool, was getting oome in benchmarks with creating new thread pool each time)-

CostBalancerStrategyBenchmark.testBenchmark[0]: [measured 1000 out of 1010 rounds, threads: 1 (sequential)]
 round: 0.02 [+- 0.01], round.block: 0.00 [+- 0.00], round.gc: 0.00 [+- 0.00], GC.calls: 7, GC.time: 0.01, time.total: 21.60, time.warmup: 0.29, time.bench: 21.31
CostBalancerStrategyBenchmark.testBenchmark[1]: [measured 1000 out of 1010 rounds, threads: 1 (sequential)]
 round: 0.01 [+- 0.00], round.block: 0.00 [+- 0.00], round.gc: 0.00 [+- 0.00], GC.calls: 9, GC.time: 0.01, time.total: 5.93, time.warmup: 0.06, time.bench: 5.87

After -

CostBalancerStrategyBenchmark.testBenchmark[0]: [measured 1000 out of 1010 rounds, threads: 1 (sequential)]
 round: 0.01 [+- 0.00], round.block: 0.00 [+- 0.00], round.gc: 0.00 [+- 0.00], GC.calls: 7, GC.time: 0.01, time.total: 8.32, time.warmup: 0.17, time.bench: 8.14
CostBalancerStrategyBenchmark.testBenchmark[1]: [measured 1000 out of 1010 rounds, threads: 1 (sequential)]
 round: 0.00 [+- 0.00], round.block: 0.00 [+- 0.00], round.gc: 0.00 [+- 0.00], GC.calls: 9, GC.time: 0.01, time.total: 3.64, time.warmup: 0.04, time.bench: 3.60

Joda gap vs custom timeMillis (10X show improvement)-

CostBalancerStrategyBenchmark.testJodaGap[0]: [measured 1000000 out of 1001000 rounds, threads: 1 (sequential)]
 round: 0.00 [+- 0.00], round.block: 0.00 [+- 0.00], round.gc: 0.00 [+- 0.00], GC.calls: 118, GC.time: 0.32, time.total: 31.99, time.warmup: 0.04, time.bench: 31.95
CostBalancerStrategyBenchmark.testBalancerGapMillis[0]: [measured 1000000 out of 1001000 rounds, threads: 1 (sequential)]
 round: 0.00 [+- 0.00], round.block: 0.00 [+- 0.00], round.gc: 0.00 [+- 0.00], GC.calls: 0, GC.time: 0.00, time.total: 3.32, time.warmup: 0.01, time.bench: 3.32

[xvrl]

instead of making strategies closeable, how about we inject the executor service and attach it to the lifecycle? I think it would make things cleaner.

[nishantmonu51]

I initially started with that approach, but the number of balancer threads is tied to coordinator dynamic config, so i tied it to the coordinator run loop instead of lifecycle.

[fjy]

👍

[xvrl]

@nishantmonu51 and I talked offline, he'll try a better approach that doesn't require making the balancing strategy closeable, while keeping the number of threads a dynamic config.

[nishantmonu51]

@xvrl made the changes, please have a look again.

[xvrl]

Why not directly make this one a ListeningExecutorService?

[nishantmonu51]

done.

[xvrl]

you might want to use try with resources with the factory, just in case an exception is thrown earlier. This will also close the factory automatically.

[nishantmonu51]

done.

[xvrl]

minor comment #2910 (comment) but 👍 otherwise