From 679c431497acafffd1821353c48487bd5b1d7074 Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Tue, 23 Jun 2026 15:38:26 +0545 Subject: [PATCH 1/2] fix(cel): build env options only on cache miss RunExpressionContext built the full CEL env option set (GetCelEnv, including kubernetes.Library() and one cel.Variable per env key) on every call before consulting the compiled-program cache, then discarded it on cache hits, since cel.NewEnv only runs to compile a new program. This made GetCelEnv (notably kubernetes.Library()) the dominant lifetime allocator on the CEL path: in a production config-db heap profile it accounted for ~38% of alloc_space, called on every evaluation. Move the cache lookup ahead of GetCelEnv and construct env options only inside the cache-miss branch where cel.NewEnv actually consumes them. Cache-hit steady state now skips GetCelEnv entirely. Behavior is unchanged: the cached cel.Program already carries its variable bindings and Eval still receives the serialized data. --- template.go | 49 +++++++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/template.go b/template.go index 9f54937c7..7ea84e606 100644 --- a/template.go +++ b/template.go @@ -187,28 +187,11 @@ func RunExpressionContext(ctx commonsContext.Context, _environment map[string]an return "", err } - envOptions := GetCelEnv(data) - for name, fn := range template.Functions { - _name := name - _fn := fn - envOptions = append(envOptions, cel.Function(_name, cel.Overload( - _name, - nil, - cel.AnyType, - cel.FunctionBinding(func(values ...ref.Val) ref.Val { - ogFunc, ok := _fn.(func() any) - if !ok { - return types.WrapErr(fmt.Errorf("%s is expected to be of type func() any", _name)) - } - - out := ogFunc() - return types.DefaultTypeAdapter.NativeToValue(out) - }), - ))) - } - - envOptions = append(envOptions, template.CelEnvs...) - + // Look up the compiled-program cache BEFORE constructing the CEL env options. + // GetCelEnv (notably kubernetes.Library()) is the dominant allocation on the CEL + // path. On the overwhelmingly common cache hit it would be built and then + // immediately discarded, since cel.NewEnv is only needed to compile a new + // program. Build env options only when we actually need to compile. var prg cel.Program if template.IsCacheable() { cached, ok := celExpressionCache.Get(template.cacheKey(_environment)) @@ -220,6 +203,28 @@ func RunExpressionContext(ctx commonsContext.Context, _environment map[string]an } if prg == nil { + envOptions := GetCelEnv(data) + for name, fn := range template.Functions { + _name := name + _fn := fn + envOptions = append(envOptions, cel.Function(_name, cel.Overload( + _name, + nil, + cel.AnyType, + cel.FunctionBinding(func(values ...ref.Val) ref.Val { + ogFunc, ok := _fn.(func() any) + if !ok { + return types.WrapErr(fmt.Errorf("%s is expected to be of type func() any", _name)) + } + + out := ogFunc() + return types.DefaultTypeAdapter.NativeToValue(out) + }), + ))) + } + + envOptions = append(envOptions, template.CelEnvs...) + env, err := cel.NewEnv(envOptions...) if err != nil { return "", err From 23e1cff2d7f984b0bd4be5abf9f8dc80d5516aad Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Tue, 23 Jun 2026 15:38:36 +0545 Subject: [PATCH 2/2] test(cel): add RunExpressionContext cache-hit benchmark + CI Add BenchmarkRunExpressionContext (cacheHit smallEnv/largeEnv) guarding the cache-hit steady state where GetCelEnv must not run, modeled on the production config-item env shape with no duty/config-db dependencies. Wire it into the benchmark workflow alongside BenchmarkSerialize: base-vs-head benchstat, a report comment posted to the PR, and the existing 5% regression gate. Copy the new bench file into the base worktree so both runs execute the same suite, only the implementation under test differing. --- .github/workflows/benchmark.yml | 7 +- run_expression_bench_test.go | 130 ++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 run_expression_bench_test.go diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 6244e231e..a97364242 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -70,17 +70,18 @@ jobs: run: | mkdir -p .bench git worktree add .bench/base "${{ github.event.pull_request.base.sha }}" - # Override the base's bench file with HEAD's so both runs execute + # Override the base's bench files with HEAD's so both runs execute # the same benchmark suite — only the implementation under test differs. cp serialize_bench_test.go .bench/base/serialize_bench_test.go + cp run_expression_bench_test.go .bench/base/run_expression_bench_test.go - name: Benchmark base run: | cd .bench/base - go test -run=^$ -bench=BenchmarkSerialize -count=6 -timeout 20m \ + go test -run=^$ -bench='BenchmarkSerialize|BenchmarkRunExpressionContext' -count=6 -timeout 20m \ github.com/flanksource/gomplate/v3 | tee "$GITHUB_WORKSPACE/bench-base.txt" - name: Benchmark head run: | - go test -run=^$ -bench=BenchmarkSerialize -count=6 -timeout 20m \ + go test -run=^$ -bench='BenchmarkSerialize|BenchmarkRunExpressionContext' -count=6 -timeout 20m \ github.com/flanksource/gomplate/v3 | tee bench-head.txt - name: Compare run: | diff --git a/run_expression_bench_test.go b/run_expression_bench_test.go new file mode 100644 index 000000000..ac0faca93 --- /dev/null +++ b/run_expression_bench_test.go @@ -0,0 +1,130 @@ +package gomplate + +// This benchmark exercises the CEL expression evaluation path (RunExpressionContext) +// the way callers use it at runtime: with a CacheKey so the compiled cel.Program is +// served from celExpressionCache on every iteration after the first. +// +// Motivation: a production heap profile showed RunExpressionContext -> GetCelEnv +// (notably kubernetes.Library()) + Serialize accounting for the single largest slice +// of lifetime allocation, because GetCelEnv was rebuilt on EVERY evaluation even +// though the compiled program is cached. After iteration 1 (the cache miss), all +// remaining iterations are cache hits; any allocation that remains is the per-call +// overhead that runs regardless of the program cache. +// +// Only the cache-hit steady state is benchmarked: that is the prod hot path and the +// regression guard for the "GetCelEnv must not run on cache hits" invariant. +// +// Run: +// go test -run=^$ -bench=BenchmarkRunExpressionContext -benchmem +// +// Capture a heap profile and inspect it the same way we inspect prod ones: +// go test -run=^$ -bench=BenchmarkRunExpressionContext/cacheHit -benchmem \ +// -memprofile /tmp/cel.mem.pprof -memprofilerate=1 +// go tool pprof -alloc_space -top -nodecount=25 /tmp/cel.mem.pprof +// go tool pprof -alloc_space -peek 'GetCelEnv$' /tmp/cel.mem.pprof + +import ( + "fmt" + "testing" +) + +// benchExprEnv returns an env map shaped like a Kubernetes Pod config item as the +// scraper passes it to template evaluation. GetCelEnv registers one cel.Variable per +// top-level key and Serialize walks the entire structure, so env size directly drives +// the per-call allocation under test. +func benchExprEnv(withNestedConfig bool) map[string]any { + env := map[string]any{ + "id": "0192f0a4-1234-7000-8000-aaaaaaaaaaaa", + "name": "nginx-7c5ddbdf54-abcde", + "namespace": "default", + "config_type": "Kubernetes::Pod", + "config_class": "Pod", + "tags": map[string]any{ + "cluster": "production", + "namespace": "default", + }, + } + + if withNestedConfig { + containers := make([]any, 0, 3) + for i := 0; i < 3; i++ { + containers = append(containers, map[string]any{ + "name": fmt.Sprintf("container-%d", i), + "image": fmt.Sprintf("registry.example.com/app:%d.2.3", i), + "ports": []any{map[string]any{"containerPort": 8080 + i, "protocol": "TCP"}}, + "env": []any{ + map[string]any{"name": "LOG_LEVEL", "value": "info"}, + map[string]any{"name": "REGION", "value": "us-east-1"}, + }, + "resources": map[string]any{ + "limits": map[string]any{"cpu": "500m", "memory": "512Mi"}, + "requests": map[string]any{"cpu": "100m", "memory": "128Mi"}, + }, + }) + } + + env["config"] = map[string]any{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": map[string]any{ + "name": "nginx-7c5ddbdf54-abcde", + "namespace": "default", + "labels": map[string]any{ + "app": "nginx", "team": "platform", "env": "production", "version": "v1.2.3", + }, + "annotations": map[string]any{ + "prometheus.io/scrape": "true", + "prometheus.io/port": "8080", + }, + "ownerReferences": []any{ + map[string]any{"apiVersion": "apps/v1", "kind": "ReplicaSet", "name": "nginx-7c5ddbdf54"}, + }, + }, + "spec": map[string]any{"containers": containers, "nodeName": "ip-10-0-1-23"}, + "status": map[string]any{"phase": "Running", "podIP": "10.0.5.12", "hostIP": "10.0.1.23"}, + } + } + + return env +} + +// exprBenchSink prevents the compiler from optimizing away results. +var exprBenchSink any + +// BenchmarkRunExpressionContext measures the CEL evaluation path on the cache-hit +// steady state: a CacheKey is set so the compiled cel.Program is reused from +// celExpressionCache. After the warm-up run, every iteration is a cache hit, and the +// reported B/op / allocs/op is the per-call overhead that runs regardless of the program +// cache (Serialize + Eval, plus GetCelEnv if a regression reintroduces it before the +// cache lookup). +func BenchmarkRunExpressionContext(b *testing.B) { + const expression = `config_type == "Kubernetes::Pod"` + + for _, withConfig := range []bool{false, true} { + name := "cacheHit/smallEnv" + if withConfig { + name = "cacheHit/largeEnv" + } + b.Run(name, func(b *testing.B) { + env := benchExprEnv(withConfig) + tmpl := Template{ + Expression: expression, + CacheKey: "bench.RunExpressionContext:config_type==Kubernetes::Pod", + } + // Warm the cache once so we measure steady state, not the one-time compile. + if _, err := RunExpression(env, tmpl); err != nil { + b.Fatal(err) + } + + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + out, err := RunExpression(env, tmpl) + if err != nil { + b.Fatal(err) + } + exprBenchSink = out + } + }) + } +}