GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — perf(compile): remove redundant work from instruction placement by sekosen · Pull Request #3379 · microsoft/apm

Conversation

Sergei Kozyrev (sekosen) commented Oct 24, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Description

On large projects with overlapping scoped applyTo patterns, apm compile spends most of its time in ContextOptimizer repeating the same work. This PR removes that repeated work. Results are unchanged.

File project-relative paths are cached instead of resolved for every pattern.
Directories are resolved once per optimization call instead of on every coverage check.
The depth mean in _calculate_distribution_score is computed once instead of once per directory.
The candidate coverage check stops at the first uncovered directory.

Both caches are in memory and are cleared each time the project structure is analyzed. Matching, CompileInventory use, symlinks, exclusions and hidden roots are unchanged.

Benchmark

Run with sekosen/apm-placement-benchmark: apm compile --dry-run --no-links --target claude, fastest of 3 runs, peak RSS. Before is main at 29200432.

project	directories	before	after	speedup	before MB	after MB
`proj_small`	~6	0.25s	0.24s	1.0x	66	66
`proj_1000`	~1000	2.69s	0.58s	4.7x	75	74
`proj_2000`	~2000	7.95s	0.91s	8.8x	85	83
`proj_10000`	~10000	153.11s	3.70s	41.3x	165	153

Issue and approved scope

Issue: #2969

Human scope-approval comment: #2969 (comment)

Replaces #2878 with a new history and without the excluded symlink behavior change.

Fixes #2969

Commits

test(compile): pin placement behavior before the perf fix: unit parity tests, pass on main.
test(compile): cover placement and file inclusion through apm compile: end-to-end test through the real CLI, passes on main.
perf(compile): remove redundant work from placement: the fix. Existing tests pass unmodified.
test(compile): catch regressions of the placement perf fix: regression tests for each optimization. Each was checked to fail when the code it guards is broken.

"Done when" coverage

Criterion	Coverage
Repeated mean computation	`test_depth_mean_computed_once`
Unnecessary path resolution	`test_file_resolved_once_across_patterns`, `test_directory_resolved_once_per_call`
Failure to short-circuit	`test_candidate_check_stops_at_first_uncovered_directory`
Stale caches between calls	`test_rel_path_cache_entry_does_not_survive`, `test_resolved_dir_cache_entry_does_not_survive`
Parity: files and in-project/external/dangling symlinks	`test_context_optimizer_symlinked_files.py`, `test_compile_placement_parity_e2e.py`
Parity: excluded paths, hidden roots	`test_compile_placement_parity_e2e.py` (`compilation.exclude`, `node_modules`, `.github`, untargeted hidden dir)
Real `apm compile` coverage	`test_compile_placement_parity_e2e.py`
`CompileInventory`	Unchanged
Docs	No changes needed; compile docs don't cover placement performance

Notes for reviewers

Directory resolution is cached, not skipped. perf(compile): fix quadratic and redundant-resolve costs in placement #2878 skipped resolve() for directories. On Windows, a junction can put an unresolved path into the inventory, so skipping it could change placement. This PR keeps resolve() and caches the result instead.
Differential check. main and this branch were compared on a fixture with in-project, external, dangling, looping and directory symlinks, nested .git, excluded and hidden directories, odd file names, an aliased base dir and emulated Windows junctions, across 41 patterns. No differences.
Not changed. On main, generated AGENTS.md files count as project files on the next compile, so a rerun can place instructions differently. Out of scope; the e2e test deletes them between runs.

Type of change

Bug fix
New feature
Documentation
Maintenance / refactor

Testing

Tested locally
All existing tests pass
Added tests for new functionality (if applicable)

Each commit passes tests/unit/compilation and the compile e2e tests on Python 3.10, 3.12 and 3.14. Lint job checks and ratchet tests pass locally.

Spec conformance (OpenAPM v0.1)

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Sergei Kozyrev (sekosen) requested review from Daniel Meppiel (danielmeppiel) and Sergio Sisternes (sergio-sisternes-epam) as code owners

October 24, 2026 12:15

Copilot AI balanced review requested due to automatic review settings

October 24, 2026 12:15

Copilot started reviewing on behalf of Sergei Kozyrev (sekosen)

October 24, 2026 12:16

View session

Copilot AI reviewed

Oct 24, 2026

View reviewed changes

Copilot AI left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

🟢 Approval recommended

Static review identified no blocking issues in the focused optimizations and regression coverage; tests were not executed during this review.

No open issues 🎉

What changed in this PR

This PR addresses #2969 by reducing repeated work in ContextOptimizer, improving instruction-placement performance on large projects while preserving existing matching and symlink behavior.

Changes:

Cache file-relative paths and resolved directories, clearing both caches when project structure is analyzed.
Compute the depth mean once and stop candidate coverage checks at the first uncovered directory.
Add regression tests for performance, cache resets, scoring, symlinks, and compile-output parity.

File	Description
`tests/unit/compilation/test_context_optimizer_symlinked_files.py`	Pins matching behavior for internal, external, dangling, and looping symlinks.
`tests/unit/compilation/test_context_optimizer_phase3.py`	Adds an exact distribution-score assertion.
`tests/unit/compilation/test_context_optimizer_perf_regressions.py`	Checks cache resets, resolution counts, mean computation, and short-circuiting.
`tests/integration/test_compile_placement_parity_e2e.py`	Checks placement and file inclusion through the in-process CLI.
`src/apm_cli/compilation/context_optimizer.py`	Adds path caches and removes redundant scoring and coverage work.

