Skip to content
This repository was archived by the owner on Aug 13, 2019. It is now read-only.

Commit 89a90fe

Browse files
brian-brazilbrian-brazil
authored
Simplify mergedPostings.Seek (#595)
The current implementation leads to very slow behaviour when there's many lists, this no worse than n log k, where k is the number of posting lists. Adjust benchmark to catch this. Remove flattening of without lists, not needed anymore. Benchmark versus 0.4.0 (used in Prometheus 2.7): ``` benchmark old ns/op new ns/op delta BenchmarkHeadPostingForMatchers/n="1"-8 189907976 188863880 -0.55% BenchmarkHeadPostingForMatchers/n="1",j="foo"-8 113950106 110791414 -2.77% BenchmarkHeadPostingForMatchers/j="foo",n="1"-8 104965646 102388760 -2.45% BenchmarkHeadPostingForMatchers/n="1",j!="foo"-8 138743592 104424250 -24.74% BenchmarkHeadPostingForMatchers/i=~".*"-8 5279594954 5206096267 -1.39% BenchmarkHeadPostingForMatchers/i=~".+"-8 8004610589 6184527719 -22.74% BenchmarkHeadPostingForMatchers/i=~""-8 2476042646 1003920432 -59.45% BenchmarkHeadPostingForMatchers/i!=""-8 7178244655 6059725323 -15.58% BenchmarkHeadPostingForMatchers/n="1",i=~".*",j="foo"-8 199342649 166642946 -16.40% BenchmarkHeadPostingForMatchers/n="1",i=~".*",i!="2",j="foo"-8 215774683 167515095 -22.37% BenchmarkHeadPostingForMatchers/n="1",i!=""-8 2214714769 392943663 -82.26% BenchmarkHeadPostingForMatchers/n="1",i!="",j="foo"-8 2148727410 322289262 -85.00% BenchmarkHeadPostingForMatchers/n="1",i=~".+",j="foo"-8 2170658009 338458171 -84.41% BenchmarkHeadPostingForMatchers/n="1",i=~"1.+",j="foo"-8 235720135 70597905 -70.05% BenchmarkHeadPostingForMatchers/n="1",i=~".+",i!="2",j="foo"-8 2190570590 343034307 -84.34% BenchmarkHeadPostingForMatchers/n="1",i=~".+",i!~"2.*",j="foo"-8 2373784439 387297908 -83.68% benchmark old allocs new allocs delta BenchmarkHeadPostingForMatchers/n="1"-8 33 33 +0.00% BenchmarkHeadPostingForMatchers/n="1",j="foo"-8 33 33 +0.00% BenchmarkHeadPostingForMatchers/j="foo",n="1"-8 33 33 +0.00% BenchmarkHeadPostingForMatchers/n="1",j!="foo"-8 41 39 -4.88% BenchmarkHeadPostingForMatchers/i=~".*"-8 56 56 +0.00% BenchmarkHeadPostingForMatchers/i=~".+"-8 251577 100115 -60.21% BenchmarkHeadPostingForMatchers/i=~""-8 251123 100077 -60.15% BenchmarkHeadPostingForMatchers/i!=""-8 251525 100112 -60.20% BenchmarkHeadPostingForMatchers/n="1",i=~".*",j="foo"-8 42 39 -7.14% BenchmarkHeadPostingForMatchers/n="1",i=~".*",i!="2",j="foo"-8 52 42 -19.23% BenchmarkHeadPostingForMatchers/n="1",i!=""-8 251069 100101 -60.13% BenchmarkHeadPostingForMatchers/n="1",i!="",j="foo"-8 251473 100101 -60.19% BenchmarkHeadPostingForMatchers/n="1",i=~".+",j="foo"-8 250914 100102 -60.11% BenchmarkHeadPostingForMatchers/n="1",i=~"1.+",j="foo"-8 30038 11181 -62.78% BenchmarkHeadPostingForMatchers/n="1",i=~".+",i!="2",j="foo"-8 250813 100105 -60.09% BenchmarkHeadPostingForMatchers/n="1",i=~".+",i!~"2.*",j="foo"-8 281503 111260 -60.48% benchmark old bytes new bytes delta BenchmarkHeadPostingForMatchers/n="1"-8 10887600 10887600 +0.00% BenchmarkHeadPostingForMatchers/n="1",j="foo"-8 5456416 5456416 +0.00% BenchmarkHeadPostingForMatchers/j="foo",n="1"-8 5456416 5456416 +0.00% BenchmarkHeadPostingForMatchers/n="1",j!="foo"-8 5456640 5456544 -0.00% BenchmarkHeadPostingForMatchers/i=~".*"-8 258254504 258254472 -0.00% BenchmarkHeadPostingForMatchers/i=~".+"-8 520126192 281554792 -45.87% BenchmarkHeadPostingForMatchers/i=~""-8 263446640 24908456 -90.55% BenchmarkHeadPostingForMatchers/i!=""-8 520121144 281553664 -45.87% BenchmarkHeadPostingForMatchers/n="1",i=~".*",j="foo"-8 7062448 7062272 -0.00% BenchmarkHeadPostingForMatchers/n="1",i=~".*",i!="2",j="foo"-8 7063473 7062384 -0.02% BenchmarkHeadPostingForMatchers/n="1",i!=""-8 274325656 35793776 -86.95% BenchmarkHeadPostingForMatchers/n="1",i!="",j="foo"-8 268926824 30362624 -88.71% BenchmarkHeadPostingForMatchers/n="1",i=~".+",j="foo"-8 268882992 30363000 -88.71% BenchmarkHeadPostingForMatchers/n="1",i=~"1.+",j="foo"-8 33193401 4269304 -87.14% BenchmarkHeadPostingForMatchers/n="1",i=~".+",i!="2",j="foo"-8 268875024 30363096 -88.71% BenchmarkHeadPostingForMatchers/n="1",i=~".+",i!~"2.*",j="foo"-8 300589656 33099784 -88.99% ``` Signed-off-by: Brian Brazil <[email protected]>
1 parent 6ac81cc commit 89a90fe

File tree

3 files changed

+27
-47
lines changed

3 files changed

+27
-47
lines changed

‎head_bench_test.go

+13-11
Original file line numberDiff line numberDiff line change
@@ -56,20 +56,21 @@ func BenchmarkHeadPostingForMatchers(b *testing.B) {
5656
testutil.Ok(b, h.Close())
5757
}()
5858

59-
var hash uint64
59+
var ref uint64
60+
61+
addSeries := func(l labels.Labels) {
62+
ref++
63+
h.getOrCreateWithID(ref, l.Hash(), l)
64+
}
65+
6066
for n := 0; n < 10; n++ {
6167
for i := 0; i < 100000; i++ {
62-
h.getOrCreate(hash, labels.FromStrings("i", strconv.Itoa(i), "n", strconv.Itoa(i), "j", "foo"))
63-
hash++
68+
addSeries(labels.FromStrings("i", strconv.Itoa(i), "n", strconv.Itoa(n), "j", "foo"))
6469
// Have some series that won't be matched, to properly test inverted matches.
65-
h.getOrCreate(hash, labels.FromStrings("i", strconv.Itoa(i), "n", strconv.Itoa(i), "j", "bar"))
66-
hash++
67-
h.getOrCreate(hash, labels.FromStrings("i", strconv.Itoa(i), "n", "0_"+strconv.Itoa(i), "j", "bar"))
68-
hash++
69-
h.getOrCreate(hash, labels.FromStrings("i", strconv.Itoa(i), "n", "1_"+strconv.Itoa(i), "j", "bar"))
70-
hash++
71-
h.getOrCreate(hash, labels.FromStrings("i", strconv.Itoa(i), "n", "2_"+strconv.Itoa(i), "j", "bar"))
72-
hash++
70+
addSeries(labels.FromStrings("i", strconv.Itoa(i), "n", strconv.Itoa(n), "j", "bar"))
71+
addSeries(labels.FromStrings("i", strconv.Itoa(i), "n", "0_"+strconv.Itoa(n), "j", "bar"))
72+
addSeries(labels.FromStrings("i", strconv.Itoa(i), "n", "1_"+strconv.Itoa(n), "j", "bar"))
73+
addSeries(labels.FromStrings("i", strconv.Itoa(i), "n", "2_"+strconv.Itoa(n), "j", "foo"))
7374
}
7475
}
7576

@@ -100,6 +101,7 @@ func BenchmarkHeadPostingForMatchers(b *testing.B) {
100101
{`i!=""`, []labels.Matcher{iNotEmpty}},
101102
{`n="1",i=~".*",j="foo"`, []labels.Matcher{n1, iStar, jFoo}},
102103
{`n="1",i=~".*",i!="2",j="foo"`, []labels.Matcher{n1, iStar, iNot2, jFoo}},
104+
{`n="1",i!=""`, []labels.Matcher{n1, iNotEmpty}},
103105
{`n="1",i!="",j="foo"`, []labels.Matcher{n1, iNotEmpty, jFoo}},
104106
{`n="1",i=~".+",j="foo"`, []labels.Matcher{n1, iPlus, jFoo}},
105107
{`n="1",i=~"1.+",j="foo"`, []labels.Matcher{n1, i1Plus, jFoo}},

‎index/postings.go

+14-27
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,6 @@ func (h *postingsHeap) Pop() interface{} {
404404
type mergedPostings struct {
405405
h postingsHeap
406406
initilized bool
407-
heaped bool
408407
cur uint64
409408
err error
410409
}
@@ -434,12 +433,9 @@ func (it *mergedPostings) Next() bool {
434433
return false
435434
}
436435

437-
if !it.heaped {
438-
heap.Init(&it.h)
439-
it.heaped = true
440-
}
441436
// The user must issue an initial Next.
442437
if !it.initilized {
438+
heap.Init(&it.h)
443439
it.cur = it.h[0].At()
444440
it.initilized = true
445441
return true
@@ -477,33 +473,24 @@ func (it *mergedPostings) Seek(id uint64) bool {
477473
return false
478474
}
479475
}
480-
if it.cur >= id {
481-
return true
482-
}
483-
// Heapifying when there is lots of Seeks is inefficient,
484-
// mark to be re-heapified on the Next() call.
485-
it.heaped = false
486-
lowest := ^uint64(0)
487-
n := 0
488-
for _, i := range it.h {
489-
if i.Seek(id) {
490-
it.h[n] = i
491-
n++
492-
if i.At() < lowest {
493-
lowest = i.At()
476+
for it.cur < id {
477+
cur := it.h[0]
478+
if !cur.Seek(id) {
479+
heap.Pop(&it.h)
480+
if cur.Err() != nil {
481+
it.err = cur.Err()
482+
return false
494483
}
495-
} else {
496-
if i.Err() != nil {
497-
it.err = i.Err()
484+
if it.h.Len() == 0 {
498485
return false
499486
}
487+
} else {
488+
// Value of top of heap has changed, re-heapify.
489+
heap.Fix(&it.h, 0)
500490
}
491+
492+
it.cur = it.h[0].At()
501493
}
502-
it.h = it.h[:n]
503-
if len(it.h) == 0 {
504-
return false
505-
}
506-
it.cur = lowest
507494
return true
508495
}
509496

‎querier.go

-9
Original file line numberDiff line numberDiff line change
@@ -332,15 +332,6 @@ func PostingsForMatchers(ix IndexReader, ms ...labels.Matcher) (index.Postings,
332332
it := index.Intersect(its...)
333333

334334
for _, n := range notIts {
335-
if _, ok := n.(*index.ListPostings); !ok {
336-
// Best to pre-calculate the merged lists via next rather than have a ton
337-
// of seeks in Without.
338-
pl, err := index.ExpandPostings(n)
339-
if err != nil {
340-
return nil, err
341-
}
342-
n = index.NewListPostings(pl)
343-
}
344335
it = index.Without(it, n)
345336
}
346337

0 commit comments

Comments
 (0)