From 49569cd87d8a8499760c355e5569030076f191f1 Mon Sep 17 00:00:00 2001 From: romnn Date: Wed, 29 Apr 2026 23:06:25 +0200 Subject: [PATCH 1/4] =?UTF-8?q?perf(templater):=20skip=20template=20engine?= =?UTF-8?q?=20for=20strings=20without=20`{{`=20(13.7s=20=E2=86=92=205.0s)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/templater/templater.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/templater/templater.go b/internal/templater/templater.go index 896cba23c1..254aeafd0a 100644 --- a/internal/templater/templater.go +++ b/internal/templater/templater.go @@ -82,6 +82,13 @@ func ReplaceWithExtra[T any](v T, cache *Cache, extra map[string]any) T { // Traverse the value and parse any template variables copy, err := deepcopy.TraverseStringsFunc(v, func(v string) (string, error) { + // Skip the template engine entirely if the string has no actions. + // Go templates only fire on `{{`; pure literals can short-circuit. + // This is a large win when iterating many merged taskfile env vars + // whose values are static strings. + if !strings.Contains(v, "{{") { + return v, nil + } tpl, err := template.New("").Funcs(templateFuncs).Parse(v) if err != nil { return v, err From 0e3771ed8b1833316fe974c9082ff73daa098aea Mon Sep 17 00:00:00 2001 From: romnn Date: Wed, 29 Apr 2026 23:37:38 +0200 Subject: [PATCH 2/4] =?UTF-8?q?perf(compiler,templater):=20share=20one=20C?= =?UTF-8?q?ache=20across=20rangeFunc=20loop=20in=20getVariables=20(5.0s=20?= =?UTF-8?q?=E2=86=92=203.1s)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- compiler.go | 37 ++++++++++++++++++++++++--------- internal/templater/templater.go | 32 ++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/compiler.go b/compiler.go index 733d5f3b21..3b00bc3cab 100644 --- a/compiler.go +++ b/compiler.go @@ -54,30 +54,45 @@ func (c *Compiler) getVariables(t *ast.Task, call *Call, evaluateShVars bool) (* result.Set(k, ast.Var{Value: v}) } + // Share a single Cache across every rangeFunc invocation below. The + // previous implementation allocated a fresh Cache per variable, which + // caused Replace -> ToCacheMap to rebuild the full cacheMap on every + // call. SyncVarSet keeps the cacheMap in sync after each result.Set so + // subsequent Replace calls on the same cache can reuse it. + sharedCache := &templater.Cache{Vars: result} + getRangeFunc := func(dir string) func(k string, v ast.Var) error { return func(k string, v ast.Var) error { - cache := &templater.Cache{Vars: result} + // Each variable is processed independently, so clear any sticky + // error from a previous template before evaluating this one. + sharedCache.ResetErr() // Replace values - newVar := templater.ReplaceVar(v, cache) + newVar := templater.ReplaceVar(v, sharedCache) // If the variable should not be evaluated, but is nil, set it to an empty string // This stops empty interface errors when using the templater to replace values later // Preserve the Sh field so it can be displayed in summary if !evaluateShVars && newVar.Value == nil { - result.Set(k, ast.Var{Value: "", Sh: newVar.Sh}) + out := ast.Var{Value: "", Sh: newVar.Sh} + result.Set(k, out) + sharedCache.SyncVarSet(k, out) return nil } // If the variable should not be evaluated and it is set, we can set it and return if !evaluateShVars { - result.Set(k, ast.Var{Value: newVar.Value, Sh: newVar.Sh}) + out := ast.Var{Value: newVar.Value, Sh: newVar.Sh} + result.Set(k, out) + sharedCache.SyncVarSet(k, out) return nil } // Now we can check for errors since we've handled all the cases when we don't want to evaluate - if err := cache.Err(); err != nil { + if err := sharedCache.Err(); err != nil { return err } // If the variable is already set, we can set it and return if newVar.Value != nil || newVar.Sh == nil { - result.Set(k, ast.Var{Value: newVar.Value}) + out := ast.Var{Value: newVar.Value} + result.Set(k, out) + sharedCache.SyncVarSet(k, out) return nil } // If the variable is dynamic, we need to resolve it first @@ -85,7 +100,9 @@ func (c *Compiler) getVariables(t *ast.Task, call *Call, evaluateShVars bool) (* if err != nil { return err } - result.Set(k, ast.Var{Value: static}) + out := ast.Var{Value: static} + result.Set(k, out) + sharedCache.SyncVarSet(k, out) return nil } } @@ -95,11 +112,11 @@ func (c *Compiler) getVariables(t *ast.Task, call *Call, evaluateShVars bool) (* if t != nil { // NOTE(@andreynering): We're manually joining these paths here because // this is the raw task, not the compiled one. - cache := &templater.Cache{Vars: result} - dir := templater.Replace(t.Dir, cache) - if err := cache.Err(); err != nil { + dir := templater.Replace(t.Dir, sharedCache) + if err := sharedCache.Err(); err != nil { return nil, err } + sharedCache.ResetErr() dir = filepathext.SmartJoin(c.Dir, dir) taskRangeFunc = getRangeFunc(dir) } diff --git a/internal/templater/templater.go b/internal/templater/templater.go index 254aeafd0a..b1a5f192ee 100644 --- a/internal/templater/templater.go +++ b/internal/templater/templater.go @@ -31,6 +31,38 @@ func (r *Cache) Err() error { return r.err } +// ResetErr clears the sticky error flag so a single Cache can be reused for +// a sequence of independent template operations. Without this, the first +// failed Replace would short-circuit every subsequent call on the same +// Cache. +func (r *Cache) ResetErr() { + r.err = nil +} + +// SyncVarSet updates the internal cacheMap after the caller mutates Vars via +// [ast.Vars.Set]. This lets a single Cache be reused across many Replace +// calls (each interleaved with a Vars.Set) without rebuilding the entire +// cacheMap from scratch via [ast.Vars.ToCacheMap]. The mirroring rules match +// [ast.Vars.ToCacheMap]: dynamic-only entries are excluded, Live takes +// precedence over Value. +// +// No-op if cacheMap has not yet been initialized — the lazy init in Replace +// will pick up the value on first use. +func (r *Cache) SyncVarSet(key string, v ast.Var) { + if r.cacheMap == nil { + return + } + if v.Sh != nil && *v.Sh != "" { + delete(r.cacheMap, key) + return + } + if v.Live != nil { + r.cacheMap[key] = v.Live + } else { + r.cacheMap[key] = v.Value + } +} + func ResolveRef(ref string, cache *Cache) any { // If there is already an error, do nothing if cache.err != nil { From 533a718a9fbb9f0b9333d552f8a38da4731ced71 Mon Sep 17 00:00:00 2001 From: romnn Date: Thu, 30 Apr 2026 00:19:12 +0200 Subject: [PATCH 3/4] =?UTF-8?q?perf(templater):=20share=20cacheMap=20when?= =?UTF-8?q?=20template=20cant=20mutate=20it;=20pool=20clones=20otherwise?= =?UTF-8?q?=20(3.1s=20=E2=86=92=200.44s)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/templater/templater.go | 83 +++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 5 deletions(-) diff --git a/internal/templater/templater.go b/internal/templater/templater.go index b1a5f192ee..58a5ff9931 100644 --- a/internal/templater/templater.go +++ b/internal/templater/templater.go @@ -5,6 +5,7 @@ import ( "fmt" "maps" "strings" + "sync" "github.com/go-task/template" @@ -12,6 +13,43 @@ import ( "github.com/go-task/task/v3/taskfile/ast" ) +// dataMapPool reuses map[string]any backing memory across [ReplaceWithExtra] +// calls. When a template might mutate the dot via sprig's set/unset, we +// clone cache.cacheMap into a fresh map so those mutations cannot leak back +// into cache.cacheMap. The maps are short-lived and identically shaped, so a +// pool drops the per-call allocation without changing semantics. +var dataMapPool = sync.Pool{ + New: func() any { + m := make(map[string]any, 64) + return &m + }, +} + +func acquireDataMap() *map[string]any { + return dataMapPool.Get().(*map[string]any) +} + +func releaseDataMap(m *map[string]any) { + clear(*m) + dataMapPool.Put(m) +} + +// templateMayMutateDot reports whether a template source string might mutate +// its dot (root data map). The only functions registered in templateFuncs +// that mutate the dict passed to them are sprig's `set` and `unset`. Other +// sprig mutators (push/append/prepend on slices) operate on slice values +// retrieved from the dict — they do not modify the dict itself, and the +// original code's shallow [maps.Clone] never protected against those either. +// +// This is a conservative substring check: false positives are fine (they +// just force an unnecessary clone), false negatives would leak mutations. +// "set" appearing inside a literal string ({{"set"}}) or inside an unrelated +// identifier (settings, subset) is treated as potentially mutating, which is +// safe. Checking for "set" alone also catches "unset". +func templateMayMutateDot(s string) bool { + return strings.Contains(s, "set") +} + // Cache is a help struct that allow us to call "replaceX" funcs multiple // times, without having to check for error each time. The first error that // happen will be assigned to r.err, and consecutive calls to funcs will just @@ -105,11 +143,46 @@ func ReplaceWithExtra[T any](v T, cache *Cache, extra map[string]any) T { cache.cacheMap = cache.Vars.ToCacheMap() } - // Create a copy of the cache map to avoid editing the original - // If there is extra data, merge it with the cache map - data := maps.Clone(cache.cacheMap) - if extra != nil { - maps.Copy(data, extra) + // Decide whether the template can read directly from cache.cacheMap or + // whether we must clone it. Cloning is required when there is `extra` + // data to merge in, or when any template traversed below might mutate + // the dict via sprig's set/unset. For all other cases the clone is pure + // overhead — tpl.Execute only reads from the data map. + // + // nil values (nil interface, nil *string) cannot run any template at + // all, so they don't need a cloned data map. Strings are checked for + // "set" anywhere. For other concrete types we conservatively clone. + mayMutate := false + switch { + case extra != nil: + mayMutate = true + case any(v) == nil: + // nil interface — TraverseStringsFunc has no strings to visit. + mayMutate = false + default: + switch tv := any(v).(type) { + case string: + mayMutate = templateMayMutateDot(tv) + case *string: + // nil pointer — no template runs. + mayMutate = tv != nil && templateMayMutateDot(*tv) + default: + // []string, map, struct, etc. — don't risk it. + mayMutate = true + } + } + + var data map[string]any + if mayMutate { + dataPtr := acquireDataMap() + defer releaseDataMap(dataPtr) + data = *dataPtr + maps.Copy(data, cache.cacheMap) + if extra != nil { + maps.Copy(data, extra) + } + } else { + data = cache.cacheMap } // Traverse the value and parse any template variables From 6e29b2323d303146b9f29222a1ea91ece1aa0b90 Mon Sep 17 00:00:00 2001 From: romnn Date: Thu, 30 Apr 2026 00:25:09 +0200 Subject: [PATCH 4/4] =?UTF-8?q?perf(templater):=20cache=20parsed=20templat?= =?UTF-8?q?es=20by=20source=20string=20(0.44s=20=E2=86=92=200.25s)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/templater/templater.go | 37 +++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/internal/templater/templater.go b/internal/templater/templater.go index 58a5ff9931..7a0af39a5f 100644 --- a/internal/templater/templater.go +++ b/internal/templater/templater.go @@ -50,6 +50,39 @@ func templateMayMutateDot(s string) bool { return strings.Contains(s, "set") } +// parsedTemplateCache memoizes parsed [template.Template] objects keyed by +// their source string. Each call to template.New("").Funcs(templateFuncs) +// copies the ~170-entry FuncMap into a fresh Template (sprig + go-task +// builtins) — that copy is the single largest allocator on the hot path of +// listing tasks in a project with many includes, because the same merged +// TaskfileEnv values are processed once per task. +// +// Caching the parsed Template is safe: once parsed, a Template may be +// executed concurrently from multiple goroutines (Go docs guarantee this), +// and Execute reads from but does not mutate the Template. The cache key is +// the unparsed source — sufficient because templateFuncs is a package-level +// singleton. +// +// We cache errors as well so that a malformed template surfaces the same +// error on every call instead of being re-parsed each time. +var parsedTemplateCache sync.Map // map[string]parsedTemplateEntry + +type parsedTemplateEntry struct { + tpl *template.Template + err error +} + +func cachedParse(text string) (*template.Template, error) { + if v, ok := parsedTemplateCache.Load(text); ok { + entry := v.(parsedTemplateEntry) + return entry.tpl, entry.err + } + tpl, err := template.New("").Funcs(templateFuncs).Parse(text) + actual, _ := parsedTemplateCache.LoadOrStore(text, parsedTemplateEntry{tpl: tpl, err: err}) + entry := actual.(parsedTemplateEntry) + return entry.tpl, entry.err +} + // Cache is a help struct that allow us to call "replaceX" funcs multiple // times, without having to check for error each time. The first error that // happen will be assigned to r.err, and consecutive calls to funcs will just @@ -115,7 +148,7 @@ func ResolveRef(ref string, cache *Cache) any { if ref == "." { return cache.cacheMap } - t, err := template.New("resolver").Funcs(templateFuncs).Parse(fmt.Sprintf("{{%s}}", ref)) + t, err := cachedParse(fmt.Sprintf("{{%s}}", ref)) if err != nil { cache.err = err return nil @@ -194,7 +227,7 @@ func ReplaceWithExtra[T any](v T, cache *Cache, extra map[string]any) T { if !strings.Contains(v, "{{") { return v, nil } - tpl, err := template.New("").Funcs(templateFuncs).Parse(v) + tpl, err := cachedParse(v) if err != nil { return v, err }