clone smallest bitmap, adjust naive/workshy and thresholds in FastAggregation.and

[richardstartin]

This is based on observations made in Apache Pinot and separately in #608

• for small numbers of bitmaps, naive_and can be faster than the more complex workShyAnd method
• putting the smallest bitmap first in the aggregation can accelerate intersections

[richardstartin]

I don't have time to evaluate this exhaustively, this has been determined to be effective already by Apache Pinot, but ran a quick benchmark on my laptop:

before:

Benchmark                         (constructionStrategy)  (count)  (probability)  (seed)   (size)   Mode  Cnt      Score     Error  Units
FastAggregationRLEStressTest.and         CONSTANT_MEMORY       10           0.01   99999  1000000  thrpt    5   8469.991 ± 710.023  ops/s
FastAggregationRLEStressTest.and         CONSTANT_MEMORY       10            0.1   99999  1000000  thrpt    5  18620.905 ± 229.084  ops/s
FastAggregationRLEStressTest.and         CONSTANT_MEMORY       10            0.5   99999  1000000  thrpt    5  12077.086 ±  21.410  ops/s
FastAggregationRLEStressTest.and         CONSTANT_MEMORY      100           0.01   99999  1000000  thrpt    5   1444.833 ± 135.439  ops/s
FastAggregationRLEStressTest.and         CONSTANT_MEMORY      100            0.1   99999  1000000  thrpt    5    163.065 ±   3.768  ops/s
FastAggregationRLEStressTest.and         CONSTANT_MEMORY      100            0.5   99999  1000000  thrpt    5    457.596 ±  15.069  ops/s

after:

Benchmark                         (constructionStrategy)  (count)  (probability)  (seed)   (size)   Mode  Cnt      Score      Error  Units
FastAggregationRLEStressTest.and         CONSTANT_MEMORY       10           0.01   99999  1000000  thrpt    5   9597.927 ± 1438.935  ops/s
FastAggregationRLEStressTest.and         CONSTANT_MEMORY       10            0.1   99999  1000000  thrpt    5  18970.302 ±  186.505  ops/s
FastAggregationRLEStressTest.and         CONSTANT_MEMORY       10            0.5   99999  1000000  thrpt    5  12065.076 ±   50.169  ops/s
FastAggregationRLEStressTest.and         CONSTANT_MEMORY      100           0.01   99999  1000000  thrpt    5   1459.275 ±   19.018  ops/s
FastAggregationRLEStressTest.and         CONSTANT_MEMORY      100            0.1   99999  1000000  thrpt    5    163.538 ±    2.663  ops/s
FastAggregationRLEStressTest.and         CONSTANT_MEMORY      100            0.5   99999  1000000  thrpt    5    457.225 ±    8.471  ops/s

[lemire]

optionally '10' could be made a variable with a comment indicating that it is based on a heuristic (so we know it is a rule of thumb).

[richardstartin]

The user can already call the underlying methods if they want to and I think the parameter would be confusing

[lemire]

@richardstartin This is a fine answer.

[lemire]

@frensjan Would you be willing to check this out and maybe run a benchmark?

[frensjan]

Yes, I'd be happy to. Sorry for the lack of response on #608, still enjoying my winter vacation :)

[frensjan]

LGTM!

In my macro benchmarking for a particular query, FastAggregation.and used to dominate the benchmark at ~80% in cloning a large bitmap to ~4% in FastAggregation.and in total.

In my work I dropped the use of FastAggregation.and(...) and incrementally computed the intersection. Something like:

RoaringBitmap postings = null;

for( ... ) {
    RoaringBitmap p = readBitmap(...);
    postings = and(postings, p);
}

private static RoaringBitmap and(RoaringBitmap a, RoaringBitmap b) {
    if (a == null) {
        return b;
    }

    if (a.getLongSizeInBytes() < b.getLongSizeInBytes()) {
        RoaringBitmap result = a.clone();
        result.and(b);
        return result;
    } else {
        RoaringBitmap result = b.clone();
        result.and(a);
        return result;
    }
}

This improvement in FastAggregation yields slightly faster results in my macro benchmark. The difference seems to stem from the inability to use a cheaper metric to compare the bitmaps on. getLongSizeInBytes comes up as a fairly significant method CPU-wise in profiling; bitmap.highLowContainer.size() as @richardstartin is using should be way cheaper.

I've also tried a variant with cardinalityExceeds but that doesn't make much of a difference.

if (a.cardinalityExceeds(b.getLongCardinality())) {
    ...
`

[richardstartin]

@lemire I'm pretty confident that this isn't going to cause problems so I recommend we merge this

[richardstartin]

Thanks for the feedback @frensjan, much appreciated.

[lemire]

Merging.

I will issue a release.

[richardstartin]

Merging.

I will issue a release.

The release doesn't seem to have propagated yet, but it looks like the tag was issued 45 minutes ago

[lemire]

The release doesn't seem to have propagated yet, but it looks like the tag was issued 45 minutes ago

Yes. A new release is upcoming.