perf: eliminate bufio.Scanner allocations and redundant file read in parse/YAML hot paths (#28557)

Author: Copilot

docs/adr/28557-zero-allocation-fast-path-for-parser-hot-paths.md

@@ -0,0 +1,82 @@
+# ADR-28557: Zero-Allocation Fast-Path Strategy for Parser Hot Paths
+
+**Date**: 2026-04-26
+**Status**: Draft
+**Deciders**: pelikhan (copilot-swe-agent)
+
+---
+
+## Part 1 — Narrative (Human-Friendly)
+
+### Context
+
+Benchmark profiling revealed a 12–13.5% performance regression in `BenchmarkYAMLGeneration` and `BenchmarkParseWorkflow`. The root causes were: (a) `bufio.Scanner` being allocated on every parse invocation — 6+ instances per call, accounting for approximately 20% of total allocations — and (b) a redundant `os.ReadFile` call in `generateYAML` that re-read a file whose content had already been parsed and held in memory. Both issues occur in the hot path executed on every workflow compilation. The `pkg/parser` package is called on each compile, making allocation pressure and redundant I/O directly proportional to throughput.
+
+### Decision
+
+We will replace `bufio.Scanner` in parser hot paths with Go 1.24 zero-allocation string iterators (`strings.Lines` and `strings.SplitSeq`), add fast-path pre-checks (`hasIncludeDirectives`) that return early before any scanner or buffer is allocated when no directives are present, and cache the raw markdown body in `WorkflowData.RawMarkdown` so that `generateYAML` can compute the frontmatter hash from pre-parsed data rather than re-reading the file from disk. The fast-path pre-check strategy is the primary driver: most workflow files contain no `@include`/`@import`/`{{#import` directives, so the common case can be handled without any scanner allocation.
+
+### Alternatives Considered
+
+#### Alternative 1: `sync.Pool` for `bufio.Scanner` reuse
+
+A pool of pre-allocated scanners could amortize the per-call allocation cost. This was considered because it requires no Go version change and keeps the existing scanning logic intact. It was rejected because it adds pool lifecycle complexity (reset state between uses, handle concurrent access), whereas Go 1.24 iterators are simpler, have zero ongoing cost, and the fast-path pre-check eliminates the allocation entirely for the common case rather than merely amortizing it.
+
+#### Alternative 2: Pre-allocated fixed-size byte buffer
+
+Allocating a reusable `[]byte` buffer at startup and passing it to a custom scanner would reduce heap pressure. This was not chosen because it requires manual buffer sizing, risks stack-overflow or truncation for unexpectedly large content, and still allocates the scanner struct itself; the `strings.Lines` iterator avoids all of this by operating directly on the string without a separate buffer.
+
+#### Alternative 3: Optimize only the redundant file read (partial fix)
+
+Addressing only the `generateYAML` re-read without changing the scanner strategy would recover 3–6% of the regression. This was rejected as a standalone approach because profiling showed the scanner allocations were the dominant cost (~20% of total allocations); a partial fix would leave the larger problem unaddressed.
+
+### Consequences
+
+#### Positive
+- `BenchmarkParseWorkflow`: 11% throughput improvement, 39% memory reduction per operation; `bufio` allocations eliminated.
+- `BenchmarkYAMLGeneration`: 3% throughput improvement, 6% memory reduction per operation; redundant disk read eliminated.
+- The fast-path pre-check (`hasIncludeDirectives`) is a string-contains scan — O(n) but cache-friendly — and pays for itself whenever directives are absent (the common case).
+- `RawMarkdown` caching in `WorkflowData` makes the data flow more explicit: content read once during parsing flows through to YAML generation without additional I/O.
+
+#### Negative
+- `strings.Lines` and `strings.SplitSeq` require Go 1.24, setting a hard minimum runtime version for this package.
+- `generateYAML` now has two code paths (fast path using `RawMarkdown`, fallback using disk read), adding branching complexity that must be kept in sync when the hashing logic changes.
+- The `hasIncludeDirectives` pre-check can produce false positives (content containing `@include` in a comment but not as a directive), causing unnecessary scanner allocation in those edge cases; however, the check is conservative and correct — it never produces false negatives.
+
+#### Neutral
+- The `RawMarkdown` field is added to `WorkflowData`, a central struct; callers that construct `WorkflowData` externally without setting `RawMarkdown` automatically fall back to the disk-read path via the explicit `else` branch, preserving backward compatibility.
+- H1 header scanning in `ExtractWorkflowNameFromMarkdownBody` is now bounded to the first 64 lines (previously unbounded), which is a behavior change for pathologically large files but is semantically correct because H1 headers appear at the top of Markdown documents.
+
+---
+
+## Part 2 — Normative Specification (RFC 2119)
+
+> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119).
+
+### Fast-Path Pre-Checks
+
+1. Functions that iterate over content lines **MUST** call `hasIncludeDirectives(content)` before allocating any `bufio.Scanner` or buffer when the only reason to iterate is to process include/import directives.
+2. `hasIncludeDirectives` **MUST** return `true` if and only if the content string contains at least one of the substrings `@include`, `@import`, or `{{#import`.
+3. Functions that return early via the fast path **MUST** preserve the behavioral contract of the full code path, including trailing-newline normalization for content mode and returning `"{}"` for tool-extraction mode.
+4. Fast-path pre-checks **MUST NOT** be added to functions whose iteration purpose is not exclusively include/import directive processing.
+
+### String Iteration in Hot Paths
+
+1. New line-scanning code in `pkg/parser` **MUST** use `strings.Lines` or `strings.SplitSeq` (Go 1.24) instead of `bufio.NewScanner(strings.NewReader(...))` when the input is an in-memory string.
+2. Implementations **MUST NOT** wrap an in-memory string in `strings.NewReader` solely to pass it to `bufio.NewScanner`; use the zero-allocation iterators instead.
+3. When an upper bound on lines to scan is required (e.g., searching for an H1 header), the iteration **MUST** break after the configured maximum (currently 64 lines for H1 header extraction) to bound worst-case cost.
+
+### `WorkflowData.RawMarkdown` Caching
+
+1. Code that constructs `WorkflowData` from a parsed result **MUST** populate `RawMarkdown` with the raw markdown body (before include expansion) when that content is available in memory at construction time.
+2. `generateYAML` **MUST** use `ComputeFrontmatterHashFromParsedContent` when `WorkflowData.RawMarkdown` is non-empty, and **MUST** fall back to `ComputeFrontmatterHashFromFileWithParsedFrontmatter` (disk read) when `RawMarkdown` is empty.
+3. The fallback path **SHOULD** log a debug message identifying the file path when it reads from disk, to aid future performance investigations.
+4. Callers that construct `WorkflowData` externally **MAY** leave `RawMarkdown` empty; the fallback path **MUST** remain functional in this case.
+
+### Conformance
+
+An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24954772505) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/actionpins/data/action_pins.json

