Skip to content

perf: fix 60% YAMLGeneration regression — skip yaml.Unmarshal for clean compiled workflows#39097

Closed
Copilot wants to merge 3 commits into
mainfrom
copilot/performance-regression-yamlgeneration
Closed

perf: fix 60% YAMLGeneration regression — skip yaml.Unmarshal for clean compiled workflows#39097
Copilot wants to merge 3 commits into
mainfrom
copilot/performance-regression-yamlgeneration

Conversation

Copilot AI commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Every compilation triggered a full yaml.Unmarshal on ~80KB of generated YAML despite schema validation being disabled by default — causing a 60% slowdown vs. historical baseline (~9ms vs ~5.6ms/op).

Root cause

validateTemplateInjection Path B used hasAnyExpressionInRunContent as a cheap pre-scan gate before calling yaml.Unmarshal. Since every compiled workflow contains ${{ vars.GH_AW_DEFAULT_MAX_AI_CREDITS || '1000' }} in a run: block — a compiler-owned, intentionally allowed expression — the gate always fired, making the expensive parse unconditional.

Fix

pkg/workflow/template_injection_validation.go + compiler.go

Replace the pre-scan gate with hasNonAllowedExpressionInRunContent, which filters each found ${{ … }} against allowedRunScriptExpressionRegex and returns true only when a non-allowed expression is present. For correctly compiled workflows this returns false, skipping yaml.Unmarshal entirely.

Remove the now-dead hasAnyExpressionInRunContent and hasExpressionInRunContent functions.

// Before: always fires because vars.GH_AW_DEFAULT_MAX_AI_CREDITS is in every workflow
if hasAnyExpressionInRunContent(yamlContent) {
    // yaml.Unmarshal on ~80KB ...
}

// After: returns false for compiler-owned expressions → no Unmarshal
if hasNonAllowedExpressionInRunContent(yamlContent) {
    // yaml.Unmarshal on ~80KB ...
}

pkg/parser/{include_processor,include_expander,frontmatter_content}.go (ADR-28557)

Replace bufio.NewScanner with strings.Lines/strings.SplitSeq zero-allocation iterators (Go 1.24+) in parser hot paths. Consolidate sequential TrimRight("\n") + TrimRight("\r") calls into a single TrimRight("\n\r").

Result

BenchmarkYAMLGeneration: ~1.55ms/op — down from ~9ms, well below the 5.6ms historical average (~83% improvement).

Copilot AI and others added 2 commits June 13, 2026 16:05
…r allowed expressions

Root cause: validateTemplateInjection Path B (skipValidation=true, the default) used
hasAnyExpressionInRunContent as a pre-scan gate. Every compiled workflow contains
${{ vars.GH_AW_DEFAULT_MAX_AI_CREDITS || '1000' }} in a run: block (an allowed
compiler-owned expression), so the gate always fired and triggered yaml.Unmarshal
on ~80KB of YAML every compilation.

Fix: replace the gate with hasNonAllowedExpressionInRunContent, which filters each
found expression against allowedRunScriptExpressionRegex and returns true only when
a non-allowed expression is present. For correctly compiled workflows this returns
false, skipping yaml.Unmarshal entirely.

Also apply ADR-28557 bufio.Scanner → strings.Lines/SplitSeq zero-allocation changes
in parser hot paths (include_processor, include_expander, frontmatter_content).

Result: BenchmarkYAMLGeneration ~1.55ms/op (was ~9ms, historical avg ~5.57ms)

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
… hot paths

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix performance regression in YAML generation by 60% perf: fix 60% YAMLGeneration regression — skip yaml.Unmarshal for clean compiled workflows Jun 13, 2026
Copilot AI requested a review from pelikhan June 13, 2026 16:10
@pelikhan pelikhan closed this Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants