Skip to content

perf: fix algorithmic regressions and expand performance-expert agent#2124

Open
sergio-sisternes-epam wants to merge 5 commits into
mainfrom
sergio-sisternes-epam-perf-audit-and-fixes
Open

perf: fix algorithmic regressions and expand performance-expert agent#2124
sergio-sisternes-epam wants to merge 5 commits into
mainfrom
sergio-sisternes-epam-perf-audit-and-fixes

Conversation

@sergio-sisternes-epam

Copy link
Copy Markdown
Collaborator

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:

  • O(n) linear scan in has_dependency() called per-package during resolution
  • Unconditional full directory scan in HTTP cache on every store() call
  • Triple sequential iteration over git refs in marketplace ref resolution
  • Repeated environment variable parsing across host-classification functions

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Testing

  • Tested locally
  • All existing tests pass (18,204 unit tests, 0 failures)
  • Added tests for new functionality (if applicable)

7 new benchmark tests in tests/benchmarks/test_lookup_and_cache_benchmarks.py
validate 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_size running total with a fast-path skip
    in _enforce_size_cap() -- full os.scandir only runs when tracked size
    exceeds the 100MB cap.
  • marketplace/builder.py: Replaced triple O(n) iteration with a single-pass
    dict index (tags_by_name, refs_by_name, branches_by_name) + O(1) lookups.
  • utils/github_host.py: Extracted repeated os.environ.get().strip().lower().split()
    into _get_ghes_host(), _get_gitlab_single_host(), _get_gitlab_hosts_list().

Benchmarks (commit 2):

  • 7 tests covering all three optimisation areas using the existing @pytest.mark.benchmark
    convention and scaling-guard ratio assertions.

Agent expansion (commit 3):

  • Expanded performance-expert.agent.md with an "Algorithmic performance lens"
    section covering Big O analysis, missing indexes, unconditional expensive ops,
    import costs, redundant computation, and parallelism opportunities.
  • Created references/algorithmic-patterns.md as a lazy-loaded pattern catalogue
    (6 anti-patterns with code examples and a quantification checklist).
  • Broadened the panel activation in apm-review-panel/SKILL.md to fire on
    utils/, marketplace/, compilation/ paths and on diffs introducing loops
    over collections or adding os.scandir/subprocess.run calls.

Spec conformance (OpenAPM v0.1)

  • N/A -- this PR does not change OpenAPM-observable behaviour.

Sergio Sisternes and others added 3 commits July 10, 2026 14:04
…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>
Copilot AI review requested due to automatic review settings July 10, 2026 14:30
@sergio-sisternes-epam sergio-sisternes-epam added the panel-review Trigger the apm-review-panel gh-aw workflow label Jul 10, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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 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 every store().
  • 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.

Comment thread tests/benchmarks/test_lookup_and_cache_benchmarks.py Outdated
Comment thread tests/benchmarks/test_lookup_and_cache_benchmarks.py Outdated
Comment thread tests/benchmarks/test_lookup_and_cache_benchmarks.py Outdated
Comment thread tests/benchmarks/test_lookup_and_cache_benchmarks.py Outdated
Comment thread src/apm_cli/utils/github_host.py
Comment thread src/apm_cli/cache/http_cache.py Outdated
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

panel-review Trigger the apm-review-panel gh-aw workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants