perf: fix algorithmic regressions and expand performance-expert agent#2124
Open
sergio-sisternes-epam wants to merge 5 commits into
Open
perf: fix algorithmic regressions and expand performance-expert agent#2124sergio-sisternes-epam wants to merge 5 commits into
sergio-sisternes-epam wants to merge 5 commits into
Conversation
…ss ref resolution - dependency_graph.py: Replace O(n) has_dependency() linear scan with O(1) set lookup via _repo_url_index maintained on add_node() - http_cache.py: Add tracked size fast-path to _enforce_size_cap() so the full directory scan is skipped when total cache is under the 100MB cap (the common case on every store() call) - marketplace/builder.py: Replace triple sequential iteration over git refs list (tags, full refnames, branches) with a single-pass dict index built once per resolution, reducing O(3n) to O(n) + O(1) lookups - github_host.py: Extract repeated os.environ.get + strip + lower + split chains into shared helpers (_get_ghes_host, _get_gitlab_single_host, _get_gitlab_hosts_list) to eliminate redundant string processing on every host classification call Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Cover the three main optimisation areas: - has_dependency() O(1) index vs O(n) scan scaling - HttpCache size-cap fast-path (linear scaling, no quadratic rescan) - github_host env-var helpers throughput Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ivation Add Big O analysis, missing-index detection, unconditional-expensive-op flagging, import-cost awareness, and scaling-guard recommendations to the performance-expert agent. Create references/algorithmic-patterns.md as a lazy-loaded catalogue of the six core anti-patterns. Broaden the panel activation rules to fire on utils/, marketplace/, compilation/ paths and on diffs that introduce loops over collections or repeated I/O calls. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets install/resolve hot paths by removing a few algorithmic inefficiencies (O(n) scans and repeated parsing) and adds benchmark coverage plus agent/panel guidance to help prevent similar regressions.
Changes:
DependencyTree.has_dependency()now uses an internal set index for O(1) lookups.HttpCache._enforce_size_cap()adds a tracked-size fast path to avoid unconditional directory scans on everystore().- Marketplace explicit ref resolution builds single-pass lookup dictionaries instead of iterating git refs multiple times; plus new benchmarks and updated review-panel/perf-agent guidance.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/benchmarks/test_lookup_and_cache_benchmarks.py | Adds benchmark-style scaling guards for the optimized lookup/cache/env-parsing paths. |
| src/apm_cli/utils/github_host.py | Extracts env parsing into helpers to reduce repeated normalization work. |
| src/apm_cli/marketplace/builder.py | Replaces triple-pass remote-ref scans with single-pass indexing for O(1) lookups. |
| src/apm_cli/deps/dependency_graph.py | Adds _repo_url_index to make has_dependency() constant-time. |
| src/apm_cli/cache/http_cache.py | Adds _tracked_size and a fast-path skip in _enforce_size_cap(). |
| .apm/agents/references/algorithmic-patterns.md | New reference catalog of algorithmic performance anti-patterns and scaling-guard guidance. |
| .apm/agents/performance-expert.agent.md | Expands the perf agent with an explicit Big-O / algorithmic performance review lens. |
| .agents/skills/apm-review-panel/SKILL.md | Broadens perf-expert routing triggers and adds algorithmic-regression heuristics for activation. |
…strings - Use owner/repo identity format (not full HTTPS URLs) in benchmark lookups - Fix comment mismatch: slow-path threshold is 50ms, not 5ms - Fix comment mismatch: negative-lookup threshold is 100us, not 1us - Drop false 'cached per process' claim from _get_ghes_host() docstring - Clarify _tracked_size estimate is intentionally an upper bound, not exact Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix four high-impact algorithmic performance issues found during a full-codebase
audit, add scaling-guard benchmarks, and expand the performance-expert agent
with a Big O analysis lens so the review panel catches future regressions.
The audit identified 15 issues across the codebase; the top four by impact were:
has_dependency()called per-package during resolutionstore()callType of change
Testing
7 new benchmark tests in
tests/benchmarks/test_lookup_and_cache_benchmarks.pyvalidate the optimised paths using scaling-guard assertions (ratio-based
regression detection).
Approach
Performance fixes (commit 1):
dependency_graph.py: Added_repo_url_index: set[str]to DependencyTree;has_dependency()is now O(1) set lookup instead of O(n) linear scan.http_cache.py: Added_tracked_sizerunning total with a fast-path skipin
_enforce_size_cap()-- fullos.scandironly runs when tracked sizeexceeds the 100MB cap.
marketplace/builder.py: Replaced triple O(n) iteration with a single-passdict index (
tags_by_name,refs_by_name,branches_by_name) + O(1) lookups.utils/github_host.py: Extracted repeatedos.environ.get().strip().lower().split()into
_get_ghes_host(),_get_gitlab_single_host(),_get_gitlab_hosts_list().Benchmarks (commit 2):
@pytest.mark.benchmarkconvention and scaling-guard ratio assertions.
Agent expansion (commit 3):
performance-expert.agent.mdwith an "Algorithmic performance lens"section covering Big O analysis, missing indexes, unconditional expensive ops,
import costs, redundant computation, and parallelism opportunities.
references/algorithmic-patterns.mdas a lazy-loaded pattern catalogue(6 anti-patterns with code examples and a quantification checklist).
apm-review-panel/SKILL.mdto fire onutils/,marketplace/,compilation/paths and on diffs introducing loopsover collections or adding
os.scandir/subprocess.runcalls.Spec conformance (OpenAPM v0.1)