GroupBy: When possible, sort results on read rather than continuously during insert.

[gianm]

Leans on the logic from #2571 with respect to deciding when to sort and when not to sort.

This does involve somewhat more memory use (need to copy the facts to a list and then sort the list). But as the list is just references to already-existing Map.Entry objects, the overhead shouldn't be too bad.

Before:

Result "mergeQueryableIndex":
  7899182.389 ±(99.9%) 490250.490 us/op [Average]
  (min, avg, max) = (7346007.865, 7899182.389, 9206588.959), stdev = 564573.193
  CI (99.9%): [7408931.899, 8389432.879] (assumes normal distribution)


# Run complete. Total time: 00:05:35

Benchmark                             Mode  Cnt        Score        Error  Units
GroupByBenchmark.mergeQueryableIndex  avgt   20  7899182.389 ± 490250.490  us/op

After:

Result "mergeQueryableIndex":
  5952538.010 ±(99.9%) 365008.858 us/op [Average]
  (min, avg, max) = (5645155.827, 5952538.010, 7256515.626), stdev = 420344.744
  CI (99.9%): [5587529.153, 6317546.868] (assumes normal distribution)


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

Benchmark                             Mode  Cnt        Score        Error  Units
GroupByBenchmark.mergeQueryableIndex  avgt   20  5952538.010 ± 365008.858  us/op

Depends on #2870

[himanshug]

this may be ok for on-heap index but will unexpectedly put everything on-heap even for any off-heap implementation of IncrementalIndex (for example #2847 )
one option would be to update QueryToolChest to not exercise this path when off-heap merging was requested.

[gianm]

I was thinking that for the current offheap impl this is fine (even the offheap one still has an onheap facts map) but things are different if the facts map is offheap.

I am ok waiting on this PR until #2847 is settled and then figuring out what is best.

[gianm]

closing for now.