From 4ef9baf5bd24b6a105f78eba1147dad9ffabd82a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Tue, 24 Jan 2023 20:57:15 +0100 Subject: [PATCH] Only invoke a given cached partial once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note that this is backed by a LRU cache (which we soon shall see more usage of), so if you're a heavy user of cached partials it may be evicted and refreshed if needed. But in most cases every partial is only invoked once. This commit also adds a timeout (the global `timeout` config option) to make infinite recursion in partials easier to reason about. ``` name old time/op new time/op delta IncludeCached-10 8.92ms ± 0% 8.48ms ± 1% -4.87% (p=0.016 n=4+5) name old alloc/op new alloc/op delta IncludeCached-10 6.65MB ± 0% 5.17MB ± 0% -22.32% (p=0.002 n=6+6) name old allocs/op new allocs/op delta IncludeCached-10 117k ± 0% 71k ± 0% -39.44% (p=0.002 n=6+6) ``` Closes #4086 Updates #9588 --- deps/deps.go | 14 +- go.mod | 2 + go.sum | 6 +- helpers/general.go | 22 +-- helpers/general_test.go | 7 - hugolib/page_test.go | 35 ++++ hugolib/resource_chain_test.go | 5 +- hugolib/site.go | 4 + identity/identity.go | 15 +- identity/identityhash.go | 69 +++++++ identity/identityhash_test.go | 45 +++++ metrics/metrics.go | 4 +- resources/image.go | 5 +- resources/images/config.go | 4 +- resources/images/filters_test.go | 11 +- resources/internal/key.go | 4 +- resources/page/site.go | 8 + resources/resource_factories/create/remote.go | 6 +- tpl/partials/integration_test.go | 56 +++++- tpl/partials/partials.go | 187 ++++++++---------- tpl/partials/partials_test.go | 40 ---- 21 files changed, 346 insertions(+), 203 deletions(-) create mode 100644 identity/identityhash.go create mode 100644 identity/identityhash_test.go delete mode 100644 tpl/partials/partials_test.go diff --git a/deps/deps.go b/deps/deps.go index 02730e825..6842c7331 100644 --- a/deps/deps.go +++ b/deps/deps.go @@ -11,6 +11,7 @@ import ( "github.com/gohugoio/hugo/cache/filecache" "github.com/gohugoio/hugo/common/hexec" "github.com/gohugoio/hugo/common/loggers" + "github.com/gohugoio/hugo/common/types" "github.com/gohugoio/hugo/config" "github.com/gohugoio/hugo/config/security" "github.com/gohugoio/hugo/helpers" @@ -298,11 +299,14 @@ func New(cfg DepsCfg) (*Deps, error) { sp := source.NewSourceSpec(ps, nil, fs.Source) - timeoutms := cfg.Language.GetInt("timeout") - if timeoutms <= 0 { - timeoutms = 3000 + timeout := 30 * time.Second + if cfg.Cfg.IsSet("timeout") { + v := cfg.Cfg.Get("timeout") + d, err := types.ToDurationE(v) + if err == nil { + timeout = d + } } - ignoreErrors := cast.ToStringSlice(cfg.Cfg.Get("ignoreErrors")) ignorableLogger := loggers.NewIgnorableLogger(logger, ignoreErrors...) @@ -329,7 +333,7 @@ func New(cfg DepsCfg) (*Deps, error) { BuildClosers: &Closers{}, BuildState: buildState, Running: cfg.Running, - Timeout: time.Duration(timeoutms) * time.Millisecond, + Timeout: timeout, globalErrHandler: errorHandler, } diff --git a/go.mod b/go.mod index bab064029..d67c4c3e3 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/bep/godartsass v0.16.0 github.com/bep/golibsass v1.1.0 github.com/bep/gowebp v0.2.0 + github.com/bep/lazycache v0.2.0 github.com/bep/overlayfs v0.6.0 github.com/bep/tmc v0.5.1 github.com/clbanning/mxj/v2 v2.5.7 @@ -103,6 +104,7 @@ require ( github.com/google/wire v0.5.0 // indirect github.com/googleapis/gax-go/v2 v2.3.0 // indirect github.com/googleapis/go-type-adapters v1.0.0 // indirect + github.com/hashicorp/golang-lru/v2 v2.0.1 // indirect github.com/inconshreveable/mousetrap v1.0.1 // indirect github.com/invopop/yaml v0.1.0 // indirect github.com/jmespath/go-jmespath v0.4.0 // indirect diff --git a/go.sum b/go.sum index 637bff303..15b478479 100644 --- a/go.sum +++ b/go.sum @@ -179,6 +179,8 @@ github.com/bep/golibsass v1.1.0 h1:pjtXr00IJZZaOdfryNa9wARTB3Q0BmxC3/V1KNcgyTw= github.com/bep/golibsass v1.1.0/go.mod h1:DL87K8Un/+pWUS75ggYv41bliGiolxzDKWJAq3eJ1MA= github.com/bep/gowebp v0.2.0 h1:ZVfK8i9PpZqKHEmthQSt3qCnnHycbLzBPEsVtk2ch2Q= github.com/bep/gowebp v0.2.0/go.mod h1:ZhFodwdiFp8ehGJpF4LdPl6unxZm9lLFjxD3z2h2AgI= +github.com/bep/lazycache v0.2.0 h1:HKrlZTrDxHIrNKqmnurH42ryxkngCMYLfBpyu40VcwY= +github.com/bep/lazycache v0.2.0/go.mod h1:xUIsoRD824Vx0Q/n57+ZO7kmbEhMBOnTjM/iPixNGbg= github.com/bep/overlayfs v0.6.0 h1:sgLcq/qtIzbaQNl2TldGXOkHvqeZB025sPvHOQL+DYo= github.com/bep/overlayfs v0.6.0/go.mod h1:NFjSmn3kCqG7KX2Lmz8qT8VhPPCwZap3UNogXawoQHM= github.com/bep/tmc v0.5.1 h1:CsQnSC6MsomH64gw0cT5f+EwQDcvZz4AazKunFwTpuI= @@ -238,8 +240,6 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20210512163311-63b5d3c536b0/go.m github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.mod h1:AFq3mo9L8Lqqiid3OhADV3RfLJnjiw63cSpi+fDTRC0= github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= -github.com/evanw/esbuild v0.15.18 h1:CM7eAoUjjNkZs1LH0p6fkwtADrbFr4JV2SlT1bUMjEo= -github.com/evanw/esbuild v0.15.18/go.mod h1:iINY06rn799hi48UqEnaQvVfZWe6W9bET78LbvN8VWk= github.com/evanw/esbuild v0.17.0 h1:gGx9TCZDO9k9x1PJdizx6syIpUq29RwrtHWlgDIdQH8= github.com/evanw/esbuild v0.17.0/go.mod h1:iINY06rn799hi48UqEnaQvVfZWe6W9bET78LbvN8VWk= github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k= @@ -401,6 +401,8 @@ github.com/hairyhenderson/go-codeowners v0.2.3-0.20201026200250-cdc7c0759690 h1: github.com/hairyhenderson/go-codeowners v0.2.3-0.20201026200250-cdc7c0759690/go.mod h1:8Qu9UmnhCRunfRv365Z3w+mT/WfLGKJiK+vugY9qNCU= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= +github.com/hashicorp/golang-lru/v2 v2.0.1 h1:5pv5N1lT1fjLg2VQ5KWc7kmucp2x/kvFOnxuVTqZ6x4= +github.com/hashicorp/golang-lru/v2 v2.0.1/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM= github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= diff --git a/helpers/general.go b/helpers/general.go index 462ec773d..c8a676829 100644 --- a/helpers/general.go +++ b/helpers/general.go @@ -23,7 +23,6 @@ import ( "os" "path/filepath" "sort" - "strconv" "strings" "sync" "unicode" @@ -31,8 +30,6 @@ import ( "github.com/gohugoio/hugo/common/loggers" - "github.com/mitchellh/hashstructure" - "github.com/gohugoio/hugo/common/hugo" "github.com/spf13/afero" @@ -219,7 +216,7 @@ func ReaderContains(r io.Reader, subslice []byte) bool { // GetTitleFunc returns a func that can be used to transform a string to // title case. // -// The supported styles are +// # The supported styles are // // - "Go" (strings.Title) // - "AP" (see https://www.apstylebook.com/) @@ -523,20 +520,3 @@ func PrintFs(fs afero.Fs, path string, w io.Writer) { return nil }) } - -// HashString returns a hash from the given elements. -// It will panic if the hash cannot be calculated. -func HashString(elements ...any) string { - var o any - if len(elements) == 1 { - o = elements[0] - } else { - o = elements - } - - hash, err := hashstructure.Hash(o, nil) - if err != nil { - panic(err) - } - return strconv.FormatUint(hash, 10) -} diff --git a/helpers/general_test.go b/helpers/general_test.go index 75119f01d..95d9c5461 100644 --- a/helpers/general_test.go +++ b/helpers/general_test.go @@ -458,10 +458,3 @@ func BenchmarkUniqueStrings(b *testing.B) { } }) } - -func TestHashString(t *testing.T) { - c := qt.New(t) - - c.Assert(HashString("a", "b"), qt.Equals, "2712570657419664240") - c.Assert(HashString("ab"), qt.Equals, "590647783936702392") -} diff --git a/hugolib/page_test.go b/hugolib/page_test.go index 1d9e3e348..939d06d41 100644 --- a/hugolib/page_test.go +++ b/hugolib/page_test.go @@ -24,6 +24,7 @@ import ( "github.com/bep/clock" "github.com/gohugoio/hugo/htesting" + "github.com/gohugoio/hugo/identity" "github.com/gohugoio/hugo/markup/asciidocext" "github.com/gohugoio/hugo/markup/rst" "github.com/gohugoio/hugo/tpl" @@ -2001,3 +2002,37 @@ Page1: {{ $p1.Path }} b.AssertFileContent("public/index.html", "Lang: no", filepath.FromSlash("Page1: a/B/C/Page1.md")) } + +func TestPageHashString(t *testing.T) { + files := ` +-- config.toml -- +baseURL = "https://example.org" +[languages] +[languages.en] +weight = 1 +title = "English" +[languages.no] +weight = 2 +title = "Norsk" +-- content/p1.md -- +--- +title: "p1" +--- +-- content/p2.md -- +--- +title: "p2" +--- +` + + b := NewIntegrationTestBuilder(IntegrationTestConfig{ + T: t, + TxtarString: files, + }).Build() + + p1 := b.H.Sites[0].RegularPages()[0] + p2 := b.H.Sites[0].RegularPages()[1] + sites := p1.Sites() + + b.Assert(identity.HashString(p1), qt.Not(qt.Equals), identity.HashString(p2)) + b.Assert(identity.HashString(sites[0]), qt.Not(qt.Equals), identity.HashString(sites[1])) +} diff --git a/hugolib/resource_chain_test.go b/hugolib/resource_chain_test.go index 4edc2cb31..6249c0aad 100644 --- a/hugolib/resource_chain_test.go +++ b/hugolib/resource_chain_test.go @@ -26,11 +26,10 @@ import ( "testing" "time" - "github.com/gohugoio/hugo/helpers" - qt "github.com/frankban/quicktest" "github.com/gohugoio/hugo/common/loggers" + "github.com/gohugoio/hugo/identity" "github.com/gohugoio/hugo/resources/resource_transformers/tocss/scss" ) @@ -122,7 +121,7 @@ FAILED REMOTE ERROR DETAILS CONTENT: |failed to fetch remote resource: Internal |StatusCode: 500|ContentLength: 16|ContentType: text/plain; charset=utf-8| -`, helpers.HashString(ts.URL+"/sunset.jpg", map[string]any{}))) +`, identity.HashString(ts.URL+"/sunset.jpg", map[string]any{}))) b.AssertFileContent("public/styles.min.a1df58687c3c9cc38bf26532f7b4b2f2c2b0315dcde212376959995c04f11fef.css", "body{background-color:#add8e6}") b.AssertFileContent("public//styles2.min.1cfc52986836405d37f9998a63fd6dd8608e8c410e5e3db1daaa30f78bc273ba.css", "body{background-color:orange}") diff --git a/hugolib/site.go b/hugolib/site.go index 2ffc3a346..0ca7a81b4 100644 --- a/hugolib/site.go +++ b/hugolib/site.go @@ -778,6 +778,10 @@ func (s *SiteInfo) DisqusShortname() string { return s.Config().Services.Disqus.Shortname } +func (s *SiteInfo) GetIdentity() identity.Identity { + return identity.KeyValueIdentity{Key: "site", Value: s.language.Lang} +} + // SiteSocial is a place to put social details on a site level. These are the // standard keys that themes will expect to have available, but can be // expanded to any others on a per site basis diff --git a/identity/identity.go b/identity/identity.go index 9236f0876..033409b80 100644 --- a/identity/identity.go +++ b/identity/identity.go @@ -1,3 +1,16 @@ +// Copyright 2023 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package identity import ( @@ -107,7 +120,7 @@ func (id KeyValueIdentity) Name() string { return id.Key } -// Provider provides the hashable Identity. +// Provider provides the comparable Identity. type Provider interface { // GetIdentity is for internal use. GetIdentity() Identity diff --git a/identity/identityhash.go b/identity/identityhash.go new file mode 100644 index 000000000..ef7b5afa7 --- /dev/null +++ b/identity/identityhash.go @@ -0,0 +1,69 @@ +// Copyright 2023 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package identity + +import ( + "strconv" + + "github.com/mitchellh/hashstructure" +) + +// HashString returns a hash from the given elements. +// It will panic if the hash cannot be calculated. +// Note that this hash should be used primarily for identity, not for change detection as +// it in the more complex values (e.g. Page) will not hash the full content. +func HashString(vs ...any) string { + hash := HashUint64(vs...) + return strconv.FormatUint(hash, 10) +} + +// HashUint64 returns a hash from the given elements. +// It will panic if the hash cannot be calculated. +// Note that this hash should be used primarily for identity, not for change detection as +// it in the more complex values (e.g. Page) will not hash the full content. +func HashUint64(vs ...any) uint64 { + var o any + if len(vs) == 1 { + o = toHashable(vs[0]) + } else { + elements := make([]any, len(vs)) + for i, e := range vs { + elements[i] = toHashable(e) + } + o = elements + } + + hash, err := hashstructure.Hash(o, nil) + if err != nil { + panic(err) + } + return hash +} + +type keyer interface { + Key() string +} + +// For structs, hashstructure.Hash only works on the exported fields, +// so rewrite the input slice for known identity types. +func toHashable(v any) any { + switch t := v.(type) { + case Provider: + return t.GetIdentity() + case keyer: + return t.Key() + default: + return v + } +} diff --git a/identity/identityhash_test.go b/identity/identityhash_test.go new file mode 100644 index 000000000..378c0160d --- /dev/null +++ b/identity/identityhash_test.go @@ -0,0 +1,45 @@ +// Copyright 2023 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package identity + +import ( + "testing" + + qt "github.com/frankban/quicktest" +) + +func TestHashString(t *testing.T) { + c := qt.New(t) + + c.Assert(HashString("a", "b"), qt.Equals, "2712570657419664240") + c.Assert(HashString("ab"), qt.Equals, "590647783936702392") + + var vals []any = []any{"a", "b", tstKeyer{"c"}} + + c.Assert(HashString(vals...), qt.Equals, "12599484872364427450") + c.Assert(vals[2], qt.Equals, tstKeyer{"c"}) + +} + +type tstKeyer struct { + key string +} + +func (t tstKeyer) Key() string { + return t.key +} + +func (t tstKeyer) String() string { + return "key: " + t.key +} diff --git a/metrics/metrics.go b/metrics/metrics.go index c57b1177d..9715a3747 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -27,7 +27,7 @@ import ( "github.com/gohugoio/hugo/common/types" "github.com/gohugoio/hugo/compare" - "github.com/gohugoio/hugo/helpers" + "github.com/gohugoio/hugo/identity" ) // The Provider interface defines an interface for measuring metrics. @@ -242,7 +242,7 @@ func howSimilar(a, b any) int { return 90 } - h1, h2 := helpers.HashString(a), helpers.HashString(b) + h1, h2 := identity.HashString(a), identity.HashString(b) if h1 == h2 { return 100 } diff --git a/resources/image.go b/resources/image.go index ea8a21156..547b3305f 100644 --- a/resources/image.go +++ b/resources/image.go @@ -33,6 +33,7 @@ import ( color_extractor "github.com/marekm4/color-extractor" "github.com/gohugoio/hugo/common/paths" + "github.com/gohugoio/hugo/identity" "github.com/disintegration/gift" @@ -278,7 +279,7 @@ func (i *imageResource) Filter(filters ...any) (images.ImageResource, error) { gfilters = append(gfilters, images.ToFilters(f)...) } - conf.Key = helpers.HashString(gfilters) + conf.Key = identity.HashString(gfilters) conf.TargetFormat = i.Format return i.doWithImageConfig(conf, func(src image.Image) (image.Image, error) { @@ -430,7 +431,7 @@ func (i *imageResource) getImageMetaCacheTargetPath() string { } p1, _ := paths.FileAndExt(df.file) h, _ := i.hash() - idStr := helpers.HashString(h, i.size(), imageMetaVersionNumber, cfgHash) + idStr := identity.HashString(h, i.size(), imageMetaVersionNumber, cfgHash) p := path.Join(df.dir, fmt.Sprintf("%s_%s.json", p1, idStr)) return p } diff --git a/resources/images/config.go b/resources/images/config.go index 6f562ff8e..09a7016c1 100644 --- a/resources/images/config.go +++ b/resources/images/config.go @@ -19,7 +19,7 @@ import ( "strconv" "strings" - "github.com/gohugoio/hugo/helpers" + "github.com/gohugoio/hugo/identity" "github.com/gohugoio/hugo/media" "errors" @@ -139,7 +139,7 @@ func DecodeConfig(m map[string]any) (ImagingConfig, error) { i := ImagingConfig{ Cfg: defaultImaging, - CfgHash: helpers.HashString(m), + CfgHash: identity.HashString(m), } if err := mapstructure.WeakDecode(m, &i.Cfg); err != nil { diff --git a/resources/images/filters_test.go b/resources/images/filters_test.go index 84c8b540d..a8c5601d6 100644 --- a/resources/images/filters_test.go +++ b/resources/images/filters_test.go @@ -16,9 +16,8 @@ package images import ( "testing" - "github.com/gohugoio/hugo/helpers" - qt "github.com/frankban/quicktest" + "github.com/gohugoio/hugo/identity" ) func TestFilterHash(t *testing.T) { @@ -26,8 +25,8 @@ func TestFilterHash(t *testing.T) { f := &Filters{} - c.Assert(helpers.HashString(f.Grayscale()), qt.Equals, helpers.HashString(f.Grayscale())) - c.Assert(helpers.HashString(f.Grayscale()), qt.Not(qt.Equals), helpers.HashString(f.Invert())) - c.Assert(helpers.HashString(f.Gamma(32)), qt.Not(qt.Equals), helpers.HashString(f.Gamma(33))) - c.Assert(helpers.HashString(f.Gamma(32)), qt.Equals, helpers.HashString(f.Gamma(32))) + c.Assert(identity.HashString(f.Grayscale()), qt.Equals, identity.HashString(f.Grayscale())) + c.Assert(identity.HashString(f.Grayscale()), qt.Not(qt.Equals), identity.HashString(f.Invert())) + c.Assert(identity.HashString(f.Gamma(32)), qt.Not(qt.Equals), identity.HashString(f.Gamma(33))) + c.Assert(identity.HashString(f.Gamma(32)), qt.Equals, identity.HashString(f.Gamma(32))) } diff --git a/resources/internal/key.go b/resources/internal/key.go index 1b45d4cc4..2b95871aa 100644 --- a/resources/internal/key.go +++ b/resources/internal/key.go @@ -13,7 +13,7 @@ package internal -import "github.com/gohugoio/hugo/helpers" +import "github.com/gohugoio/hugo/identity" // ResourceTransformationKey are provided by the different transformation implementations. // It identifies the transformation (name) and its configuration (elements). @@ -38,5 +38,5 @@ func (k ResourceTransformationKey) Value() string { return k.Name } - return k.Name + "_" + helpers.HashString(k.elements...) + return k.Name + "_" + identity.HashString(k.elements...) } diff --git a/resources/page/site.go b/resources/page/site.go index 8daff95ae..90fbd5551 100644 --- a/resources/page/site.go +++ b/resources/page/site.go @@ -18,6 +18,7 @@ import ( "time" "github.com/gohugoio/hugo/common/maps" + "github.com/gohugoio/hugo/identity" "github.com/gohugoio/hugo/config" @@ -75,6 +76,9 @@ type Site interface { // Returns a map of all the data inside /data. Data() map[string]any + + // Returns the identity of this site. + GetIdentity() identity.Identity } // Sites represents an ordered list of sites (languages). @@ -117,6 +121,10 @@ func (t testSite) Current() Site { return t } +func (t testSite) GetIdentity() identity.Identity { + return identity.KeyValueIdentity{Key: "site", Value: t.l.Lang} +} + func (t testSite) IsServer() bool { return false } diff --git a/resources/resource_factories/create/remote.go b/resources/resource_factories/create/remote.go index 1ae5e095b..8f1707ed0 100644 --- a/resources/resource_factories/create/remote.go +++ b/resources/resource_factories/create/remote.go @@ -30,7 +30,7 @@ import ( "github.com/gohugoio/hugo/common/hugio" "github.com/gohugoio/hugo/common/maps" "github.com/gohugoio/hugo/common/types" - "github.com/gohugoio/hugo/helpers" + "github.com/gohugoio/hugo/identity" "github.com/gohugoio/hugo/media" "github.com/gohugoio/hugo/resources" "github.com/gohugoio/hugo/resources/resource" @@ -235,9 +235,9 @@ func (c *Client) validateFromRemoteArgs(uri string, options fromRemoteOptions) e func calculateResourceID(uri string, optionsm map[string]any) string { if key, found := maps.LookupEqualFold(optionsm, "key"); found { - return helpers.HashString(key) + return identity.HashString(key) } - return helpers.HashString(uri, optionsm) + return identity.HashString(uri, optionsm) } func addDefaultHeaders(req *http.Request) { diff --git a/tpl/partials/integration_test.go b/tpl/partials/integration_test.go index bda5ddbd5..fcebe6c05 100644 --- a/tpl/partials/integration_test.go +++ b/tpl/partials/integration_test.go @@ -21,6 +21,8 @@ import ( "strings" "testing" + qt "github.com/frankban/quicktest" + "github.com/gohugoio/hugo/htesting/hqt" "github.com/gohugoio/hugo/hugolib" ) @@ -225,7 +227,7 @@ D1 b.Assert(got, hqt.IsSameString, expect) } -// gobench --package ./tpl/partials +// gobench --package ./tpl/partials func BenchmarkIncludeCached(b *testing.B) { files := ` -- config.toml -- @@ -262,7 +264,7 @@ ABCDE } builders := make([]*hugolib.IntegrationTestBuilder, b.N) - for i, _ := range builders { + for i := range builders { builders[i] = hugolib.NewIntegrationTestBuilder(cfg) } @@ -272,3 +274,53 @@ ABCDE builders[i].Build() } } + +func TestIncludeTimeout(t *testing.T) { + t.Parallel() + + files := ` +-- config.toml -- +baseURL = 'http://example.com/' +timeout = '200ms' +-- layouts/index.html -- +{{ partials.Include "foo.html" . }} +-- layouts/partials/foo.html -- +{{ partial "foo.html" . }} + ` + + b, err := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + }, + ).BuildE() + + b.Assert(err, qt.Not(qt.IsNil)) + b.Assert(err.Error(), qt.Contains, "timed out") + +} + +func TestIncludeCachedTimeout(t *testing.T) { + t.Parallel() + + files := ` +-- config.toml -- +baseURL = 'http://example.com/' +timeout = '200ms' +-- layouts/index.html -- +{{ partials.IncludeCached "foo.html" . }} +-- layouts/partials/foo.html -- +{{ partialCached "foo.html" . }} + ` + + b, err := hugolib.NewIntegrationTestBuilder( + hugolib.IntegrationTestConfig{ + T: t, + TxtarString: files, + }, + ).BuildE() + + b.Assert(err, qt.Not(qt.IsNil)) + b.Assert(err.Error(), qt.Contains, "timed out") + +} diff --git a/tpl/partials/partials.go b/tpl/partials/partials.go index eb4ebfe32..3b08604b7 100644 --- a/tpl/partials/partials.go +++ b/tpl/partials/partials.go @@ -17,19 +17,17 @@ package partials import ( "context" - "errors" "fmt" "html/template" "io" "io/ioutil" - "reflect" "strings" - "sync" "time" - texttemplate "github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate" + "github.com/bep/lazycache" - "github.com/gohugoio/hugo/helpers" + "github.com/gohugoio/hugo/identity" + texttemplate "github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate" "github.com/gohugoio/hugo/tpl" @@ -42,32 +40,48 @@ import ( var TestTemplateProvider deps.ResourceProvider type partialCacheKey struct { - name string - variant any + Name string + Variants []any +} +type includeResult struct { + name string + result any + err error +} + +func (k partialCacheKey) Key() string { + if k.Variants == nil { + return k.Name + } + return identity.HashString(append([]any{k.Name}, k.Variants...)...) } func (k partialCacheKey) templateName() string { - if !strings.HasPrefix(k.name, "partials/") { - return "partials/" + k.name + if !strings.HasPrefix(k.Name, "partials/") { + return "partials/" + k.Name } - return k.name + return k.Name } -// partialCache represents a cache of partials protected by a mutex. +// partialCache represents a LRU cache of partials. type partialCache struct { - sync.RWMutex - p map[partialCacheKey]any + cache *lazycache.Cache[string, includeResult] } func (p *partialCache) clear() { - p.Lock() - defer p.Unlock() - p.p = make(map[partialCacheKey]any) + p.cache.DeleteFunc(func(string, includeResult) bool { + return true + }) } // New returns a new instance of the templates-namespaced template functions. func New(deps *deps.Deps) *Namespace { - cache := &partialCache{p: make(map[partialCacheKey]any)} + // This lazycache was introduced in Hugo 0.111.0. + // We're going to expand and consolidate all memory caches in Hugo using this, + // so just set a high limit for now. + lru := lazycache.New[string, includeResult](lazycache.Options{MaxEntries: 1000}) + + cache := &partialCache{cache: lru} deps.BuildStartListeners.Add( func() { cache.clear() @@ -103,21 +117,44 @@ func (c *contextWrapper) Set(in any) string { // A string if the partial is a text/template, or template.HTML when html/template. // Note that ctx is provided by Hugo, not the end user. func (ns *Namespace) Include(ctx context.Context, name string, contextList ...any) (any, error) { - name, result, err := ns.include(ctx, name, contextList...) - if err != nil { - return result, err + res := ns.includWithTimeout(ctx, name, contextList...) + if res.err != nil { + return nil, res.err } if ns.deps.Metrics != nil { - ns.deps.Metrics.TrackValue(name, result, false) + ns.deps.Metrics.TrackValue(res.name, res.result, false) + } + + return res.result, nil +} + +func (ns *Namespace) includWithTimeout(ctx context.Context, name string, dataList ...any) includeResult { + ctx, cancel := context.WithTimeout(ctx, ns.deps.Timeout) + defer cancel() + + res := make(chan includeResult, 1) + + go func() { + res <- ns.include(ctx, name, dataList...) + }() + + select { + case r := <-res: + return r + case <-ctx.Done(): + err := ctx.Err() + if err == context.DeadlineExceeded { + err = fmt.Errorf("partial %q timed out after %s. This is most likely due to infinite recursion. If this is just a slow template, you can try to increase the 'timeout' config setting.", name, ns.deps.Timeout) + } + return includeResult{err: err} } - return result, nil } // include is a helper function that lookups and executes the named partial. // Returns the final template name and the rendered output. -func (ns *Namespace) include(ctx context.Context, name string, dataList ...any) (string, any, error) { +func (ns *Namespace) include(ctx context.Context, name string, dataList ...any) includeResult { var data any if len(dataList) > 0 { data = dataList[0] @@ -137,7 +174,7 @@ func (ns *Namespace) include(ctx context.Context, name string, dataList ...any) } if !found { - return "", "", fmt.Errorf("partial %q not found", name) + return includeResult{err: fmt.Errorf("partial %q not found", name)} } var info tpl.ParseInfo @@ -164,7 +201,7 @@ func (ns *Namespace) include(ctx context.Context, name string, dataList ...any) } if err := ns.deps.Tmpl().ExecuteWithContext(ctx, templ, w, data); err != nil { - return "", nil, err + return includeResult{err: err} } var result any @@ -177,101 +214,41 @@ func (ns *Namespace) include(ctx context.Context, name string, dataList ...any) result = template.HTML(w.(fmt.Stringer).String()) } - return templ.Name(), result, nil + return includeResult{ + name: templ.Name(), + result: result, + } + } // IncludeCached executes and caches partial templates. The cache is created with name+variants as the key. // Note that ctx is provided by Hugo, not the end user. func (ns *Namespace) IncludeCached(ctx context.Context, name string, context any, variants ...any) (any, error) { - key, err := createKey(name, variants...) + start := time.Now() + key := partialCacheKey{ + Name: name, + Variants: variants, + } + + r, found, err := ns.cachedPartials.cache.GetOrCreate(key.Key(), func(string) (includeResult, error) { + r := ns.includWithTimeout(ctx, key.Name, context) + return r, r.err + }) + if err != nil { return nil, err } - result, err := ns.getOrCreate(ctx, key, context) - if err == errUnHashable { - // Try one more - key.variant = helpers.HashString(key.variant) - result, err = ns.getOrCreate(ctx, key, context) - } - - return result, err -} - -func createKey(name string, variants ...any) (partialCacheKey, error) { - var variant any - - if len(variants) > 1 { - variant = helpers.HashString(variants...) - } else if len(variants) == 1 { - variant = variants[0] - t := reflect.TypeOf(variant) - switch t.Kind() { - // This isn't an exhaustive list of unhashable types. - // There may be structs with slices, - // but that should be very rare. We do recover from that situation - // below. - case reflect.Slice, reflect.Array, reflect.Map: - variant = helpers.HashString(variant) - } - } - - return partialCacheKey{name: name, variant: variant}, nil -} - -var errUnHashable = errors.New("unhashable") - -func (ns *Namespace) getOrCreate(ctx context.Context, key partialCacheKey, context any) (result any, err error) { - start := time.Now() - defer func() { - if r := recover(); r != nil { - err = r.(error) - if strings.Contains(err.Error(), "unhashable type") { - ns.cachedPartials.RUnlock() - err = errUnHashable - } - } - }() - - ns.cachedPartials.RLock() - p, ok := ns.cachedPartials.p[key] - ns.cachedPartials.RUnlock() - - if ok { - if ns.deps.Metrics != nil { - ns.deps.Metrics.TrackValue(key.templateName(), p, true) + if ns.deps.Metrics != nil { + if found { // The templates that gets executed is measured in Execute. // We need to track the time spent in the cache to // get the totals correct. ns.deps.Metrics.MeasureSince(key.templateName(), start) } - return p, nil + ns.deps.Metrics.TrackValue(key.templateName(), r.result, found) } - // This needs to be done outside the lock. - // See #9588 - _, p, err = ns.include(ctx, key.name, context) - if err != nil { - return nil, err - } - - ns.cachedPartials.Lock() - defer ns.cachedPartials.Unlock() - // Double-check. - if p2, ok := ns.cachedPartials.p[key]; ok { - if ns.deps.Metrics != nil { - ns.deps.Metrics.TrackValue(key.templateName(), p, true) - ns.deps.Metrics.MeasureSince(key.templateName(), start) - } - return p2, nil - - } - if ns.deps.Metrics != nil { - ns.deps.Metrics.TrackValue(key.templateName(), p, false) - } - - ns.cachedPartials.p[key] = p - - return p, nil + return r.result, nil } diff --git a/tpl/partials/partials_test.go b/tpl/partials/partials_test.go deleted file mode 100644 index 490354499..000000000 --- a/tpl/partials/partials_test.go +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright 2019 The Hugo Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package partials - -import ( - "testing" - - qt "github.com/frankban/quicktest" -) - -func TestCreateKey(t *testing.T) { - c := qt.New(t) - m := make(map[any]bool) - - create := func(name string, variants ...any) partialCacheKey { - k, err := createKey(name, variants...) - c.Assert(err, qt.IsNil) - m[k] = true - return k - } - - for i := 0; i < 123; i++ { - c.Assert(create("a", "b"), qt.Equals, partialCacheKey{name: "a", variant: "b"}) - c.Assert(create("a", "b", "c"), qt.Equals, partialCacheKey{name: "a", variant: "9629524865311698396"}) - c.Assert(create("a", 1), qt.Equals, partialCacheKey{name: "a", variant: 1}) - c.Assert(create("a", map[string]string{"a": "av"}), qt.Equals, partialCacheKey{name: "a", variant: "4809626101226749924"}) - c.Assert(create("a", []string{"a", "b"}), qt.Equals, partialCacheKey{name: "a", variant: "2712570657419664240"}) - } -}