tpl/tplimpl: Rework to get rid of concurrency issues

This more or less completes the simplification of the template handling code in Hugo started in v0.62. The main motivation was to fix a long lasting issue about a crash in HTML content files without front matter. But this commit also comes with a big functional improvement. As we now have moved the base template evaluation to the build stage we now use the same lookup rules for `baseof` as for `list` etc. type of templates. This means that in this simple example you can have a `baseof` template for the `blog` section without having to duplicate the others:: ``` layouts ├── _default │ ├── baseof.html │ ├── list.html │ └── single.html └── blog └── baseof.html ``` Also, when simplifying code, you often get rid of some double work, as shown in the "site building" benchmarks below. These benchmarks looks suspiciously good: ``` name old time/op new time/op delta SiteNew/Bundle_with_image-16 13.0ms ± 1% 10.5ms ± 1% -19.55% (p=0.029 n=4+4) SiteNew/Bundle_with_JSON_file-16 13.1ms ± 1% 10.6ms ± 0% -18.91% (p=0.029 n=4+4) SiteNew/Tags_and_categories-16 46.5ms ± 0% 42.8ms ± 1% -7.96% (p=0.029 n=4+4) SiteNew/Canonify_URLs-16 52.3ms ±12% 47.9ms ± 5% ~ (p=0.200 n=4+4) SiteNew/Deep_content_tree-16 76.5ms ± 1% 70.2ms ± 1% -8.27% (p=0.029 n=4+4) SiteNew/Many_HTML_templates-16 42.7ms ± 1% 36.8ms ± 1% -13.93% (p=0.029 n=4+4) SiteNew/Page_collections-16 57.4ms ± 0% 52.4ms ± 0% -8.64% (p=0.029 n=4+4) name old alloc/op new alloc/op delta SiteNew/Bundle_with_image-16 3.81MB ± 0% 2.22MB ± 0% -41.78% (p=0.029 n=4+4) SiteNew/Bundle_with_JSON_file-16 3.60MB ± 0% 2.00MB ± 0% -44.34% (p=0.029 n=4+4) SiteNew/Tags_and_categories-16 19.1MB ± 2% 14.1MB ± 0% -26.48% (p=0.029 n=4+4) SiteNew/Canonify_URLs-16 70.6MB ± 0% 68.9MB ± 0% -2.42% (p=0.029 n=4+4) SiteNew/Deep_content_tree-16 37.0MB ± 0% 31.2MB ± 0% -15.77% (p=0.029 n=4+4) SiteNew/Many_HTML_templates-16 17.5MB ± 0% 10.5MB ± 0% -39.88% (p=0.029 n=4+4) SiteNew/Page_collections-16 25.9MB ± 0% 21.2MB ± 0% -17.96% (p=0.029 n=4+4) name old allocs/op new allocs/op delta SiteNew/Bundle_with_image-16 52.3k ± 0% 26.1k ± 0% -50.17% (p=0.029 n=4+4) SiteNew/Bundle_with_JSON_file-16 52.3k ± 0% 26.1k ± 0% -50.16% (p=0.029 n=4+4) SiteNew/Tags_and_categories-16 335k ± 1% 269k ± 0% -19.53% (p=0.029 n=4+4) SiteNew/Canonify_URLs-16 422k ± 0% 395k ± 0% -6.43% (p=0.029 n=4+4) SiteNew/Deep_content_tree-16 400k ± 0% 313k ± 0% -21.66% (p=0.029 n=4+4) SiteNew/Many_HTML_templates-16 247k ± 0% 143k ± 0% -42.05% (p=0.029 n=4+4) SiteNew/Page_collections-16 282k ± 0% 207k ± 0% -26.46% (p=0.029 n=4+4) ``` Fixes gohugoio#6716 Fixes gohugoio#6760 Fixes gohugoio#6768 Fixes gohugoio#6778
bep · Jan 20, 2020 · 9938be2 · 9938be2
1 parent d3e8ab2
commit 9938be2
Show file tree

Hide file tree

Showing 43 changed files with 1,024 additions and 1,217 deletions.
diff --git a/commands/server.go b/commands/server.go
@@ -416,7 +416,7 @@ func (c *commandeer) serve(s *serverCmd) error {
 		roots = []string{""}
 	}
 
