shithub: hugo

Download patch

ref: ed4f1edbd729bf75af89879b76fbad931693cd67
parent: 058cc6c2c3f72bdc4a703380a6d9cb39afe3dd2f
author: Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
date: Fri Sep 21 06:10:11 EDT 2018 

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


--- a/hugolib/pageCache.go
+++ b/hugolib/pageCache.go
@@ -27,7 +27,7 @@
 		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 @@
 
 }
 
-// "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 @@
 		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
 		}
--- a/hugolib/pageCache_test.go
+++ b/hugolib/pageCache_test.go
@@ -57,8 +57,8 @@
 				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()
--- a/hugolib/pageSort_test.go
+++ b/hugolib/pageSort_test.go
@@ -114,7 +114,7 @@
 	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) {
--- a/hugolib/pages_related.go
+++ b/hugolib/pages_related.go
@@ -154,7 +154,7 @@
 // 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
 		}
 	}