perf(loader): reuse cached parsed templates

[dwisiswant0]

Proposed changes

in (*Store).areWorkflowOrTemplatesValid, which
is being called during template -validate-ion.

Proof

$ hyperfine --warmup=5 --prepare='sleep 5' './bin/nuclei-dev -validate -duc' './bin/nuclei -validate -duc'
Benchmark 1: ./bin/nuclei-dev -validate -duc
  Time (mean ± σ):     93.662 s ± 41.013 s    [User: 24.995 s, System: 2.811 s]
  Range (min … max):   51.213 s … 169.758 s    10 runs
 
Benchmark 2: ./bin/nuclei -validate -duc
  Time (mean ± σ):      2.490 s ±  0.025 s    [User: 3.163 s, System: 0.301 s]
  Range (min … max):    2.456 s …  2.517 s    10 runs
 
Summary
  ./bin/nuclei -validate -duc ran
   37.62 ± 16.48 times faster than ./bin/nuclei-dev -validate -duc

Note

• ./bin/nuclei-dev built against fix: suppress warn code flag not found & excludes known misc dir #6500.
• ./bin/nuclei built against this PR branch.

Benchmark

                          │     dev      │                new                 │
                          │    sec/op    │   sec/op    vs base                │
StoreValidateTemplates-16   89.853 ± 77%   2.316 ± 1%  -97.42% (p=0.000 n=10)

                          │      dev       │                 new                  │
                          │      B/op      │     B/op      vs base                │
StoreValidateTemplates-16   3481.4Mi ± 14%   682.7Mi ± 0%  -80.39% (p=0.000 n=10)

                          │     dev      │                 new                 │
                          │  allocs/op   │  allocs/op   vs base                │
StoreValidateTemplates-16   39.30M ± 24%   10.13M ± 0%  -74.23% (p=0.000 n=10)

Note

Generated by go test -benchmem -run=^$ -bench ^BenchmarkStoreValidateTemplates$ ./pkg/catalog/loader -v -count=10 -timeout=30m.
For dev branch, apply 7039507 & 1181f1f commits first.

Checklist

• Pull request is created against the dev branch
• All checks passed (lint, unit/integration/regression tests etc.) with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Summary by CodeRabbit

• Refactor

  • Introduced caching in template parsing and loader paths to reuse parsed templates, reducing redundant work and improving performance for loading, validation, and recompilation without changing behavior.

• Tests

  • Added benchmark tests for loader validation and template parsing to measure performance and allocations.

• Chores

  • Minor test utility adjustment to initialize template-related fields only when available.

[ehsandeep]

2025/09/28 01:36:55 Test: context cancelled
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x105afb0dc]

goroutine 11368 [running]:
github.com/projectdiscovery/nuclei/v3/pkg/templates.Cluster({0xc001342408, 0x109, 0x1032828cc?})
	/Users/runner/work/nuclei/nuclei/pkg/templates/cluster.go:53 +0x20c
github.com/projectdiscovery/nuclei/v3/pkg/templates.ClusterTemplates({0xc001342408, 0x109, 0x11f}, 0xc001152008)
	/Users/runner/work/nuclei/nuclei/pkg/templates/cluster.go:128 +0xc4
github.com/projectdiscovery/nuclei/v3/pkg/core.(*Engine).ExecuteScanWithOpts(0xc000642ed0, {0x10852be08, 0xc001636320}, {0xc001342408, 0x109, 0x11f}, {0x1085384b0, 0xc000212090}, 0x0)
	/Users/runner/work/nuclei/nuclei/pkg/core/execute_options.go:44 +0x16c
github.com/projectdiscovery/nuclei/v3/lib.(*NucleiEngine).ExecuteCallbackWithCtx.func1()
	/Users/runner/work/nuclei/nuclei/lib/sdk.go:272 +0xec