_{Review effort: Balanced}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Sergei Kozyrev (sekosen) and others added 4 commits

October 24, 2026 16:20


          test(compile): pin placement behavior before the perf fix

873abaf

Cover how symlinked files match applyTo patterns (in-project, outside
the project, dangling, colliding with a real file, and symlink loops on
Python < 3.13) and pin the exact distribution score. All of these pass
on main.

Co-Authored-By: Claude Opus 5.5 <[email protected]>


          test(compile): cover placement and file inclusion through apm compile

Run the real CLI on a project with ordinary files, in-project, outside
and dangling symlinks, excluded paths and a .github hidden root, and pin
the matched directories and generated AGENTS.md files. Passes on main.

Co-Authored-By: Claude Opus 5.5 <[email protected]>


          perf(compile): remove redundant work from placement

cc171a0

Large projects with overlapping scoped applyTo patterns spent most of
compile time repeating the same work in ContextOptimizer:

- cache each file's project-relative path instead of resolving it per
  pattern check
- resolve each directory once instead of on every coverage check
- compute the depth mean once instead of once per directory
- stop checking a placement candidate at its first uncovered directory

Both new caches are cleared whenever the project structure is analyzed.
Matching and placement results are unchanged; existing tests and the
tests from the previous two commits pass without changes.

Co-Authored-By: Claude Opus 5.5 <[email protected]>


          test(compile): catch regressions of the placement perf fix

2dd6155

Fail if the per-call caches leak between optimize_instruction_placement
calls, the depth mean is recomputed per directory, a candidate check
keeps going after its first miss, or a file or directory is resolved
more than once.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

Sergei Kozyrev (sekosen) commented Oct 24, 2026

Copy link

Copy Markdown

Author

@microsoft-github-policy-service agree

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sergei Kozyrev (sekosen) force-pushed the perf/placement-large-tree-scaling-v3 branch from a7a0098 to 2dd6155 Compare

October 24, 2026 12:33

Sergio Sisternes (sergio-sisternes-epam) commented Nov 3, 2026

Copy link

Copy Markdown

Collaborator

Thank you for contributing this performance optimization. Issue #2969 currently has status/needs-design. Although a later comment records approval of a bounded performance-only scope, the issue's status label has not been updated to status/accepted. Please coordinate with Daniel Meppiel (@danielmeppiel) to reconcile the issue's recorded decision and status before proceeding. If the approved scope does not cover this change, open an issue for maintainer review and acceptance first (https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md). This PR is labelled status/deferred until then. The requested reviewers include Daniel Meppiel (@danielmeppiel) and Sergio Sisternes (@sergio-sisternes-epam).

Generated by autopilot-pr-triage-worker. This comment is AI-generated and may contain errors.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sergio Sisternes (sergio-sisternes-epam) added area/cli

CLI command surface, flags, help text (cross-cutting).

status/deferred

Not invited for implementation now; no release commitment.

triage/recommended

Automated advice completed; not human scope approval.

type/performance Latency, throughput, memory, install time. labels

Nov 3, 2026

Sergei Kozyrev (sekosen) commented Nov 4, 2026

Copy link

Copy Markdown

Author

Daniel Meppiel (@danielmeppiel), you approved the performance-only scope for #2969 on October 19 in this comment. The issue still has status/needs-design, so triage marked this PR status/deferred. Could you set #2969 to status/accepted?

The PR stays within that scope. It doesn't touch symlink or containment handling, and the description maps each "Done when" item to a test.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Daniel Meppiel (danielmeppiel) mentioned this pull request

Nov 10, 2026

fix(compile): ignore generated context outputs in placement counting #3539

Merged

8 of 18 tasks

This branch has not been deployed

No deployments

Reviewers

Copilot code review Copilot

Copilot review effort

Applies to this pull request for everyone.Learn more about Copilot code review.

Copilot left review comments

Daniel Meppiel (danielmeppiel)

Awaiting requested review from danielmeppiel

danielmeppiel is a code owner

Sergio Sisternes (sergio-sisternes-epam)

Awaiting requested review from sergio-sisternes-epam

sergio-sisternes-epam is a code owner

At least 0 approving reviews are required to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

area/cli

CLI command surface, flags, help text (cross-cutting).

status/deferred

Not invited for implementation now; no release commitment.

triage/recommended

Automated advice completed; not human scope approval.

type/performance

Latency, throughput, memory, install time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

apm compile is very slow on large monorepos with scoped applyTo patterns (quadratic placement cost)

Uh oh!

There was an error while loading. Please reload this page.

3 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Sergei Kozyrev (sekosen) commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Benchmark

Issue and approved scope

Commits

"Done when" coverage

Notes for reviewers

Type of change

Testing

Spec conformance (OpenAPM v0.1)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Sergei Kozyrev (sekosen) commented Oct 24, 2026

Uh oh!

Sergio Sisternes (sergio-sisternes-epam) commented Nov 3, 2026

Uh oh!

Sergei Kozyrev (sekosen) commented Nov 4, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Sergei Kozyrev (sekosen) commented Oct 24, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.