prometheus icon indicating copy to clipboard operation
prometheus copied to clipboard

tsdb: faster postings sort with generic slices.Sort

Open bboreham opened this issue 1 year ago • 8 comments

~Update go.mod to mandate Go 1.18 semantics so we can use generics. This will impact anyone using Prometheus as a library who has not already updated to Go 1.18.~ Edit: this was done by #11279.

Use new experimental package golang.org/x/exp/slices.

For either of the above reasons you might not want to accept the change, but I thought the difference was striking.

name                                                old time/op    new time/op    delta
MemPostings_ensureOrder/few_values_per_label-8         591ms ± 2%     262ms ±31%  -55.74%  (p=0.008 n=5+5)
MemPostings_ensureOrder/few_refs_per_label_value-8    16.1ms ± 0%    11.9ms ± 0%  -25.93%  (p=0.016 n=4+5)
MemPostings_ensureOrder/many_values_per_label-8        543ms ± 7%      80ms ± 3%  -85.26%  (p=0.008 n=5+5)

name                                                old alloc/op   new alloc/op   delta
MemPostings_ensureOrder/few_values_per_label-8         201kB ± 0%      80kB ± 1%  -60.23%  (p=0.016 n=5+4)
MemPostings_ensureOrder/few_refs_per_label_value-8    6.51kB ± 5%    4.37kB ± 1%  -32.87%  (p=0.008 n=5+5)
MemPostings_ensureOrder/many_values_per_label-8        200kB ± 1%      31kB ± 6%  -84.60%  (p=0.008 n=5+5)

name                                                old allocs/op  new allocs/op  delta
MemPostings_ensureOrder/few_values_per_label-8          53.0 ± 8%      23.0 ±22%  -56.60%  (p=0.008 n=5+5)
MemPostings_ensureOrder/few_refs_per_label_value-8      19.0 ± 5%      10.0 ± 0%  -47.37%  (p=0.008 n=5+5)
MemPostings_ensureOrder/many_values_per_label-8         47.2 ±17%      14.8 ±15%  -68.64%  (p=0.008 n=5+5)

Some of the speedup comes from comparing SeriesRef (which is an int64) directly rather than through an interface .Less() call; some comes from exp/slices using "pattern-defeating quicksort(pdqsort)" which is much better on already-sorted data.

Also we don't need the funky trick from #10500.

Across Prometheus there are many other calls to sort; I think all the sort.Strings and sort.Ints will benefit from the same change, but I don't often see them show up in profiles. Anyway I thought I'd post this one as a start.

bboreham avatar Jul 23 '22 14:07 bboreham

/prombench main

roidelapluie avatar Jul 28 '22 08:07 roidelapluie

⏱️ Welcome to Prometheus Benchmarking Tool. ⏱️

Compared versions: PR-11054 and main

After successful deployment, the benchmarking metrics can be viewed at:

Other Commands: To stop benchmark: /prombench cancel To restart benchmark: /prombench restart main

prombot avatar Jul 28 '22 08:07 prombot

Benchmark tests are running for 3 days! If this is intended ignore this message otherwise you can cancel it by commenting: /prombench cancel

prombot avatar Jul 31 '22 08:07 prombot

/prombench cancel

roidelapluie avatar Jul 31 '22 12:07 roidelapluie

Benchmark cancel is in progress.

prombot avatar Jul 31 '22 12:07 prombot

CI is failing on Windows with an error I can't believe is connected to this change:

    testing.go:1097: TempDir RemoveAll cleanup: remove C:\Users\RUNNER~1\AppData\Local\Temp\TestReadCheckpointcompress=false1728930424\001\wal\00000030: The process cannot access the file because it is being used by another process.
--- FAIL: TestReadCheckpoint (1.97s)
    --- FAIL: TestReadCheckpoint/compress=false (1.82s)

This appears to the same as described in #10368.

bboreham avatar Sep 08 '22 15:09 bboreham

Hmmm, lint is failing for unrelated change as well. Looks like a simple fix of getting rid of a type. Would you be able to do it in this PR? Thanks!

codesome avatar Sep 15 '22 07:09 codesome

I fixed the lint error.

bboreham avatar Sep 16 '22 16:09 bboreham