From ed4f1edbd729bf75af89879b76fbad931693cd67 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Fri, 21 Sep 2018 10:10:11 +0200 Subject: [PATCH] hugolib: Compare every element in pages cache MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- hugolib/pageCache.go | 16 ++++------------ hugolib/pageCache_test.go | 4 ++-- hugolib/pageSort_test.go | 2 +- hugolib/pages_related.go | 2 +- 4 files changed, 8 insertions(+), 16 deletions(-) diff --git a/hugolib/pageCache.go b/hugolib/pageCache.go index 2ac58492..15598a4f 100644 --- a/hugolib/pageCache.go +++ b/hugolib/pageCache.go @@ -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 } diff --git a/hugolib/pageCache_test.go b/hugolib/pageCache_test.go index 52a7f449..48f595f8 100644 --- a/hugolib/pageCache_test.go +++ b/hugolib/pageCache_test.go @@ -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() diff --git a/hugolib/pageSort_test.go b/hugolib/pageSort_test.go index 02fa8dcc..f842ecd7 100644 --- a/hugolib/pageSort_test.go +++ b/hugolib/pageSort_test.go @@ -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) { diff --git a/hugolib/pages_related.go b/hugolib/pages_related.go index 665019bb..2881a45e 100644 --- a/hugolib/pages_related.go +++ b/hugolib/pages_related.go @@ -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 } } -- 2.30.2