@@ -158,6 +158,11 @@
       "version": "v2.11",
       "sha": "1cb3cd8a008d80c9fa129c0f0823d69584905f5b"
     },
+    "microsoft/apm-action@v1.4.2": {
+      "repo": "microsoft/apm-action",
+      "version": "v1.4.2",
+      "sha": "9fe9337ef58b5e620e0113071ceb47a6a8a232f7"
+    },
     "oven-sh/setup-bun@v2.2.0": {
       "repo": "oven-sh/setup-bun",
       "version": "v2.2.0",
@@ -172,14 +177,14 @@
       "repo": "super-linter/super-linter",
       "version": "v8.6.0",
       "sha": "9e863354e3ff62e0727d37183162c4a88873df41"
-    },
-    "microsoft/apm-action@v1.4.2": {
-      "repo": "microsoft/apm-action",
-      "version": "v1.4.2",
-      "sha": "9fe9337ef58b5e620e0113071ceb47a6a8a232f7"
     }
   },
   "containers": {
+    "alpine:latest": {
+      "image": "alpine:latest",
+      "digest": "sha256:5b10f432ef3da1b8d4c7eb6c487f2f5a8f096bc91145e68878dd4a5019afde11",
+      "pinned_image": "alpine:latest@sha256:5b10f432ef3da1b8d4c7eb6c487f2f5a8f096bc91145e68878dd4a5019afde11"
+    },
     "docker.io/mcp/brave-search": {
       "image": "docker.io/mcp/brave-search",
       "digest": "sha256:ca96b8acb27d8cf601a8faef86a084602cffa41d8cb18caa1e29ba4d16989d22",
@@ -195,6 +200,11 @@
       "digest": "sha256:9161f2415a3306a344aca34dd671ee69f122317e0a512e66dc64c94b9c508682",
       "pinned_image": "ghcr.io/github/gh-aw-firewall/agent:0.25.20@sha256:9161f2415a3306a344aca34dd671ee69f122317e0a512e66dc64c94b9c508682"
     },
+    "ghcr.io/github/gh-aw-firewall/agent:0.25.28": {
+      "image": "ghcr.io/github/gh-aw-firewall/agent:0.25.28",
+      "digest": "sha256:a8834e285807654bf680154faa710d43fe4365a0868142f5c20e48c85e137a7a",
+      "pinned_image": "ghcr.io/github/gh-aw-firewall/agent:0.25.28@sha256:a8834e285807654bf680154faa710d43fe4365a0868142f5c20e48c85e137a7a"
+    },
     "ghcr.io/github/gh-aw-firewall/api-proxy:0.25.18": {
       "image": "ghcr.io/github/gh-aw-firewall/api-proxy:0.25.18",
       "digest": "sha256:d16a40a3ca6e989896d0cef9f31b9412bb1fcc8755bafcafb95012ae1078539b",
@@ -205,6 +215,16 @@
       "digest": "sha256:6971639e381e82e45134bcd333181f456df3a52cd6f818a3e3d6de068ff91519",
       "pinned_image": "ghcr.io/github/gh-aw-firewall/api-proxy:0.25.20@sha256:6971639e381e82e45134bcd333181f456df3a52cd6f818a3e3d6de068ff91519"
     },
+    "ghcr.io/github/gh-aw-firewall/api-proxy:0.25.28": {
+      "image": "ghcr.io/github/gh-aw-firewall/api-proxy:0.25.28",
+      "digest": "sha256:93290f2393752252911bd7c39a047f776c0b53063575e7bde4e304962a9a61cb",
+      "pinned_image": "ghcr.io/github/gh-aw-firewall/api-proxy:0.25.28@sha256:93290f2393752252911bd7c39a047f776c0b53063575e7bde4e304962a9a61cb"
+    },
+    "ghcr.io/github/gh-aw-firewall/cli-proxy:0.25.28": {
+      "image": "ghcr.io/github/gh-aw-firewall/cli-proxy:0.25.28",
+      "digest": "sha256:fdf310e4678ce58d248c466b89399e9680a3003038fd19322c388559016aaac7",
+      "pinned_image": "ghcr.io/github/gh-aw-firewall/cli-proxy:0.25.28@sha256:fdf310e4678ce58d248c466b89399e9680a3003038fd19322c388559016aaac7"
+    },
     "ghcr.io/github/gh-aw-firewall/squid:0.25.18": {
       "image": "ghcr.io/github/gh-aw-firewall/squid:0.25.18",
       "digest": "sha256:eb102afcfbae26ffcec016adebb74d3be7b0a5bf376ba306599cdf3effbe288e",
@@ -215,16 +235,31 @@
       "digest": "sha256:5411d903f73ee597e6a084971c2adef3eb0bd405910df3ed7bf5e3d6bd58a236",
       "pinned_image": "ghcr.io/github/gh-aw-firewall/squid:0.25.20@sha256:5411d903f73ee597e6a084971c2adef3eb0bd405910df3ed7bf5e3d6bd58a236"
     },
+    "ghcr.io/github/gh-aw-firewall/squid:0.25.28": {
+      "image": "ghcr.io/github/gh-aw-firewall/squid:0.25.28",
+      "digest": "sha256:844c18280f82cd1b06345eb2f4e91966b34185bfc51c9f237c3e022e848fb474",
+      "pinned_image": "ghcr.io/github/gh-aw-firewall/squid:0.25.28@sha256:844c18280f82cd1b06345eb2f4e91966b34185bfc51c9f237c3e022e848fb474"
+    },
     "ghcr.io/github/gh-aw-mcpg:v0.2.19": {
       "image": "ghcr.io/github/gh-aw-mcpg:v0.2.19",
       "digest": "sha256:44d4d8de7e6c37aaea484eba489940c52df6a0b54078ddcbc9327592d5b3c3dd",
       "pinned_image": "ghcr.io/github/gh-aw-mcpg:v0.2.19@sha256:44d4d8de7e6c37aaea484eba489940c52df6a0b54078ddcbc9327592d5b3c3dd"
     },
+    "ghcr.io/github/gh-aw-mcpg:v0.2.30": {
+      "image": "ghcr.io/github/gh-aw-mcpg:v0.2.30",
+      "digest": "sha256:e950e6d39f003862d33bfb8d4eb93e242d919cf6ca874b90728e5e0ea7434c6f",
+      "pinned_image": "ghcr.io/github/gh-aw-mcpg:v0.2.30@sha256:e950e6d39f003862d33bfb8d4eb93e242d919cf6ca874b90728e5e0ea7434c6f"
+    },
     "ghcr.io/github/github-mcp-server:v0.32.0": {
       "image": "ghcr.io/github/github-mcp-server:v0.32.0",
       "digest": "sha256:2763823c63bcca718ce53850a1d7fcf2f501ec84028394f1b63ce7e9f4f9be28",
       "pinned_image": "ghcr.io/github/github-mcp-server:v0.32.0@sha256:2763823c63bcca718ce53850a1d7fcf2f501ec84028394f1b63ce7e9f4f9be28"
     },
+    "ghcr.io/github/github-mcp-server:v1.0.0": {
+      "image": "ghcr.io/github/github-mcp-server:v1.0.0",
+      "digest": "sha256:d2550953f8050bc5a1c8f80d1678766f66f60bbfbcd953fdeaf661fe4269bd95",
+      "pinned_image": "ghcr.io/github/github-mcp-server:v1.0.0@sha256:d2550953f8050bc5a1c8f80d1678766f66f60bbfbcd953fdeaf661fe4269bd95"
+    },
     "ghcr.io/github/serena-mcp-server:latest": {
       "image": "ghcr.io/github/serena-mcp-server:latest",
       "digest": "sha256:bf343399e3725c45528f531a230f3a04521d4cdef29f9a5af6282ff0d3c393c5",
@@ -279,41 +314,6 @@
       "image": "semgrep/semgrep:latest",
       "digest": "sha256:17d89ddd91a7729bbd5de09402f7f79a70204289e2a94635086e9db532a495f2",
       "pinned_image": "semgrep/semgrep:latest@sha256:17d89ddd91a7729bbd5de09402f7f79a70204289e2a94635086e9db532a495f2"
-    },
-    "ghcr.io/github/gh-aw-firewall/agent:0.25.28": {
-      "image": "ghcr.io/github/gh-aw-firewall/agent:0.25.28",
-      "digest": "sha256:a8834e285807654bf680154faa710d43fe4365a0868142f5c20e48c85e137a7a",
-      "pinned_image": "ghcr.io/github/gh-aw-firewall/agent:0.25.28@sha256:a8834e285807654bf680154faa710d43fe4365a0868142f5c20e48c85e137a7a"
-    },
-    "ghcr.io/github/gh-aw-firewall/api-proxy:0.25.28": {
-      "image": "ghcr.io/github/gh-aw-firewall/api-proxy:0.25.28",
-      "digest": "sha256:93290f2393752252911bd7c39a047f776c0b53063575e7bde4e304962a9a61cb",
-      "pinned_image": "ghcr.io/github/gh-aw-firewall/api-proxy:0.25.28@sha256:93290f2393752252911bd7c39a047f776c0b53063575e7bde4e304962a9a61cb"
-    },
-    "ghcr.io/github/gh-aw-firewall/squid:0.25.28": {
-      "image": "ghcr.io/github/gh-aw-firewall/squid:0.25.28",
-      "digest": "sha256:844c18280f82cd1b06345eb2f4e91966b34185bfc51c9f237c3e022e848fb474",
-      "pinned_image": "ghcr.io/github/gh-aw-firewall/squid:0.25.28@sha256:844c18280f82cd1b06345eb2f4e91966b34185bfc51c9f237c3e022e848fb474"
-    },
-    "ghcr.io/github/gh-aw-firewall/cli-proxy:0.25.28": {
-      "image": "ghcr.io/github/gh-aw-firewall/cli-proxy:0.25.28",
-      "digest": "sha256:fdf310e4678ce58d248c466b89399e9680a3003038fd19322c388559016aaac7",
-      "pinned_image": "ghcr.io/github/gh-aw-firewall/cli-proxy:0.25.28@sha256:fdf310e4678ce58d248c466b89399e9680a3003038fd19322c388559016aaac7"
-    },
-    "ghcr.io/github/gh-aw-mcpg:v0.2.30": {
-      "image": "ghcr.io/github/gh-aw-mcpg:v0.2.30",
-      "digest": "sha256:e950e6d39f003862d33bfb8d4eb93e242d919cf6ca874b90728e5e0ea7434c6f",
-      "pinned_image": "ghcr.io/github/gh-aw-mcpg:v0.2.30@sha256:e950e6d39f003862d33bfb8d4eb93e242d919cf6ca874b90728e5e0ea7434c6f"
-    },
-    "ghcr.io/github/github-mcp-server:v1.0.0": {
-      "image": "ghcr.io/github/github-mcp-server:v1.0.0",
-      "digest": "sha256:d2550953f8050bc5a1c8f80d1678766f66f60bbfbcd953fdeaf661fe4269bd95",
-      "pinned_image": "ghcr.io/github/github-mcp-server:v1.0.0@sha256:d2550953f8050bc5a1c8f80d1678766f66f60bbfbcd953fdeaf661fe4269bd95"
-    },
-    "alpine:latest": {
-      "image": "alpine:latest",
-      "digest": "sha256:5b10f432ef3da1b8d4c7eb6c487f2f5a8f096bc91145e68878dd4a5019afde11",
-      "pinned_image": "alpine:latest@sha256:5b10f432ef3da1b8d4c7eb6c487f2f5a8f096bc91145e68878dd4a5019afde11"
     }
   }
 }

