Skip to content

perf(analyzer): index export lookups - #771

Open
mcdigman wants to merge 3 commits into
duriantaco:mainfrom
mcdigman:codex/index-mark-exports-upstream
Open

perf(analyzer): index export lookups#771
mcdigman wants to merge 3 commits into
duriantaco:mainfrom
mcdigman:codex/index-mark-exports-upstream

Conversation

@mcdigman

@mcdigman mcdigman commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #769 applying analogous fixes to _mark_exports.

I started drafting this before I noticed the new PR #770 had merged, which already absorbs the vast majority of the perf win this was going to have. I decided to still PR it anyway, because it squashes the last O(n^2) bit of the method and so is still a meaningful speedup on very large repositories like homeassistant/core.

I also preserved some light refactor changes I was doing that reduce the measured cyclomatic and cognitive complexity a bit more, by doing early returns and fusing loops to make the crucial control flow ordering clear and avoid differently named variables that do exactly the same thing.

Bottom line
Before this raft of O(n^2) squashing performance optimizations, my first run of homeassistant/core took ~5 hours (which is why I was only running homeassistant/core/homeassistant as a baseline). Now, with the full it takes just over 6 minutes, taking running skylos on the full repo from unfeasible to merely a quick coffee break. This sequence of O(n^2) fixes therefore represents a categorical improvement in the kinds of repos that could feasibly use skylos.

Summary

  • index transitive exported-class type lookups with sorted dotted-prefix ranges
  • fuse redundant definition scans and simplify control flow with early returns

Complexity

A lot of the line diff in the PR is just unindenting due to early returns; it meaningfully reduces the complexity metrics skylos uses to score the function, although they are still high.

_mark_exports metric Current main This PR Change
Cyclomatic (SKY-Q301) 52 47 −5
Cognitive (SKY-Q306) 114 88 −26

Behavior preservation

  • explicit __all__ remains authoritative per source file
  • direct exports remain scoped to their exact module
  • ambiguous re-export targets remain unresolved
  • TypeScript/Java/Kotlin/C# export handling remains unchanged
  • liveness_primer checked on expanded corpus, 0 diffs vs current main

Performance

An isolated 5,000-class transitive-export benchmark improved from 0.541s to 0.049s.

Corpus Current main _mark_exports This PR _mark_exports Saved Speedup
Home Assistant (homeassistant/core/homeassistant) 13.4s 0.65s 12.7s 20.6x
SciPy 0.22s 0.20s 0.025s 1.1x
Pydantic 0.03s 0.013s 0.017s 2.3x

For a full-root homeassistant/core scan with no cProfile isolation of _mark_exports, the speedup is 461.91s -> 384.77s, saving 77.14s.

Validation

  • 5 focused _mark_exports tests passed
  • 198 analyzer tests passed
  • all 15 integration tests passed
  • 85 dead-code and framework-aware tests passed
  • 5,000 randomized current-main/replacement cases matched
  • no Ruff findings on changed lines
  • Pyright produced the same 17 baseline findings on both versions
  • syntax compilation and git diff --check passed

AI Use Disclosure

The quadratic paths were identified from cProfile data. I did most of the original refactoring by hand in analogy to the previous _mark_refs speedup, with guidance from Codex. Codex integrated the optimization with the newer export semantics and ran behavior-preservation validation; the resulting diff was reviewed by Codex and Opus 5.0 agents.

@mcdigman
mcdigman marked this pull request as ready for review August 26, 2026 03:49
@mcdigman
mcdigman requested a review from duriantaco as a code owner August 26, 2026 03:49
@duriantaco

duriantaco commented Aug 27, 2026

Copy link
Copy Markdown
Owner

Thanks for putting this together the implementation looks sound.

One request before merge: could you add a deterministic regression guard for the performance property? The new tests validate preserved output, but they also pass against the previous per-exported-class full scan. A small instrumented mapping or scale probe that confirms self._global_type_map is indexed once, rather than .items() being scanned for every reachable class, would protect the actual O(n^2) fix without relying on wall-clock timing.

Separately, and completely unrelated to review or merge.. we really value the depth of your profiling and feedback. Would you be interested in being a design partner for the paid version of Skylos? We would love occasional candid feedback on workflows, pain points, etc. There is no commitment or pressure at all. If you are interested, let us know and we can find a good way to chat privately.

@mcdigman

Copy link
Copy Markdown
Contributor Author

Ok, I added unit tests with spies to check the number of calls is consistent with expectations, both in the case you described and the logarithmic search. Note if future amendments to the function add more calls to the spied-upon attributes deliberately, the test thresholds would also have to be revised.

I tried to email the aaron@skylos.dev email but my email bounced; is it listed correctly? I also sent one to the email in the pyproject.toml

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.

2 participants