-	templ, err := c.hugo().TextTmpl.Parse("__default_server_error", buildErrorTemplate)
+	templ, err := c.hugo().TextTmpl().Parse("__default_server_error", buildErrorTemplate)
 	if err != nil {
 		return err
 	}
@@ -428,7 +428,7 @@ func (c *commandeer) serve(s *serverCmd) error {
 		s:        s,
 		errorTemplate: func(ctx interface{}) (io.Reader, error) {
 			b := &bytes.Buffer{}
-			err := c.hugo().Tmpl.Execute(templ, b, ctx)
+			err := c.hugo().Tmpl().Execute(templ, b, ctx)
 			return b, err
 		},
 	}

diff --git a/common/types/types.go b/common/types/types.go
@@ -21,6 +21,12 @@ import (
 	"github.com/spf13/cast"
 )
 
+// RLocker represents the read locks in sync.RWMutex.
+type RLocker interface {
+	RLock()
+	RUnlock()
+}
+
 // KeyValueStr is a string tuple.
 type KeyValueStr struct {
 	Key   string

diff --git a/create/content_template_handler.go b/create/content_template_handler.go
@@ -129,9 +129,9 @@ func executeArcheTypeAsTemplate(s *hugolib.Site, name, kind, targetPath, archety
 	archetypeTemplate = []byte(archetypeShortcodeReplacementsPre.Replace(string(archetypeTemplate)))
 
 	// Reuse the Hugo template setup to get the template funcs properly set up.
-	templateHandler := s.Deps.Tmpl.(tpl.TemplateManager)
-	templateName := "_text/" + helpers.Filename(archetypeFilename)
-	if err := templateHandler.AddTemplate(templateName, string(archetypeTemplate)); err != nil {
+	templateHandler := s.Deps.Tmpl().(tpl.TemplateManager)
+	templateName := helpers.Filename(archetypeFilename)
+	if err := templateHandler.AddTemplate("_text/"+templateName, string(archetypeTemplate)); err != nil {
 		return nil, errors.Wrapf(err, "Failed to parse archetype file %q:", archetypeFilename)
 	}
 

diff --git a/deps/deps.go b/deps/deps.go
@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
+	"go.uber.org/atomic"
 
 	"github.com/gohugoio/hugo/cache/filecache"
 	"github.com/gohugoio/hugo/common/loggers"
@@ -38,10 +39,10 @@ type Deps struct {
 	DistinctWarningLog *helpers.DistinctLogger
 
 	// The templates to use. This will usually implement the full tpl.TemplateManager.
-	Tmpl tpl.TemplateHandler `json:"-"`
+	tmpl tpl.TemplateHandler
 
 	// We use this to parse and execute ad-hoc text templates.
-	TextTmpl tpl.TemplateParseFinder `json:"-"`
+	textTmpl tpl.TemplateParseFinder
 
 	// The file systems to use.
 	Fs *hugofs.Fs `json:"-"`
@@ -77,7 +78,7 @@ type Deps struct {
 	OutputFormatsConfig output.Formats
 
 	templateProvider ResourceProvider
-	WithTemplate     func(templ tpl.TemplateManager) error `json:"-"`
+	WithTemplate     func(templ tpl.TemplateManager) error `json:"-"` // TODO1
 
 	// Used in tests
 	OverloadedTemplateFuncs map[string]interface{}
@@ -92,6 +93,9 @@ type Deps struct {
 	// BuildStartListeners will be notified before a build starts.
 	BuildStartListeners *Listeners
 
+	// Atomic flags set during a build.
+	BuildFlags BuildFlags
+
 	*globalErrHandler
 }
 
@@ -153,9 +157,20 @@ type ResourceProvider interface {
 	Clone(deps *Deps) error
 }
 
-// TemplateHandler returns the used tpl.TemplateFinder as tpl.TemplateHandler.
-func (d *Deps) TemplateHandler() tpl.TemplateManager {
-	return d.Tmpl.(tpl.TemplateManager)
+func (d *Deps) Tmpl() tpl.TemplateHandler {
+	return d.tmpl
+}
+
+func (d *Deps) TextTmpl() tpl.TemplateParseFinder {
+	return d.textTmpl
+}
+
+func (d *Deps) SetTmpl(tmpl tpl.TemplateHandler) {
+	d.tmpl = tmpl
+}
+
+func (d *Deps) SetTextTmpl(tmpl tpl.TemplateParseFinder) {
+	d.textTmpl = tmpl
 }
 
 // LoadResources loads translations and templates.
@@ -315,6 +330,7 @@ func (d Deps) ForLanguage(cfg DepsCfg, onCreated func(d *Deps) error) (*Deps, er
 	}
 
 	d.BuildStartListeners = &Listeners{}
+	d.BuildFlags = BuildFlags{}
 
 	return &d, nil
 
@@ -358,3 +374,14 @@ type DepsCfg struct {
 	// Whether we are in running (server) mode
 	Running bool
 }
+
+// BuildFlags are flags that may be turned on during a build.
+type BuildFlags struct {
+	HasLateTemplate atomic.Bool
+}
+
+func NewBuildFlags() BuildFlags {
+	return BuildFlags{
+		//HasLateTemplate: atomic.NewBool(false),
+	}
+}
diff --git a/deps/deps_test.go b/deps/deps_test.go
@@ -0,0 +1,28 @@
+// 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 deps
+
+import (
+	"testing"
+
+	qt "github.com/frankban/quicktest"
+)
+
+func TestBuildFlags(t *testing.T) {
+	c := qt.New(t)
+	var bf BuildFlags
+	c.Assert(bf.HasLateTemplate.Load(), qt.Equals, false)
+	bf.HasLateTemplate.Store(true)
+	c.Assert(bf.HasLateTemplate.Load(), qt.Equals, true)
+}
diff --git a/go.mod b/go.mod
@@ -55,6 +55,7 @@ require (
 	github.com/yuin/goldmark v1.1.21
 	github.com/yuin/goldmark-highlighting v0.0.0-20191202084645-78f32c8dd6d5
 	go.opencensus.io v0.22.0 // indirect
+	go.uber.org/atomic v1.4.0
 	gocloud.dev v0.15.0
 	golang.org/x/image v0.0.0-20191214001246-9130b4cfad52
 	golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553 // indirect

diff --git a/go.sum b/go.sum
@@ -419,6 +419,7 @@ go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU=
 go.opencensus.io v0.22.0 h1:C9hSCOW830chIVkdja34wa6Ky+IzWllkUinR+BtRZd4=
 go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8=
 go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
+go.uber.org/atomic v1.4.0 h1:cxzIVoETapQEqDhQu3QfnvXAV4AlzcvUCxkVUFw3+EU=
 go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
 go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0=
 go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q=

diff --git a/hugolib/alias.go b/hugolib/alias.go
@@ -81,7 +81,7 @@ func (s *Site) writeDestAlias(path, permalink string, outputFormat output.Format
 }
 
 func (s *Site) publishDestAlias(allowRoot bool, path, permalink string, outputFormat output.Format, p page.Page) (err error) {
-	handler := newAliasHandler(s.Tmpl, s.Log, allowRoot)
+	handler := newAliasHandler(s.Tmpl(), s.Log, allowRoot)
 
 	s.Log.DEBUG.Println("creating alias:", path, "redirecting to", permalink)
 

diff --git a/hugolib/cascade_test.go b/hugolib/cascade_test.go
@@ -68,12 +68,12 @@ func TestCascade(t *testing.T) {
         42|taxonomy|tags/blue|blue|home.png|tags|HTML-|
         42|section|sect3|Cascade Home|home.png|sect3|HTML-|
         42|taxonomyTerm|tags|Cascade Home|home.png|tags|HTML-|
-        42|page|p2.md|Cascade Home|home.png|page|HTML-|
+        42|page|p2.md|Cascade Home|home.png||HTML-|
         42|page|sect2/p2.md|Cascade Home|home.png|sect2|HTML-|
         42|page|sect3/p1.md|Cascade Home|home.png|sect3|HTML-|
         42|taxonomy|tags/green|green|home.png|tags|HTML-|
-        42|home|_index.md|Home|home.png|page|HTML-|
-        42|page|p1.md|p1|home.png|page|HTML-|
+        42|home|_index.md|Home|home.png||HTML-|
+        42|page|p1.md|p1|home.png||HTML-|
         42|section|sect1/_index.md|Sect1|sect1.png|stype|HTML-|
         42|section|sect1/s1_2/_index.md|Sect1_2|sect1.png|stype|HTML-|
         42|page|sect1/s1_2/p1.md|Sect1_2_p1|sect1.png|stype|HTML-|

diff --git a/hugolib/content_render_hooks_test.go b/hugolib/content_render_hooks_test.go
@@ -13,7 +13,9 @@
 
 package hugolib
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestRenderHooks(t *testing.T) {
 	config := `

diff --git a/hugolib/filesystems/basefs.go b/hugolib/filesystems/basefs.go
@@ -309,7 +309,7 @@ func (d *SourceFilesystem) Path(filename string) string {
 
 		if strings.HasPrefix(filename, meta.Filename()) {
 			p := strings.TrimPrefix(strings.TrimPrefix(filename, meta.Filename()), filePathSeparator)
-			p = path.Join(meta.PathFile(), p)
+			// TODO1 p = path.Join(meta.PathFile(), p)
 			return p
 		}
 	}

diff --git a/hugolib/hugo_sites.go b/hugolib/hugo_sites.go
@@ -121,6 +121,9 @@ type hugoSitesInit struct {
 	// Loads the data from all of the /data folders.
 	data *lazy.Init
 
+	// Performs late initialization (before render) of the templates.
+	layouts *lazy.Init
+
 	// Loads the Git info for all the pages if enabled.
 	gitInfo *lazy.Init
 
@@ -130,6 +133,7 @@ type hugoSitesInit struct {
 
 func (h *hugoSitesInit) Reset() {
 	h.data.Reset()
+	h.layouts.Reset()
 	h.gitInfo.Reset()
 	h.translations.Reset()
 }
@@ -271,6 +275,7 @@ func newHugoSites(cfg deps.DepsCfg, sites ...*Site) (*HugoSites, error) {
 		Sites:        sites,
 		init: &hugoSitesInit{
 			data:         lazy.New(),
+			layouts:      lazy.New(),
 			gitInfo:      lazy.New(),
 			translations: lazy.New(),
 		},
@@ -289,6 +294,15 @@ func newHugoSites(cfg deps.DepsCfg, sites ...*Site) (*HugoSites, error) {
 		return nil, nil
 	})
 
+	h.init.layouts.Add(func() (interface{}, error) {
+		for _, s := range h.Sites {
+			if err := s.Tmpl().(tpl.TemplateManager).MarkReady(); err != nil {
+				return nil, err
+			}
+		}
+		return nil, nil
+	})
+
 	h.init.translations.Add(func() (interface{}, error) {
 		if len(h.Sites) > 1 {
 			allTranslations := pagesToTranslationsMap(h.Sites)
@@ -429,10 +443,6 @@ func NewHugoSites(cfg deps.DepsCfg) (*HugoSites, error) {
 
 func (s *Site) withSiteTemplates(withTemplates ...func(templ tpl.TemplateManager) error) func(templ tpl.TemplateManager) error {
 	return func(templ tpl.TemplateManager) error {
-		if err := templ.LoadTemplates(""); err != nil {
-			return err
-		}
-
 		for _, wt := range withTemplates {
 			if wt == nil {
 				continue
@@ -619,10 +629,10 @@ func (h *HugoSites) renderCrossSitesArtifacts() error {
 
 	s := h.Sites[0]
 
-	smLayouts := []string{"sitemapindex.xml", "_default/sitemapindex.xml", "_internal/_default/sitemapindex.xml"}
+	templ := s.lookupLayouts("sitemapindex.xml", "_default/sitemapindex.xml", "_internal/_default/sitemapindex.xml")
 
 	return s.renderAndWriteXML(&s.PathSpec.ProcessingStats.Sitemaps, "sitemapindex",
-		s.siteCfg.sitemap.Filename, h.toSiteInfos(), smLayouts...)
+		s.siteCfg.sitemap.Filename, h.toSiteInfos(), templ)
 }
 
 func (h *HugoSites) removePageByFilename(filename string) {
@@ -832,7 +842,7 @@ func (h *HugoSites) resetPageStateFromEvents(idset identity.Identities) {
 				if po.cp == nil {
 					continue
 				}
-				for id, _ := range idset {
+				for id := range idset {
 					if po.cp.dependencyTracker.Search(id) != nil {
 						po.cp.Reset()
 						continue OUTPUTS
@@ -841,7 +851,7 @@ func (h *HugoSites) resetPageStateFromEvents(idset identity.Identities) {
 			}
 
 			for _, s := range p.shortcodeState.shortcodes {
-				for id, _ := range idset {
+				for id := range idset {
 					if idm, ok := s.info.(identity.Manager); ok && idm.Search(id) != nil {
 						for _, po := range p.pageOutputs {
 							if po.cp != nil {

diff --git a/hugolib/hugo_sites_build.go b/hugolib/hugo_sites_build.go
@@ -291,6 +291,10 @@ func (h *HugoSites) assemble(bcfg *BuildCfg) error {
 }
 
 func (h *HugoSites) render(config *BuildCfg) error {
+	if _, err := h.init.layouts.Do(); err != nil {
+		return err
+	}
+
 	siteRenderContext := &siteRenderContext{cfg: config, multihost: h.multihost}
 
 	if !config.PartialReRender {
@@ -312,11 +316,6 @@ func (h *HugoSites) render(config *BuildCfg) error {
 			case <-h.Done():
 				return nil
 			default:
-				// For the non-renderable pages, we use the content iself as
-				// template and we may have to re-parse and execute it for
-				// each output format.
-				h.TemplateHandler().RebuildClone()
-
 				for _, s2 := range h.Sites {
 					// We render site by site, but since the content is lazily rendered
 					// and a site can "borrow" content from other sites, every site

diff --git a/hugolib/hugo_sites_build_errors_test.go b/hugolib/hugo_sites_build_errors_test.go
@@ -37,6 +37,8 @@ func (t testSiteBuildErrorAsserter) assertErrorMessage(e1, e2 string) {
 
 }
 
+// TODO1 add a change test with change in base template only
+
 func TestSiteBuildErrors(t *testing.T) {
 
 	const (
@@ -65,7 +67,8 @@ func TestSiteBuildErrors(t *testing.T) {
 			fileFixer: func(content string) string {
 				return strings.Replace(content, ".Title }}", ".Title }", 1)
 			},
-			assertCreateError: func(a testSiteBuildErrorAsserter, err error) {
+			// Base templates gets parsed at build time.
+			assertBuildError: func(a testSiteBuildErrorAsserter, err error) {
 				a.assertLineNumber(4, err)
 			},
 		},
@@ -90,7 +93,7 @@ func TestSiteBuildErrors(t *testing.T) {
 				a.c.Assert(fe.Position().LineNumber, qt.Equals, 5)
 				a.c.Assert(fe.Position().ColumnNumber, qt.Equals, 1)
 				a.c.Assert(fe.ChromaLexer, qt.Equals, "go-html-template")
-				a.assertErrorMessage("\"layouts/_default/single.html:5:1\": parse failed: template: _default/single.html:5: unexpected \"}\" in operand", fe.Error())
+				a.assertErrorMessage("\"layouts/foo/single.html:5:1\": parse failed: template: foo/single.html:5: unexpected \"}\" in operand", fe.Error())
 
 			},
 		},
@@ -256,6 +259,13 @@ SINGLE L3:
 SINGLE L4:
 SINGLE L5: {{ .Title }} {{ .Content }}
 {{ end }}
+`))
+
+			b.WithTemplatesAdded("layouts/foo/single.html", f(single, `
+SINGLE L2:
+SINGLE L3:
+SINGLE L4:
+SINGLE L5: {{ .Title }} {{ .Content }}
 `))
 
 			b.WithContent("myyaml.md", f(yamlcontent, `---