pkg/gitutil/spec_test.go

@@ -294,7 +294,7 @@ func TestSpec_PublicAPI_ReadFileFromHEADWithRoot(t *testing.T) {
 	}
 
 	t.Run("reads known file from HEAD without error", func(t *testing.T) {
-		content, err := ReadFileFromHEADWithRoot("go.mod", root)
+		content, err := ReadFileFromHEADWithRoot(filepath.Join(root, "go.mod"), root)
 		assert.NoError(t, err, "ReadFileFromHEADWithRoot should read go.mod without error")
 		assert.NotEmpty(t, content, "content of go.mod should not be empty")
 	})

pkg/parser/frontmatter_content.go

@@ -207,6 +207,24 @@ func ExtractMarkdownContent(content string) (string, error) {
 	return result.Markdown, nil
 }
 
+// findH1WorkflowName scans at most the first 64 lines of markdownBody for an H1 header
+// and returns the trimmed title. Returns "" if no H1 is found within those lines.
+func findH1WorkflowName(markdownBody string) string {
+	const maxLines = 64
+	lineCount := 0
+	for line := range strings.Lines(markdownBody) {
+		lineCount++
+		if lineCount > maxLines {
+			break
+		}
+		trimmed := strings.TrimSpace(line)
+		if strings.HasPrefix(trimmed, "# ") {
+			return strings.TrimSpace(trimmed[2:])
+		}
+	}
+	return ""
+}
+
 // ExtractWorkflowNameFromMarkdownBody extracts the workflow name from an already-extracted
 // markdown body (i.e. the content after the frontmatter has been stripped). This is more
 // efficient than ExtractWorkflowNameFromMarkdown or ExtractWorkflowNameFromContent because it
