Reduce postingsCacheKey() memory allocations

[pracucci]

What this PR does

We've observed some high memory allocations in postingsCacheKey() and FetchMultiPostings() in store-gateways, when dealing with thousands or even millions of postings. In particular, under some production workload, we observed 14% CPU taken by postingsCacheKey() and 20% CPU taken by the mapassign done by FetchMultiPostings():

In this PR I'm proposing an optimization to reduce the memory allocations both in postingsCacheKey() and FetchMultiPostings().

Benchmarks:

                                      │ before.txt  │             after.txt              │
                                       │   sec/op    │   sec/op     vs base               │
RemoteIndexCache_FetchMultiPostings-12   17.18m ± 6%   12.09m ± 6%  -29.62% (p=0.002 n=6)
StringCacheKeys/postings-12              1.745µ ± 2%   1.216µ ± 2%  -30.32% (p=0.002 n=6)
StringCacheKeys/series_ref-12            115.6n ± 2%   114.4n ± 1%   -1.08% (p=0.041 n=6)
StringCacheKeys/expanded_postings-12     345.3n ± 5%   342.4n ± 2%        ~ (p=0.223 n=6)
geomean                                  5.881µ        4.898µ       -16.72%

                                       │  before.txt   │               after.txt               │
                                       │     B/op      │     B/op      vs base                 │
RemoteIndexCache_FetchMultiPostings-12   12.054Mi ± 0%   6.289Mi ± 0%  -47.83% (p=0.002 n=6)
StringCacheKeys/postings-12               2480.00 ± 0%     80.00 ± 0%  -96.77% (p=0.002 n=6)
StringCacheKeys/series_ref-12               64.00 ± 0%     64.00 ± 0%        ~ (p=1.000 n=6) ¹
StringCacheKeys/expanded_postings-12        192.0 ± 0%     192.0 ± 0%        ~ (p=1.000 n=6) ¹
geomean                                   4.326Ki        1.558Ki       -63.98%
¹ all samples are equal

                                       │ before.txt  │              after.txt               │
                                       │  allocs/op  │  allocs/op   vs base                 │
RemoteIndexCache_FetchMultiPostings-12   61.11k ± 0%   20.27k ± 0%  -66.83% (p=0.002 n=6)
StringCacheKeys/postings-12               5.000 ± 0%    1.000 ± 0%  -80.00% (p=0.002 n=6)
StringCacheKeys/series_ref-12             1.000 ± 0%    1.000 ± 0%        ~ (p=1.000 n=6) ¹
StringCacheKeys/expanded_postings-12      3.000 ± 0%    3.000 ± 0%        ~ (p=1.000 n=6) ¹
geomean                                   30.94         15.70       -49.25%
¹ all samples are equal

Note: the size of postings cached in RemoteIndexCache_FetchMultiPostings is not realistic (too small) so don't look too much at the memory reduction % there, but allocations reduction is still legit.

Which issue(s) this PR fixes or relates to

N/A

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[56quarters]

LGTM modulo the test failure (I wasn't able to reproduce locally)

[56quarters]

nit: this seems very similar to what bytes.Buffer does

[pracucci]

That's true, but I haven't found an easy way to do base64.RawURLEncoding.Encode() without an extra allocation. Also bytes.Buffer.String() does an extra allocation too.

[pracucci]

LGTM modulo the test failure (I wasn't able to reproduce locally)

I'm not sure what's happening, but I noticed that it reports 1 extra allocation when tests run with -race (as in CI). No extra allocation is reported without -race. To move forward, I've changed the test assertion to tolerate 1 extra allocation and added a comment to explain it.