Performance improvement

[cchacin]

JMH for the old code => 802.984 ns/op

Result "actual":
  21734.998 ±(99.9%) 802.984 ns/op [Average]
  (min, avg, max) = (19540.622, 21734.998, 25702.090), stdev = 1622.067
  CI (99.9%): [20932.014, 22537.981] (assumes normal distribution)

# Run complete. Total time: 00:06:25

Benchmark                  (inputSize)  Mode  Cnt      Score     Error  Units
AllAsListBenchmark.actual            4  avgt   50     93.465 ±   1.880  ns/op
AllAsListBenchmark.actual           16  avgt   50    320.043 ±   6.123  ns/op
AllAsListBenchmark.actual           64  avgt   50   1213.432 ±  31.704  ns/op
AllAsListBenchmark.actual          256  avgt   50   5118.282 ± 177.067  ns/op
AllAsListBenchmark.actual         1024  avgt   50  21734.998 ± 802.984  ns/op

JMH for the new code => 381.775 ns/op

Result "actual":
  19520.580 ±(99.9%) 381.775 ns/op [Average]
  (min, avg, max) = (18472.210, 19520.580, 21327.029), stdev = 771.205
  CI (99.9%): [19138.804, 19902.355] (assumes normal distribution)

# Run complete. Total time: 00:06:25

Benchmark                  (inputSize)  Mode  Cnt      Score     Error  Units
AllAsListBenchmark.actual            4  avgt   50     95.670 ±   2.884  ns/op
AllAsListBenchmark.actual           16  avgt   50    327.474 ±   7.080  ns/op
AllAsListBenchmark.actual           64  avgt   50   1262.304 ±  22.849  ns/op
AllAsListBenchmark.actual          256  avgt   50   5040.248 ±  97.250  ns/op
AllAsListBenchmark.actual         1024  avgt   50  19520.580 ± 381.775  ns/op

[codecov-io]

Current coverage is 100% (diff: 100%)

Merging #49 into master will not change coverage

@@           master   #49   diff @@
===================================
  Files           4     4          
  Lines          64    66     +2   
  Methods         0     0          
  Messages        0     0          
  Branches        3     3          
===================================
+ Hits           64    66     +2   
  Misses          0     0          
  Partials        0     0          

Powered by Codecov. Last update ef8b88f...791afdf

[pettermahlen]

I think you misread the JMH output: 21734.998 ±(99.9%) 802.984 ns/op vs 19520.580 ±(99.9%) 381.775 ns/op - the confidence interval is smaller/halved in that measurement, but the performance improvement is much smaller (21.7k to 19.5k). I'm in fact a little surprised that you get a performance improvement at all, given that the . size() calls might be inlineable. But I suppose it's possible for the size of the list to change outside of this method, so it's probably a good change.

I would be interested in seeing the JMH test you were using, as well as if there are any benefits from the changes you made to the .thenApply lambda (as in, comparing original/optimise array creation/optimise list creation/optimise both). That array size should be an inlineable constant, so it would be a bit surprising to me if that change has any impact.

[dflemstr]

We are running the benchmark with java.util.Collections$CopiesList which has some quirks since it is a virtual list of the same element (final int n; final E element;). Thus in the benchmark we are mostly comparing the differences due to the array allocations and it's hard to reason about the list being inlined.

I think the benchmark should be updated to use ArrayList with distinct instances of CompletableFutures, and additionally those futures should be reset occasionally so that the benchmark covers not-yet-completed futures.

Of course this is a micro benchmark so the differences in performance are minuscule. The point of the benchmark was to prove that the old implementation (using streams) was extremely slow in comparison.