created by github.com/projectdiscovery/nuclei/v3/lib.(*NucleiEngine).ExecuteCallbackWithCtx in goroutine 12
	/Users/runner/work/nuclei/nuclei/lib/sdk.go:270 +0x590
FAIL	github.com/projectdiscovery/nuclei/v3/lib	80.305s
=== RUN   TestSimpleNuclei
[ERR] Could not read nuclei-ignore file: open /Users/runner/Library/Application Support/nuclei/.nuclei-ignore: no such file or directory
[INF] Your current nuclei-templates  are outdated. Latest is v10.2.9
[INF] Successfully updated nuclei-templates (v10.2.9) to /Users/runner/nuclei-templates. GoodLuck!

Nuclei Templates v10.2.9 Changelog
┌───────┬───────┬──────────┬─────────┐
│ TOTAL │ ADDED │ MODIFIED │ REMOVED │
├───────┼───────┼──────────┼─────────┤
│ 11618 │ 11618 │ 0        │ 0       │
└───────┴───────┴──────────┴─────────┘
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x103d7718c]

goroutine 11370 [running]:
github.com/projectdiscovery/nuclei/v3/pkg/templates.Cluster({0xc00618ee00, 0x18, 0xc000093c38?})
	/Users/runner/work/nuclei/nuclei/pkg/templates/cluster.go:53 +0x20c
github.com/projectdiscovery/nuclei/v3/pkg/templates.ClusterTemplates({0xc00618ee00, 0x18, 0x20}, 0xc001374788)
	/Users/runner/work/nuclei/nuclei/pkg/templates/cluster.go:128 +0xc4
github.com/projectdiscovery/nuclei/v3/pkg/core.(*Engine).ExecuteScanWithOpts(0xc001b10150, {0x1067bee78, 0x109319ec0}, {0xc00618ee00, 0x18, 0x20}, {0x1067cb710, 0xc0004b9128}, 0x0)
	/Users/runner/work/nuclei/nuclei/pkg/core/execute_options.go:44 +0x16c
github.com/projectdiscovery/nuclei/v3/lib.(*NucleiEngine).ExecuteCallbackWithCtx.func1()
	/Users/runner/work/nuclei/nuclei/lib/sdk.go:272 +0xec
created by github.com/projectdiscovery/nuclei/v3/lib.(*NucleiEngine).ExecuteCallbackWithCtx in goroutine 14
	/Users/runner/work/nuclei/nuclei/lib/sdk.go:270 +0x590
FAIL	github.com/projectdiscovery/nuclei/v3/lib/tests	80.399s
?   	github.com/projectdiscovery/nuclei/v3/pkg/authprovider	[no test files]

[coderabbitai]

Walkthrough

Adds a parser-cache accessor to the loader and templates parsing flow, refactors Parse to use source-based, cache-aware parsing and option propagation, adjusts testutils initialization, and adds benchmark tests for loader validation and template parsing.

Changes

Cohort / File(s)	Summary
Loader: parser cache integration pkg/catalog/loader/loader.go	Adds Store.parserCacheOnce to lazily obtain a templates.Cache from ExecutorOptions; updated metadata loading and validation to consult cache (Has/prefetch) before parsing while preserving existing error handling and fallbacks.
Loader benchmarks pkg/catalog/loader/loader_bench_test.go	Adds BenchmarkStoreValidateTemplates to measure ValidateTemplates() using a parser-enabled executer; includes setup and benchmark loop.
Templates compile: caching and flow refactor pkg/templates/compile.go	Introduces centralized option propagation (updateRequestOptions), getParser, and parseFromSource; refactors Parse to use parser from options and cache-aware parsing/recompilation logic, reusing cached templates where valid and applying updated engine/options.
Templates benchmarks pkg/templates/compile_bench_test.go	Adds BenchmarkParse and BenchmarkParseTemplateFromReader to measure template parsing from file and reader with path-enforced options and allocation reporting.
Test utils init tweak pkg/testutils/testutils.go	NewMockExecuterOptions now assigns TemplateID, TemplateInfo, and TemplatePath only when info is non-nil (conditional initialization).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Bench as Bench/Caller
  participant Loader as Loader.Store
  participant Parser as Templates.Parser
  participant Cache as Templates.Cache

  Bench->>Loader: ValidateTemplates()
  activate Loader
  Loader->>Loader: parserCacheOnce()
  Loader-->>Loader: templatesCache (may be nil)
  alt Cache available
    Loader->>Cache: Has(templatePath)
    alt In cache
      Cache-->>Loader: Compiled template
      Loader-->>Bench: Use cached, skip parse
    else Not in cache
      Loader->>Parser: Parse(templatePath, options)
      Parser-->>Loader: Compiled template (+cached)
      Loader-->>Bench: Return result
    end
  else No cache
    Loader->>Parser: Parse(templatePath, options)
    Parser-->>Loader: Compiled template
    Loader-->>Bench: Return result
  end
  deactivate Loader

