Skip to content

Reduce BenchmarkValidation latency by caching permission-scope validation#35076

Merged
pelikhan merged 2 commits into
mainfrom
copilot/fix-benchmark-validation-performance
May 27, 2026
Merged

Reduce BenchmarkValidation latency by caching permission-scope validation#35076
pelikhan merged 2 commits into
mainfrom
copilot/fix-benchmark-validation-performance

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 26, 2026

BenchmarkValidation regressed significantly in the validation pipeline. The hot path was repeatedly re-validating permission scope names from YAML on each validation pass, adding avoidable parse and allocation overhead.

  • Root cause in validation hot path

    • validatePermissions always called ValidatePermissionScopeNames(workflowData.Permissions), even when WorkflowData had already been normalized and cached in applyDefaults.
  • Caching added to WorkflowData

    • Introduced cached fields to store permission-scope validation result state:
      • CachedPermissionScopeNamesErr
      • CachedPermissionScopeNamesSet
    • Populated once in applyDefaults, alongside other existing compiler caches.
  • Validator now reuses cached result

    • validatePermissions now:
      • uses cached scope-validation result when present
      • falls back to direct validation only when cache is not initialized (e.g., direct test construction paths)
  • Targeted regression guard

    • Added focused unit coverage to ensure validatePermissions consumes cached permission-scope validation results when set.
var scopeValidationErr error
if workflowData.CachedPermissionScopeNamesSet {
	scopeValidationErr = workflowData.CachedPermissionScopeNamesErr
} else {
	scopeValidationErr = ValidatePermissionScopeNames(workflowData.Permissions)
}
if scopeValidationErr != nil {
	return nil, formatCompilerError(markdownPath, "error", scopeValidationErr.Error(), scopeValidationErr)
}

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix performance regression in BenchmarkValidation Reduce BenchmarkValidation latency by caching permission-scope validation May 26, 2026
Copilot AI requested a review from gh-aw-bot May 26, 2026 23:49
@pelikhan pelikhan marked this pull request as ready for review May 27, 2026 00:05
Copilot AI review requested due to automatic review settings May 27, 2026 00:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a performance regression in the workflow validation hot path by caching the result of permission-scope name validation on WorkflowData, so repeated validation passes don’t re-parse permissions YAML.

Changes:

  • Cache ValidatePermissionScopeNames(...) result in Compiler.applyDefaults via WorkflowData.CachedPermissionScopeNamesErr + CachedPermissionScopeNamesSet.
  • Update Compiler.validatePermissions to reuse the cached permission-scope validation result when available, with a fallback for direct WorkflowData construction paths.
  • Add a unit test ensuring the cached validation result is honored.
Show a summary per file
File Description
pkg/workflow/tools.go Populates the cached permission-scope validation result during applyDefaults.
pkg/workflow/permissions_compiler_validator.go Reuses cached permission-scope validation result in validatePermissions when set.
pkg/workflow/compiler_validators_test.go Adds a targeted test asserting validatePermissions uses the cached result.
pkg/workflow/compiler_types.go Adds cached permission-scope validation fields to WorkflowData.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 4/4 changed files
  • Comments generated: 0

@pelikhan pelikhan merged commit 85e7a26 into main May 27, 2026
41 checks passed
@pelikhan pelikhan deleted the copilot/fix-benchmark-validation-performance branch May 27, 2026 00:10
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.

[performance] Regression in BenchmarkValidation: +91.0% slower

4 participants