hugolib: Compare every element in pages cache

It is slightly slower, but correctnes is, of course, more important:

```bash
benchmark                             old ns/op     new ns/op     delta
BenchmarkSortByWeightAndReverse-4     367           645           +75.75%

benchmark                             old allocs     new allocs     delta
BenchmarkSortByWeightAndReverse-4     2              2              +0.00%

benchmark                             old bytes     new bytes     delta
BenchmarkSortByWeightAndReverse-4     64            64            +0.00%
```

Running the same benchmark without any cache (i.e. resorting the slice on every iteration) and then compare it to the current version shows that it still is plenty worth it:

```bash
▶ benchcmp 2.bench 1.bench
benchmark                             old ns/op     new ns/op     delta
BenchmarkSortByWeightAndReverse-4     1358757       645           -99.95%

benchmark                             old allocs     new allocs     delta
BenchmarkSortByWeightAndReverse-4     17159          2              -99.99%

benchmark                             old bytes     new bytes     delta
BenchmarkSortByWeightAndReverse-4     274573        64            -99.98%
```

Closes #5239
This commit is contained in:
Bjørn Erik Pedersen 2018-09-21 10:10:11 +02:00
parent 058cc6c2c3
commit ed4f1edbd7
4 changed files with 8 additions and 16 deletions

View file

@ -27,7 +27,7 @@ func (entry pageCacheEntry) matches(pageLists []Pages) bool {
return false
}
for i, p := range pageLists {
if !fastEqualPages(p, entry.in[i]) {
if !pagesEqual(p, entry.in[i]) {
return false
}
}
@ -103,10 +103,8 @@ func (c *pageCache) getP(key string, apply func(p *Pages), pageLists ...Pages) (
}
// "fast" as in: we do not compare every element for big slices, but that is
// good enough for our use cases.
// TODO(bep) there is a similar method in pagination.go. DRY.
func fastEqualPages(p1, p2 Pages) bool {
// pagesEqual returns whether p1 and p2 are equal.
func pagesEqual(p1, p2 Pages) bool {
if p1 == nil && p2 == nil {
return true
}
@ -123,13 +121,7 @@ func fastEqualPages(p1, p2 Pages) bool {
return true
}
step := 1
if len(p1) >= 50 {
step = len(p1) / 10
}
for i := 0; i < len(p1); i += step {
for i := 0; i < len(p1); i++ {
if p1[i] != p2[i] {
return false
}

View file

@ -57,8 +57,8 @@ func TestPageCache(t *testing.T) {
l1.Unlock()
p2, c2 := c1.get("k1", nil, p)
assert.True(t, c2)
assert.True(t, fastEqualPages(p, p2))
assert.True(t, fastEqualPages(p, pages))
assert.True(t, pagesEqual(p, p2))
assert.True(t, pagesEqual(p, pages))
assert.NotNil(t, p)
l2.Lock()

View file

@ -114,7 +114,7 @@ func TestPageSortReverse(t *testing.T) {
assert.Equal(t, 9, p2[0].fuzzyWordCount)
assert.Equal(t, 0, p2[9].fuzzyWordCount)
// cached
assert.True(t, fastEqualPages(p2, p1.Reverse()))
assert.True(t, pagesEqual(p2, p1.Reverse()))
}
func TestPageSortByParam(t *testing.T) {

View file

@ -154,7 +154,7 @@ func newSearchIndexHandler(cfg related.Config) *relatedDocsHandler {
// This assumes that a lock has been acquired.
func (s *relatedDocsHandler) getIndex(p Pages) *related.InvertedIndex {
for _, ci := range s.postingLists {
if fastEqualPages(p, ci.p) {
if pagesEqual(p, ci.p) {
return ci.postingList
}
}