Loading

sequenceDiagram
  autonumber
  participant Caller as Caller
  participant Templates as templates.Parse
  participant GetP as getParser
  participant Parser as Parser
  participant Cache as Parser.Cache

  Caller->>Templates: Parse(filePath, options)
  Templates->>GetP: getParser(options)
  GetP-->>Templates: Parser (from options)
  Templates->>Parser: parseFromSource(filePath, preprocessor, options)
  alt Template in Cache and valid
    Parser->>Cache: Has/Fetch
    Cache-->>Templates: Cached compiled template
    Templates->>Templates: updateRequestOptions / UpdateNewEngineOptions
    Templates-->>Caller: Return compiled (options applied)
  else Not cached or invalid
    Parser->>Parser: Compile template/workflow
    Parser->>Cache: Store compiled template
    Templates->>Templates: updateRequestOptions / UpdateNewEngineOptions
    Templates-->>Caller: Return compiled
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• feat: loading templates performance improvements #6364 — Modifies template parsing and parser caching; closely related to cache retrieval and reuse changes.
• fix: segfault in template caching logic #6421 — Adjusts compile/cache validation and option propagation in compile.go, similar to parseFromSource and recompilation logic here.

Poem

In burrows of code I nibble lines,
A cached carrot saves me time.
Parsers hum and workflows hop,
Benchmarks drum — I never stop.
Merge the patch, I'll twitch with glee,
A faster burrow, just for me! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly identifies the primary change—improving loader performance by reusing cached parsed templates—and follows conventional formatting (scope “loader” with a concise imperative description), making it clear and relevant to the PR’s main objective.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch dwisiswant0/perf/loader/reuse-cached-parsed-templates

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b987b74 and e01196e.

📒 Files selected for processing (1)