@@ -215,14 +233,9 @@ func ExtractMarkdownContent(content string) (string, error) {
 func ExtractWorkflowNameFromMarkdownBody(markdownBody string, virtualPath string) (string, error) {
 	log.Printf("Extracting workflow name from markdown body: virtualPath=%s, size=%d bytes", virtualPath, len(markdownBody))
 
-	scanner := bufio.NewScanner(strings.NewReader(markdownBody))
-	for scanner.Scan() {
-		line := strings.TrimSpace(scanner.Text())
-		if strings.HasPrefix(line, "# ") {
-			workflowName := strings.TrimSpace(line[2:])
-			log.Printf("Found workflow name from H1 header: %s", workflowName)
-			return workflowName, nil
-		}
+	if name := findH1WorkflowName(markdownBody); name != "" {
+		log.Printf("Found workflow name from H1 header: %s", name)
+		return name, nil
 	}
 
 	defaultName := generateDefaultWorkflowName(virtualPath)
@@ -241,14 +254,9 @@ func ExtractWorkflowNameFromContent(content string, virtualPath string) (string,
 		return "", err
 	}
 
-	scanner := bufio.NewScanner(strings.NewReader(markdownContent))
-	for scanner.Scan() {
-		line := strings.TrimSpace(scanner.Text())
-		if strings.HasPrefix(line, "# ") {
-			workflowName := strings.TrimSpace(line[2:])
-			log.Printf("Found workflow name from H1 header: %s", workflowName)
-			return workflowName, nil
-		}
+	if name := findH1WorkflowName(markdownContent); name != "" {
+		log.Printf("Found workflow name from H1 header: %s", name)
+		return name, nil
 	}
 
 	defaultName := generateDefaultWorkflowName(virtualPath)

pkg/parser/frontmatter_hash.go

@@ -122,6 +122,28 @@ func marshalSorted(data any) string {
 	}
 }
 
+// ComputeFrontmatterHashFromParsedContent computes the frontmatter hash from already-parsed
+// workflow data, avoiding a redundant file read when content has already been loaded.
+// frontmatterText is the raw text between the --- delimiters (e.g. WorkflowData.FrontmatterYAML).
+// markdownBody is the raw markdown body before include expansion (e.g. WorkflowData.RawMarkdown).
+// parsedFrontmatter is used to detect the inlined-imports flag.
+// baseDir is the directory containing the workflow file, used for resolving imports.
+func ComputeFrontmatterHashFromParsedContent(frontmatterText, markdownBody string, parsedFrontmatter map[string]any, baseDir string, cache *ImportCache, fileReader FileReader) (string, error) {
+	frontmatterHashLog.Printf("Computing hash from parsed content (baseDir=%s)", baseDir)
+
+	inlinedImports := parseBoolFromFrontmatter(parsedFrontmatter, "inlined-imports")
+
+	var relevantExpressions []string
+	var fullBody string
+	if inlinedImports {
+		fullBody = normalizeFrontmatterText(markdownBody)
+	} else {
+		relevantExpressions = extractRelevantTemplateExpressions(markdownBody)
+	}
+
+	return computeFrontmatterHashTextBasedWithReader(frontmatterText, fullBody, baseDir, cache, relevantExpressions, fileReader)
+}
+
 // ComputeFrontmatterHashFromFile computes the frontmatter hash for a workflow file
 // using text-based approach (no YAML parsing) to match JavaScript implementation
 func ComputeFrontmatterHashFromFile(filePath string, cache *ImportCache) (string, error) {