From 503d20954f10507b9b43c6ee1c38001e53cf0b14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Fri, 3 May 2024 11:04:57 +0200 Subject: Make the cache eviction logic for stale entities more robust Fixes #12458 --- cache/dynacache/dynacache.go | 34 +++++++++++++++++++++++++++++----- cache/dynacache/dynacache_test.go | 12 ++++++------ hugolib/content_map_page.go | 12 +++++++----- hugolib/page__content.go | 29 ++++++++++++++++++----------- hugolib/page__per_output.go | 4 ++-- hugolib/rebuild_test.go | 28 +++++++++++++++++++++++----- hugolib/rendershortcodes_test.go | 37 +++++++++++++++++++++++++++++++++++++ hugolib/site_benchmark_new_test.go | 2 +- resources/resource.go | 17 ++++++++++------- resources/resource/resourcetypes.go | 24 +++++++++++++++++------- resources/transform.go | 5 +++-- tpl/transform/unmarshal.go | 8 ++++---- 12 files changed, 157 insertions(+), 55 deletions(-) diff --git a/cache/dynacache/dynacache.go b/cache/dynacache/dynacache.go index 87ef68f5b..6190dd234 100644 --- a/cache/dynacache/dynacache.go +++ b/cache/dynacache/dynacache.go @@ -385,13 +385,37 @@ type Partition[K comparable, V any] struct { // GetOrCreate gets or creates a value for the given key. func (p *Partition[K, V]) GetOrCreate(key K, create func(key K) (V, error)) (V, error) { + v, err := p.doGetOrCreate(key, create) + if err != nil { + return p.zero, err + } + if resource.StaleVersion(v) > 0 { + p.c.Delete(key) + return p.doGetOrCreate(key, create) + } + return v, err +} + +func (p *Partition[K, V]) doGetOrCreate(key K, create func(key K) (V, error)) (V, error) { v, _, err := p.c.GetOrCreate(key, create) return v, err } +func (p *Partition[K, V]) GetOrCreateWitTimeout(key K, duration time.Duration, create func(key K) (V, error)) (V, error) { + v, err := p.doGetOrCreateWitTimeout(key, duration, create) + if err != nil { + return p.zero, err + } + if resource.StaleVersion(v) > 0 { + p.c.Delete(key) + return p.doGetOrCreateWitTimeout(key, duration, create) + } + return v, err +} + // GetOrCreateWitTimeout gets or creates a value for the given key and times out if the create function // takes too long. -func (p *Partition[K, V]) GetOrCreateWitTimeout(key K, duration time.Duration, create func(key K) (V, error)) (V, error) { +func (p *Partition[K, V]) doGetOrCreateWitTimeout(key K, duration time.Duration, create func(key K) (V, error)) (V, error) { resultch := make(chan V, 1) errch := make(chan error, 1) @@ -448,7 +472,7 @@ func (p *Partition[K, V]) clearOnRebuild(changeset ...identity.Identity) { shouldDelete := func(key K, v V) bool { // We always clear elements marked as stale. - if resource.IsStaleAny(v) { + if resource.StaleVersion(v) > 0 { return true } @@ -503,8 +527,8 @@ func (p *Partition[K, V]) Keys() []K { func (p *Partition[K, V]) clearStale() { p.c.DeleteFunc(func(key K, v V) bool { - isStale := resource.IsStaleAny(v) - if isStale { + staleVersion := resource.StaleVersion(v) + if staleVersion > 0 { p.trace.Log( logg.StringFunc( func() string { @@ -514,7 +538,7 @@ func (p *Partition[K, V]) clearStale() { ) } - return isStale + return staleVersion > 0 }) } diff --git a/cache/dynacache/dynacache_test.go b/cache/dynacache/dynacache_test.go index 275e63f0b..a58a8d94b 100644 --- a/cache/dynacache/dynacache_test.go +++ b/cache/dynacache/dynacache_test.go @@ -29,12 +29,12 @@ var ( ) type testItem struct { - name string - isStale bool + name string + staleVersion uint32 } -func (t testItem) IsStale() bool { - return t.isStale +func (t testItem) StaleVersion() uint32 { + return t.staleVersion } func (t testItem) IdentifierBase() string { @@ -109,7 +109,7 @@ func newTestCache(t *testing.T) *Cache { p2.GetOrCreate("clearBecauseStale", func(string) (testItem, error) { return testItem{ - isStale: true, + staleVersion: 32, }, nil }) @@ -121,7 +121,7 @@ func newTestCache(t *testing.T) *Cache { p2.GetOrCreate("clearNever", func(string) (testItem, error) { return testItem{ - isStale: false, + staleVersion: 0, }, nil }) diff --git a/hugolib/content_map_page.go b/hugolib/content_map_page.go index 5a6b49c55..a0bff7472 100644 --- a/hugolib/content_map_page.go +++ b/hugolib/content_map_page.go @@ -824,10 +824,10 @@ func (s *contentNodeShifter) Insert(old, new contentNodeI) contentNodeI { if !ok { panic(fmt.Sprintf("unknown type %T", new)) } - if newp != old { - resource.MarkStale(old) - } if vv.s.languagei == newp.s.languagei { + if newp != old { + resource.MarkStale(old) + } return new } is := make(contentNodeIs, s.numLanguages) @@ -843,7 +843,6 @@ func (s *contentNodeShifter) Insert(old, new contentNodeI) contentNodeI { if oldp != newp { resource.MarkStale(oldp) } - vv[newp.s.languagei] = new return vv case *resourceSource: @@ -852,6 +851,9 @@ func (s *contentNodeShifter) Insert(old, new contentNodeI) contentNodeI { panic(fmt.Sprintf("unknown type %T", new)) } if vv.LangIndex() == newp.LangIndex() { + if vv != newp { + resource.MarkStale(vv) + } return new } rs := make(resourceSources, s.numLanguages) @@ -1064,7 +1066,7 @@ func (h *HugoSites) resolveAndClearStateForIdentities( ) for _, id := range changes { - if staler, ok := id.(resource.Staler); ok && !staler.IsStale() { + if staler, ok := id.(resource.Staler); ok { var msgDetail string if p, ok := id.(*pageState); ok && p.File() != nil { msgDetail = fmt.Sprintf(" (%s)", p.File().Filename()) diff --git a/hugolib/page__content.go b/hugolib/page__content.go index f10c25d7b..99ed824cd 100644 --- a/hugolib/page__content.go +++ b/hugolib/page__content.go @@ -418,6 +418,8 @@ func (c *cachedContent) mustSource() []byte { func (c *contentParseInfo) contentSource(s resource.StaleInfo) ([]byte, error) { key := c.sourceKey + versionv := s.StaleVersion() + v, err := c.h.cacheContentSource.GetOrCreate(key, func(string) (*resources.StaleValue[[]byte], error) { b, err := c.readSourceAll() if err != nil { @@ -426,8 +428,8 @@ func (c *contentParseInfo) contentSource(s resource.StaleInfo) ([]byte, error) { return &resources.StaleValue[[]byte]{ Value: b, - IsStaleFunc: func() bool { - return s.IsStale() + StaleVersionFunc: func() uint32 { + return s.StaleVersion() - versionv }, }, nil }) @@ -487,7 +489,7 @@ type contentPlainPlainWords struct { func (c *cachedContent) contentRendered(ctx context.Context, cp *pageContentOutput) (contentSummary, error) { ctx = tpl.Context.DependencyScope.Set(ctx, pageDependencyScopeGlobal) key := c.pi.sourceKey + "/" + cp.po.f.Name - versionv := cp.contentRenderedVersion + versionv := c.version(cp) v, err := c.pm.cacheContentRendered.GetOrCreate(key, func(string) (*resources.StaleValue[contentSummary], error) { cp.po.p.s.Log.Trace(logg.StringFunc(func() string { @@ -504,8 +506,8 @@ func (c *cachedContent) contentRendered(ctx context.Context, cp *pageContentOutp } rs := &resources.StaleValue[contentSummary]{ - IsStaleFunc: func() bool { - return c.IsStale() || cp.contentRenderedVersion != versionv + StaleVersionFunc: func() uint32 { + return c.version(cp) - versionv }, } @@ -607,7 +609,7 @@ var setGetContentCallbackInContext = hcontext.NewContextDispatcher[func(*pageCon func (c *cachedContent) contentToC(ctx context.Context, cp *pageContentOutput) (contentTableOfContents, error) { key := c.pi.sourceKey + "/" + cp.po.f.Name - versionv := cp.contentRenderedVersion + versionv := c.version(cp) v, err := c.pm.contentTableOfContents.GetOrCreate(key, func(string) (*resources.StaleValue[contentTableOfContents], error) { source, err := c.pi.contentSource(c) @@ -713,8 +715,8 @@ func (c *cachedContent) contentToC(ctx context.Context, cp *pageContentOutput) ( return &resources.StaleValue[contentTableOfContents]{ Value: ct, - IsStaleFunc: func() bool { - return c.IsStale() || cp.contentRenderedVersion != versionv + StaleVersionFunc: func() uint32 { + return c.version(cp) - versionv }, }, nil }) @@ -725,16 +727,21 @@ func (c *cachedContent) contentToC(ctx context.Context, cp *pageContentOutput) ( return v.Value, nil } +func (c *cachedContent) version(cp *pageContentOutput) uint32 { + // Both of these gets incremented on change. + return c.StaleVersion() + cp.contentRenderedVersion +} + func (c *cachedContent) contentPlain(ctx context.Context, cp *pageContentOutput) (contentPlainPlainWords, error) { key := c.pi.sourceKey + "/" + cp.po.f.Name - versionv := cp.contentRenderedVersion + versionv := c.version(cp) v, err := c.pm.cacheContentPlain.GetOrCreateWitTimeout(key, cp.po.p.s.Conf.Timeout(), func(string) (*resources.StaleValue[contentPlainPlainWords], error) { var result contentPlainPlainWords rs := &resources.StaleValue[contentPlainPlainWords]{ - IsStaleFunc: func() bool { - return c.IsStale() || cp.contentRenderedVersion != versionv + StaleVersionFunc: func() uint32 { + return c.version(cp) - versionv }, } diff --git a/hugolib/page__per_output.go b/hugolib/page__per_output.go index 2adb5cbb7..6b4b8f55e 100644 --- a/hugolib/page__per_output.go +++ b/hugolib/page__per_output.go @@ -89,8 +89,8 @@ type pageContentOutput struct { // typically included with .RenderShortcodes. otherOutputs map[uint64]*pageContentOutput - contentRenderedVersion int // Incremented on reset. - contentRendered bool // Set on content render. + contentRenderedVersion uint32 // Incremented on reset. + contentRendered bool // Set on content render. // Renders Markdown hooks. renderHooks *renderHooks diff --git a/hugolib/rebuild_test.go b/hugolib/rebuild_test.go index 542810b64..0db418ee1 100644 --- a/hugolib/rebuild_test.go +++ b/hugolib/rebuild_test.go @@ -53,6 +53,11 @@ title: "Home" Home Content. -- content/hometext.txt -- Home Text Content. +-- content/myothersection/myothersectionpage.md -- +--- +title: "myothersectionpage" +--- +myothersectionpage Content. -- layouts/_default/single.html -- Single: {{ .Title }}|{{ .Content }}$ Resources: {{ range $i, $e := .Resources }}{{ $i }}:{{ .RelPermalink }}|{{ .Content }}|{{ end }}$ @@ -135,8 +140,8 @@ func TestRebuildRenameTextFileInLeafBundle(t *testing.T) { b.RenameFile("content/mysection/mysectionbundle/mysectionbundletext.txt", "content/mysection/mysectionbundle/mysectionbundletext2.txt").Build() b.AssertFileContent("public/mysection/mysectionbundle/index.html", "mysectionbundletext2", "My Section Bundle Text 2 Content.", "Len Resources: 2|") - b.AssertRenderCountPage(3) - b.AssertRenderCountContent(3) + b.AssertRenderCountPage(5) + b.AssertRenderCountContent(6) }) } @@ -147,6 +152,19 @@ func TestRebuilEditContentFileInLeafBundle(t *testing.T) { b.AssertFileContent("public/mysection/mysectionbundle/index.html", "My Section Bundle Content Content Edited.") } +func TestRebuilEditContentFileThenAnother(t *testing.T) { + b := TestRunning(t, rebuildFilesSimple) + b.EditFileReplaceAll("content/mysection/mysectionbundle/mysectionbundlecontent.md", "Content Content.", "Content Content Edited.").Build() + b.AssertFileContent("public/mysection/mysectionbundle/index.html", "My Section Bundle Content Content Edited.") + b.AssertRenderCountPage(1) + b.AssertRenderCountContent(2) + + b.EditFileReplaceAll("content/myothersection/myothersectionpage.md", "myothersectionpage Content.", "myothersectionpage Content Edited.").Build() + b.AssertFileContent("public/myothersection/myothersectionpage/index.html", "myothersectionpage Content Edited") + b.AssertRenderCountPage(1) + b.AssertRenderCountContent(1) +} + func TestRebuildRenameTextFileInBranchBundle(t *testing.T) { b := TestRunning(t, rebuildFilesSimple) b.AssertFileContent("public/mysection/index.html", "My Section") @@ -163,7 +181,7 @@ func TestRebuildRenameTextFileInHomeBundle(t *testing.T) { b.RenameFile("content/hometext.txt", "content/hometext2.txt").Build() b.AssertFileContent("public/index.html", "hometext2", "Home Text Content.") - b.AssertRenderCountPage(2) + b.AssertRenderCountPage(3) } func TestRebuildRenameDirectoryWithLeafBundle(t *testing.T) { @@ -179,7 +197,7 @@ func TestRebuildRenameDirectoryWithBranchBundle(t *testing.T) { b.AssertFileContent("public/mysectionrenamed/index.html", "My Section") b.AssertFileContent("public/mysectionrenamed/mysectionbundle/index.html", "My Section Bundle") b.AssertFileContent("public/mysectionrenamed/mysectionbundle/mysectionbundletext.txt", "My Section Bundle Text 2 Content.") - b.AssertRenderCountPage(2) + b.AssertRenderCountPage(3) } func TestRebuildRenameDirectoryWithRegularPageUsedInHome(t *testing.T) { @@ -278,7 +296,7 @@ func TestRebuildRenameDirectoryWithBranchBundleFastRender(t *testing.T) { b.AssertFileContent("public/mysectionrenamed/index.html", "My Section") b.AssertFileContent("public/mysectionrenamed/mysectionbundle/index.html", "My Section Bundle") b.AssertFileContent("public/mysectionrenamed/mysectionbundle/mysectionbundletext.txt", "My Section Bundle Text 2 Content.") - b.AssertRenderCountPage(2) + b.AssertRenderCountPage(3) } func TestRebuilErrorRecovery(t *testing.T) { diff --git a/hugolib/rendershortcodes_test.go b/hugolib/rendershortcodes_test.go index 67b1a85ef..313c80a73 100644 --- a/hugolib/rendershortcodes_test.go +++ b/hugolib/rendershortcodes_test.go @@ -201,6 +201,43 @@ Myshort Original. b.AssertFileContent("public/p1/index.html", "Edited") } +func TestRenderShortcodesEditSectionContentWithShortcodeInIncludedPageIssue12458(t *testing.T) { + t.Parallel() + + files := ` +-- hugo.toml -- +disableLiveReload = true +disableKinds = ["home", "taxonomy", "term", "rss", "sitemap", "robotsTXT", "404"] +-- content/mysection/_index.md -- +--- +title: "My Section" +--- +## p1-h1 +{{% include "p2" %}} +-- content/mysection/p2.md -- +--- +title: "p2" +--- +### Original +{{% myshort %}} +-- layouts/shortcodes/include.html -- +{{ $p := .Page.GetPage (.Get 0) }} +{{ $p.RenderShortcodes }} +-- layouts/shortcodes/myshort.html -- +Myshort Original. +-- layouts/_default/list.html -- + {{ .Content }} + + + +` + b := TestRunning(t, files) + + b.AssertFileContent("public/mysection/index.html", "p1-h1") + b.EditFileReplaceAll("content/mysection/_index.md", "p1-h1", "p1-h1 Edited").Build() + b.AssertFileContent("public/mysection/index.html", "p1-h1 Edited") +} + func TestRenderShortcodesNestedPageContextIssue12356(t *testing.T) { t.Parallel() diff --git a/hugolib/site_benchmark_new_test.go b/hugolib/site_benchmark_new_test.go index c028ca526..023d8e4d5 100644 --- a/hugolib/site_benchmark_new_test.go +++ b/hugolib/site_benchmark_new_test.go @@ -487,7 +487,7 @@ Edited!!`, p.Title())) // We currently rebuild all the language versions of the same content file. // We could probably optimize that case, but it's not trivial. - b.Assert(int(counters.contentRenderCounter.Load()), qt.Equals, 33) + b.Assert(int(counters.contentRenderCounter.Load()), qt.Equals, 4) b.AssertFileContent("public"+p.RelPermalink()+"index.html", "Edited!!") } diff --git a/resources/resource.go b/resources/resource.go index 867b262fb..0fee69cdd 100644 --- a/resources/resource.go +++ b/resources/resource.go @@ -296,16 +296,19 @@ type hashProvider interface { hash() string } +var _ resource.StaleInfo = (*StaleValue[any])(nil) + type StaleValue[V any] struct { // The value. Value V - // IsStaleFunc reports whether the value is stale. - IsStaleFunc func() bool + // StaleVersionFunc reports the current version of the value. + // This always starts out at 0 and get incremented on staleness. + StaleVersionFunc func() uint32 } -func (s *StaleValue[V]) IsStale() bool { - return s.IsStaleFunc() +func (s *StaleValue[V]) StaleVersion() uint32 { + return s.StaleVersionFunc() } type AtomicStaler struct { @@ -313,11 +316,11 @@ type AtomicStaler struct { } func (s *AtomicStaler) MarkStale() { - atomic.StoreUint32(&s.stale, 1) + atomic.AddUint32(&s.stale, 1) } -func (s *AtomicStaler) IsStale() bool { - return atomic.LoadUint32(&(s.stale)) > 0 +func (s *AtomicStaler) StaleVersion() uint32 { + return atomic.LoadUint32(&(s.stale)) } // For internal use. diff --git a/resources/resource/resourcetypes.go b/resources/resource/resourcetypes.go index 0766fe232..5d9533223 100644 --- a/resources/resource/resourcetypes.go +++ b/resources/resource/resourcetypes.go @@ -233,17 +233,27 @@ type StaleMarker interface { // StaleInfo tells if a resource is marked as stale. type StaleInfo interface { - IsStale() bool + StaleVersion() uint32 } -// IsStaleAny reports whether any of the os is marked as stale. -func IsStaleAny(os ...any) bool { - for _, o := range os { - if s, ok := o.(StaleInfo); ok && s.IsStale() { - return true +// StaleVersion returns the StaleVersion for the given os, +// or 0 if not set. +func StaleVersion(os any) uint32 { + if s, ok := os.(StaleInfo); ok { + return s.StaleVersion() + } + return 0 +} + +// StaleVersionSum calculates the sum of the StaleVersionSum for the given oss. +func StaleVersionSum(oss ...any) uint32 { + var version uint32 + for _, o := range oss { + if s, ok := o.(StaleInfo); ok && s.StaleVersion() > 0 { + version += s.StaleVersion() } } - return false + return version } // MarkStale will mark any of the oses as stale, if possible. diff --git a/resources/transform.go b/resources/transform.go index d9084b178..b498924f5 100644 --- a/resources/transform.go +++ b/resources/transform.go @@ -657,8 +657,9 @@ type resourceAdapterInner struct { *publishOnce } -func (r *resourceAdapterInner) IsStale() bool { - return r.Staler.IsStale() || r.target.IsStale() +func (r *resourceAdapterInner) StaleVersion() uint32 { + // Both of these are incremented on change. + return r.Staler.StaleVersion() + r.target.StaleVersion() } type resourceTransformations struct { diff --git a/tpl/transform/unmarshal.go b/tpl/transform/unmarshal.go index d876c88d7..dc9029c8d 100644 --- a/tpl/transform/unmarshal.go +++ b/tpl/transform/unmarshal.go @@ -95,8 +95,8 @@ func (ns *Namespace) Unmarshal(args ...any) (any, error) { return &resources.StaleValue[any]{ Value: v, - IsStaleFunc: func() bool { - return resource.IsStaleAny(r) + StaleVersionFunc: func() uint32 { + return resource.StaleVersion(r) }, }, nil }) @@ -132,8 +132,8 @@ func (ns *Namespace) Unmarshal(args ...any) (any, error) { return &resources.StaleValue[any]{ Value: v, - IsStaleFunc: func() bool { - return false + StaleVersionFunc: func() uint32 { + return 0 }, }, nil }) -- cgit v1.2.3