• pkg/templates/compile.go (5 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Format Go code using go fmt
Run static analysis with go vet

Files:

• pkg/templates/compile.go

🧠 Learnings (3)

📚 Learning: 2025-07-16T21:27:14.937Z

Learnt from: hdm
PR: projectdiscovery/nuclei#6322
File: pkg/templates/compile.go:79-81
Timestamp: 2025-07-16T21:27:14.937Z
Learning: To make the template caching mechanism in pkg/templates/compile.go production-ready, DSLs need to be updated to use runtime options instead of cached variables, rather than restoring the Compile() calls on each request.

Applied to files:

• pkg/templates/compile.go

📚 Learning: 2025-07-16T21:27:14.937Z

Learnt from: hdm
PR: projectdiscovery/nuclei#6322
File: pkg/templates/compile.go:79-81
Timestamp: 2025-07-16T21:27:14.937Z
Learning: In pkg/templates/compile.go, the template caching mechanism intentionally skips calling Compile() on copied requests to achieve performance benefits. This is the intended design, not a bug. The current implementation isn't production-ready but represents the desired direction.

Applied to files:

• pkg/templates/compile.go

📚 Learning: 2025-07-16T21:28:08.073Z

Learnt from: hdm
PR: projectdiscovery/nuclei#6322
File: pkg/templates/compile.go:59-78
Timestamp: 2025-07-16T21:28:08.073Z
Learning: The shallow copy behavior (`tplCopy := *value`) in pkg/templates/compile.go is intentional design for the template caching mechanism. The partial-copy approach is part of the performance optimization strategy, not a bug requiring deep copying.

Applied to files:

• pkg/templates/compile.go

🧬 Code graph analysis (1)

pkg/templates/compile.go (6)

pkg/templates/templates.go (1)

• Template (35-164)

pkg/templates/preprocessors.go (1)

• Preprocessor (12-17)

pkg/protocols/protocols.go (1)

• ExecutorOptions (61-141)

pkg/templates/parser.go (1)

• Parser (21-30)

pkg/utils/utils.go (1)

• ReaderFromPathOrURL (42-56)

pkg/protocols/common/globalmatchers/globalmatchers.go (1)

• Item (22-27)

🔇 Additional comments (1)

pkg/templates/compile.go (1)

52-101: Don't take the address of the loop-local copy.

Each loop copies the request (rCopy := *r) and then stores &rCopy. Because rCopy is the same variable reused on every iteration, every slot ends up pointing at the very last request instance. Multi-request templates collapse to a single request and the cached data becomes corrupt. Allocate a fresh struct per iteration before updating options (e.g. clone := new(dns.Request); *clone = *r; clone.UpdateOptions(template.Options); template.RequestsDNS[i] = clone) and apply the same fix to all protocol slices in this helper.

⛔ Skipped due to learnings

Learnt from: hdm
PR: projectdiscovery/nuclei#6322
File: pkg/templates/compile.go:79-81
Timestamp: 2025-07-16T21:27:14.937Z
Learning: To make the template caching mechanism in pkg/templates/compile.go production-ready, DSLs need to be updated to use runtime options instead of cached variables, rather than restoring the Compile() calls on each request.

Learnt from: hdm
PR: projectdiscovery/nuclei#6322
File: pkg/templates/compile.go:79-81
Timestamp: 2025-07-16T21:27:14.937Z
Learning: In pkg/templates/compile.go, the template caching mechanism intentionally skips calling Compile() on copied requests to achieve performance benefits. This is the intended design, not a bug. The current implementation isn't production-ready but represents the desired direction.

Learnt from: hdm
PR: projectdiscovery/nuclei#6322
File: pkg/templates/compile.go:59-78
Timestamp: 2025-07-16T21:28:08.073Z
Learning: The shallow copy behavior (`tplCopy := *value`) in pkg/templates/compile.go is intentional design for the template caching mechanism. The partial-copy approach is part of the performance optimization strategy, not a bug requiring deep copying.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/catalog/loader/loader.go (1)

337-359: Fix nil deref on cache miss in metadata path

template may be nil when not present in cache; dereferencing its fields panics. Guard before using.

Apply this diff:

 for templatePath := range validPaths {
-    template, _, _ := templatesCache.Has(templatePath)
-
-    if len(template.RequestsHeadless) > 0 && !store.config.ExecutorOptions.Options.Headless {
+    template, _, _ := templatesCache.Has(templatePath)
+    if template == nil {
+        // Not in cache; skip in metadata-only path
+        continue
+    }
+    if len(template.RequestsHeadless) > 0 && !store.config.ExecutorOptions.Options.Headless {
         continue
     }
-    if len(template.RequestsCode) > 0 && !store.config.ExecutorOptions.Options.EnableCodeTemplates {
+    if len(template.RequestsCode) > 0 && !store.config.ExecutorOptions.Options.EnableCodeTemplates {
         continue
     }
-    if template.IsFuzzing() && !store.config.ExecutorOptions.Options.DAST {
+    if template.IsFuzzing() && !store.config.ExecutorOptions.Options.DAST {
         continue
     }
-    if template.SelfContained && !store.config.ExecutorOptions.Options.EnableSelfContainedTemplates {
+    if template.SelfContained && !store.config.ExecutorOptions.Options.EnableSelfContainedTemplates {
         continue
     }
-    if template.HasFileProtocol() && !store.config.ExecutorOptions.Options.EnableFileTemplates {
+    if template.HasFileProtocol() && !store.config.ExecutorOptions.Options.EnableFileTemplates {
         continue
     }
 
-    if template != nil {
+    if template != nil {
         if loadedTemplateIDs.Has(template.ID) {
             store.logger.Debug().Msgf("Skipping duplicate template ID '%s' from path '%s'", template.ID, templatePath)
             continue
         }

🧹 Nitpick comments (7)

pkg/templates/compile.go (2)

172-197: Reduce global state and potential leaks in parserAccessCache

Global map keyed by *ExecutorOptions can grow unbounded if many option instances are created. getParser can directly assert the concrete type without caching.

Apply these diffs:

@@
-var (
-    ErrCreateTemplateExecutor          = errors.New("cannot create template executer")
-    ErrIncompatibleWithOfflineMatching = errors.New("template can't be used for offline matching")
-    // track how many templates are verfied and by which signer
-    SignatureStats = map[string]*atomic.Uint64{}
-    parserAccessCache = map[*protocols.ExecutorOptions]*Parser{}
-    parserCacheMutex  sync.RWMutex
-)
+var (
+    ErrCreateTemplateExecutor          = errors.New("cannot create template executer")
+    ErrIncompatibleWithOfflineMatching = errors.New("template can't be used for offline matching")
+    // track how many templates are verfied and by which signer
+    SignatureStats = map[string]*atomic.Uint64{}
+)

@@
-    SignatureStats[Unsigned] = &atomic.Uint64{}
-    parserAccessCache = make(map[*protocols.ExecutorOptions]*Parser)
+    SignatureStats[Unsigned] = &atomic.Uint64{}

@@
-func getParser(options *protocols.ExecutorOptions) *Parser {
-    parserCacheMutex.RLock()
-    if parser, exists := parserAccessCache[options]; exists {
-        parserCacheMutex.RUnlock()
-        return parser
-    }
-    parserCacheMutex.RUnlock()
-
-    parserCacheMutex.Lock()
-    defer parserCacheMutex.Unlock()
-
-    if parser, exists := parserAccessCache[options]; exists {
-        return parser
-    }
-
-    parser, ok := options.Parser.(*Parser)
-    if !ok {
-        panic("invalid parser")
-    }
-
-    parserAccessCache[options] = parser
-    return parser
-}
+func getParser(options *protocols.ExecutorOptions) *Parser {
+    parser, ok := options.Parser.(*Parser)
+    if !ok || parser == nil {
+        panic("invalid parser")
+    }
+    return parser
+}

Also applies to: 38-40, 52-53

---

525-538: Use srcOptions.TemplatePath for format detection and error messages

template.Path isn’t set yet here; prefer srcOptions.TemplatePath for reliable format detection and clearer errors.

Apply this diff:

-    template := &Template{}
-    var err error
-    switch config.GetTemplateFormatFromExt(template.Path) {
+    template := &Template{}
+    var err error
+    tplPath := srcOptions.TemplatePath
+    switch config.GetTemplateFormatFromExt(tplPath) {
@@
-    if err != nil {
-        return nil, errkit.Wrapf(err, "failed to parse %s", template.Path)
-    }
+    if err != nil {
+        return nil, errkit.Wrapf(err, "failed to parse %s", tplPath)
+    }

pkg/templates/compile_bench_test.go (3)

16-18: Prime caches before timing to measure steady‑state parsing

Do one warm‑up parse before ResetTimer so measured iterations reflect cache‑hit behavior. Based on learnings.

 setup()
- b.ResetTimer()
- b.ReportAllocs()
+ // warm-up (cold start outside timing)
+ if _, err := templates.Parse(filePath, nil, executerOpts); err != nil {
+   b.Fatalf("warm-up parse failed: %v", err)
+ }
+ b.ResetTimer()
+ b.ReportAllocs()

---

60-62: Prime caches before timing in reader benchmark

Warm up ParseTemplateFromReader once to exclude cold start costs. Based on learnings.

- b.ResetTimer()
- b.ReportAllocs()
+ // warm-up (cold start outside timing)
+ if _, err := templates.ParseTemplateFromReader(bytes.NewReader(content), nil, opts); err != nil {
+   b.Fatalf("warm-up parse (reader) failed: %v", err)
+ }
+ b.ResetTimer()
+ b.ReportAllocs()

---

63-69: Reduce per‑iteration alloc noise by reusing bytes.Reader

Reusing a bytes.Reader via Reset() trims allocations unrelated to parse cost.

- for b.Loop() {
-   reader := bytes.NewReader(content)
-   _, err := templates.ParseTemplateFromReader(reader, nil, opts)
+ r := bytes.NewReader(nil)
+ for b.Loop() {
+   r.Reset(content)
+   _, err := templates.ParseTemplateFromReader(r, nil, opts)
    if err != nil {
      b.Fatalf("could not parse template from reader: %s", err)
    }
  }

pkg/catalog/loader/loader_bench_test.go (2)

37-39: Warm up once before timing to exclude cold start

Prime caches with an initial ValidateTemplates run outside the timer. Based on learnings.

- b.ResetTimer()
- b.ReportAllocs()
+ // warm-up outside timing
+ if err := store.ValidateTemplates(); err != nil {
+   b.Fatalf("warm-up ValidateTemplates failed: %v", err)
+ }
+ b.ResetTimer()
+ b.ReportAllocs()

---

16-23: Silence logs to avoid benchmark noise

Use a no-op writer via SetWriter to drop all log output during benchmarks:

// test setup
type noopWriter struct{}

func (w *noopWriter) Write(_ []byte, _ levels.Level) {}

// in BenchmarkStoreValidateTemplates
options.Logger = gologger.DefaultLogger
options.Logger.SetWriter(&noopWriter{})

(pkg.go.dev)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f8f89bb and b987b74.

📒 Files selected for processing (5)

• pkg/catalog/loader/loader.go (6 hunks)
• pkg/catalog/loader/loader_bench_test.go (1 hunks)
• pkg/templates/compile.go (6 hunks)
• pkg/templates/compile_bench_test.go (1 hunks)
• pkg/testutils/testutils.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Format Go code using go fmt
Run static analysis with go vet

Files:

• pkg/templates/compile.go
• pkg/catalog/loader/loader.go
• pkg/testutils/testutils.go
• pkg/templates/compile_bench_test.go
• pkg/catalog/loader/loader_bench_test.go

🧠 Learnings (4)

📓 Common learnings

Learnt from: hdm
PR: projectdiscovery/nuclei#6322
File: pkg/templates/compile.go:79-81
Timestamp: 2025-07-16T21:27:14.937Z
Learning: To make the template caching mechanism in pkg/templates/compile.go production-ready, DSLs need to be updated to use runtime options instead of cached variables, rather than restoring the Compile() calls on each request.

📚 Learning: 2025-07-16T21:27:14.937Z

Learnt from: hdm
PR: projectdiscovery/nuclei#6322
File: pkg/templates/compile.go:79-81
Timestamp: 2025-07-16T21:27:14.937Z
Learning: To make the template caching mechanism in pkg/templates/compile.go production-ready, DSLs need to be updated to use runtime options instead of cached variables, rather than restoring the Compile() calls on each request.

Applied to files:

• pkg/templates/compile.go
• pkg/catalog/loader/loader.go
• pkg/templates/compile_bench_test.go

📚 Learning: 2025-07-16T21:27:14.937Z

Learnt from: hdm
PR: projectdiscovery/nuclei#6322
File: pkg/templates/compile.go:79-81
Timestamp: 2025-07-16T21:27:14.937Z
Learning: In pkg/templates/compile.go, the template caching mechanism intentionally skips calling Compile() on copied requests to achieve performance benefits. This is the intended design, not a bug. The current implementation isn't production-ready but represents the desired direction.

Applied to files:

• pkg/templates/compile.go
• pkg/catalog/loader/loader.go
• pkg/templates/compile_bench_test.go

📚 Learning: 2025-07-16T21:28:08.073Z

Learnt from: hdm
PR: projectdiscovery/nuclei#6322
File: pkg/templates/compile.go:59-78
Timestamp: 2025-07-16T21:28:08.073Z
Learning: The shallow copy behavior (`tplCopy := *value`) in pkg/templates/compile.go is intentional design for the template caching mechanism. The partial-copy approach is part of the performance optimization strategy, not a bug requiring deep copying.

Applied to files:

• pkg/templates/compile.go

🧬 Code graph analysis (4)

pkg/templates/compile.go (5)

pkg/protocols/protocols.go (1)

• ExecutorOptions (61-141)

pkg/templates/parser.go (1)

• Parser (21-30)

pkg/templates/templates.go (1)

• Template (35-164)

pkg/utils/utils.go (1)

• ReaderFromPathOrURL (42-56)

pkg/protocols/common/globalmatchers/globalmatchers.go (1)

• Item (22-27)

pkg/catalog/loader/loader.go (3)

pkg/templates/cache.go (1)

• Cache (9-11)

pkg/protocols/protocols.go (1)

• ExecutorOptions (61-141)

pkg/templates/parser.go (1)

• Parser (21-30)

pkg/templates/compile_bench_test.go (1)

pkg/templates/compile.go (1)

• ParseTemplateFromReader (448-516)

pkg/catalog/loader/loader_bench_test.go (5)

pkg/catalog/config/nucleiconfig.go (1)

• DefaultConfig (23-23)

pkg/testutils/testutils.go (1)

• NewMockExecuterOptions (94-116)

pkg/templates/parser.go (1)

• NewParser (32-39)

pkg/loader/workflow/workflow_loader.go (1)

• NewLoader (19-42)

pkg/catalog/loader/loader.go (1)

• NewConfig (101-128)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Tests (macOS-latest)
• GitHub Check: Tests (windows-latest)
• GitHub Check: Tests (ubuntu-latest)

🔇 Additional comments (10)

pkg/testutils/testutils.go (1)

106-112: Good: avoid zero-value metadata when info is nil

Moving TemplateInfo/ID/Path assignments behind the nil check prevents unintended zeroing. LGTM.

pkg/catalog/loader/loader.go (2)

405-434: Nice: cache-first, parse-fallback in validation path

Leveraging parsed cache then falling back to templates.Parse reduces redundant parsing while preserving behavior. LGTM.

---

162-173: OnceValue usage is safe – go.mod targets Go 1.24.2 and ExecutorOptions.Parser isn’t reassigned after Store creation
go.mod specifies Go 1.24.2, and in pkg/catalog no code mutates cfg.ExecutorOptions.Parser after New; sync.OnceValue is supported and poses no lifecycle risk here.

pkg/templates/compile.go (4)

55-107: Reapplying options via shallow copies across request types looks correct

The per-request UpdateOptions with shallow copies is consistent with the intended caching strategy to avoid full recompilation. LGTM.

Based on learnings

---

109-170: Solid: source-based parse with caches + global matchers handling

Good reuse of parsed bytes cache, proper GlobalMatchers short-circuit, and compiled cache store. LGTM.

---

199-273: Good: compiled-template reuse with selective option propagation

Cache hit path reuses compiled template, reapplies engine options, updates requests, and recompiles workflows as needed. Matches the intended design. LGTM.

Based on learnings

---

681-689: Toolchain verified—go.mod specifies Go 1.24.2, which includes sync.OnceFunc. No changes required.

pkg/catalog/loader/loader_bench_test.go (1)

40-42: Confirm b.Loop() availability for target Go version

Same note as other file: ensure go.mod targets a Go version that supports b.Loop(); otherwise replace with for i := 0; i < b.N; i++.

Use the earlier script; apply fallback if needed.

pkg/templates/compile_bench_test.go (2)

19-24: Remove the fallback suggestion—testing.B.Loop was added in Go 1.24.0 and go.mod specifies Go 1.24.2, so no classic for i := 0; i < b.N; i++ loop is needed.

---

15-23: No action required – setup() and executerOpts are defined in pkg/templates/compile_test.go within the same templates_test package, so compile_bench_test.go can reference them without issue.

Likely an incorrect or invalid review comment.

[Mzack9999]

lgtm!

Proposal: what do you think about an additional check that performs cache cleanup when the templates are updated but the cache already populated, so that they are reloaded correctly while still benefitting from caching performance improvements (could be also a follow up task)?

[dwisiswant0]

[...] performs cache cleanup when the templates are updated but the cache already populated, [...]

It looks like the (parsed|compiled)Template caches are populated during the first call to (*Parser).ParseTemplate, which happens after a template update. Both caches stay in mem for the parser’s lifetime and will keep returning the cached copy even if the file on disk changes, unless the update process runs mid-execution or something explicitly invalidates / purges the cache -- but I don't see any reference to (parsed|compiled)Template.Purge() being called anywhere.

Correct me if I’m wrong. cc: @Ice3man543

[dwisiswant0]

For SDK use case, where a single Parser instance remains active while templates on disk may change, then yeah, we can address this by integrate a minimal/quick metadata validation layer into Cache.Has, so the cache stays fast but it better handles disk changes w/o forcing a full reload.

@coderabbitai create an issue for that.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Create a GitHub issue for cache invalidation enhancement

gh issue create \
  --title "Integrate metadata validation in Cache.Has to detect template disk changes" \
  --body "## Problem

In SDK use cases where a single \`Parser\` instance remains active for an extended period, templates on disk may change while the parser continues to use cached copies. Currently, the \`parsedTemplate\` and \`compiledTemplate\` caches are populated during the first call to \`(*Parser).ParseTemplate\` and remain in memory for the parser's lifetime. These caches return cached copies even if the file on disk changes, unless the update process runs mid-execution or something explicitly invalidates/purges the cache.

No calls to \`(parsed|compiled)Template.Purge()\` are currently present in the codebase to handle this scenario.

## Proposed Solution

Integrate a minimal/quick metadata validation layer into \`Cache.Has\` that:
- Stays fast but better handles disk changes
- Avoids forcing a full template reload
- Validates template metadata efficiently to detect changes

This would allow the cache to remain performant while properly invalidating entries when the underlying template files are modified.

## Context

- Related PR: https://github.com/projectdiscovery/nuclei/pull/6504
- Discussion: https://github.com/projectdiscovery/nuclei/pull/6504#discussion_r1888577889
- Requested by: @dwisiswant0" \
  --assignee dwisiswant0

Length of output: 54

---

@dwisiswant0: I've created issue #6515 to track the enhancement of integrating a metadata validation layer into Cache.Has. The issue describes the current behavior where cached templates aren't invalidated when disk files change, and proposes a solution that maintains cache performance while properly detecting template modifications.

---

🧠 Learnings used

Learnt from: hdm
PR: projectdiscovery/nuclei#6322
File: pkg/templates/compile.go:79-81
Timestamp: 2025-07-16T21:27:14.937Z
Learning: To make the template caching mechanism in pkg/templates/compile.go production-ready, DSLs need to be updated to use runtime options instead of cached variables, rather than restoring the Compile() calls on each request.