Skip to content

Optimize include extraction hot path in findIncludesInContent#35004

Merged
pelikhan merged 5 commits into
mainfrom
copilot/fix-performance-regression-benchmarkfindincludesin
May 26, 2026
Merged

Optimize include extraction hot path in findIncludesInContent#35004
pelikhan merged 5 commits into
mainfrom
copilot/fix-performance-regression-benchmarkfindincludesin

Conversation

Copilot AI commented May 26, 2026

Copy link
Copy Markdown
Contributor

BenchmarkFindIncludesInContent regressed by ~45.8% versus the historical run, pointing to extra work in include-detection. This change tightens the scan path by removing redundant traversal while preserving include parsing behavior.

  • Regression focus

    • The benchmark target (findIncludesInContent) was doing an additional full-content pre-scan before line parsing.
    • On include-heavy content (the benchmark case), this introduced avoidable overhead.
  • Code changes

    • Removed hasDirectiveMarker and its call site from findIncludesInContent.
    • Kept existing line-by-line parsing (parseIncludePath) unchanged, so directive semantics remain intact.
  • Behavioral impact

    • Include detection logic is unchanged functionally.
    • Execution now performs a single pass for extraction instead of pre-scan + extraction.
// Before
if !hasDirectiveMarker(content) {
    return []string{}, nil
}
for each line {
    parseIncludePath(line)
}

// After
for each line {
    parseIncludePath(line)
}

pr-sous-chef: branch update attempted in https://github.com/github/gh-aw/actions/runs/26470389940

Generated by 👨‍🍳 PR Sous Chef · gpt54 36.7M ·

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix performance regression in BenchmarkFindIncludesInContent Optimize include extraction hot path in findIncludesInContent May 26, 2026
Copilot AI requested a review from gh-aw-bot May 26, 2026 17:33
@pelikhan pelikhan marked this pull request as ready for review May 26, 2026 18:26
Copilot AI review requested due to automatic review settings May 26, 2026 18:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 benchmark regression by removing the extra full-content pre-scan in findIncludesInContent, so include extraction is performed via a single line-by-line pass while keeping parseIncludePath semantics intact.

Changes:

  • Removed hasDirectiveMarker and the early-return “no directives” fast path from findIncludesInContent.
  • Kept the existing manual line scan and parseIncludePath logic for directive parsing.
Show a summary per file
File Description
pkg/cli/remove_command.go Removes the directive-marker pre-scan helper and always performs the existing line-based include extraction pass.

Copilot's findings

Tip

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

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

Comment thread pkg/cli/remove_command.go
return []string{}, nil
}

var includes []string
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot review all comments

@github-actions

Copy link
Copy Markdown
Contributor

\n\n@copilot please confirm the benchmark improvement and add a brief note addressing the existing review comment.

Generated by 👨‍🍳 PR Sous Chef · gpt54 26.4M ·

Copilot AI and others added 2 commits May 26, 2026 18:44
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

@copilot review all comments

Addressed. I reviewed the feedback and pushed db4d56b, which restores a non-nil empty-slice return for findIncludesInContent and adds a focused regression test for empty content while keeping the single-pass performance optimization.

Copilot AI requested a review from pelikhan May 26, 2026 18:47
@github-actions github-actions Bot mentioned this pull request May 26, 2026
@pelikhan pelikhan merged commit 3ae30af into main May 26, 2026
@pelikhan pelikhan deleted the copilot/fix-performance-regression-benchmarkfindincludesin branch May 26, 2026 20:47
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 BenchmarkFindIncludesInContent: +45.8% slower

4 participants