Skip to content

Reduce ExtractWorkflowNameFromFile overhead by removing deferred close path#34777

Merged
pelikhan merged 4 commits into
mainfrom
copilot/fix-extract-workflow-name-performance
May 25, 2026
Merged

Reduce ExtractWorkflowNameFromFile overhead by removing deferred close path#34777
pelikhan merged 4 commits into
mainfrom
copilot/fix-extract-workflow-name-performance

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 25, 2026

BenchmarkExtractWorkflowNameFromFile regressed materially versus historical runtime (+35.3%). The hotspot is the per-call file handling path in extractWorkflowNameFromFile.

  • What changed

    • Reworked extractWorkflowNameFromFile to use explicit close sequencing instead of deferred close + named return mutation.
    • Preserved error precedence semantics:
      • parse error returns first
      • close error still propagates when parse succeeds
      • title/fallback behavior unchanged
  • Why this matters for the regression

    • Removes defer and closure overhead from a hot helper invoked in benchmarked CLI workflow resolution.
    • Keeps functional behavior stable while tightening the fast path.
  • Code change (core)

    title, err := fastParseTitleFromReader(fd)
    closeErr := fd.Close()
    if err != nil {
        return "", err
    }
    if closeErr != nil {
        return "", fmt.Errorf("failed to close workflow file %s: %w", filePath, closeErr)
    }

Copilot AI and others added 2 commits May 25, 2026 21:41
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix performance regression in ExtractWorkflowNameFromFile Reduce ExtractWorkflowNameFromFile overhead by removing deferred close path May 25, 2026
Copilot AI requested a review from gh-aw-bot May 25, 2026 21:50
@pelikhan pelikhan marked this pull request as ready for review May 25, 2026 21:52
Copilot AI review requested due to automatic review settings May 25, 2026 21:52
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 targets a performance regression in BenchmarkExtractWorkflowNameFromFile by removing the deferred Close() path in extractWorkflowNameFromFile, and it updates the smoke Antigravity workflow to run awf using an explicit JSON config file.

Changes:

  • Refactored extractWorkflowNameFromFile to close the file explicitly rather than via defer, aiming to reduce overhead in a hot path.
  • Updated .github/workflows/smoke-antigravity.lock.yml to generate an awf-config.json and invoke awf with --config.
Show a summary per file
File Description
pkg/cli/workflows.go Refactors workflow-title extraction to avoid deferred close overhead.
.github/workflows/smoke-antigravity.lock.yml Switches awf invocation to use a generated --config JSON file.

Copilot's findings

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment thread pkg/cli/workflows.go Outdated
Comment on lines +413 to +415
if closeErr != nil {
return "", fmt.Errorf("failed to close workflow file %s: %w", filePath, closeErr)
}
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot review and resolve all comments.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 25, 2026

@copilot review and resolve all comments.

Addressed the review feedback and pushed a fix in commit 004f14c.

I updated extractWorkflowNameFromFile so a close failure now returns the computed workflow title together with the close error, preserving prior semantics while keeping the performance optimization.

Copilot AI requested a review from pelikhan May 25, 2026 22:20
@pelikhan pelikhan merged commit db2cb7f into main May 25, 2026
@pelikhan pelikhan deleted the copilot/fix-extract-workflow-name-performance branch May 25, 2026 22:32
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 ExtractWorkflowNameFromFile: +35.3% slower

4 participants