Skip to content

feat(configfileregistry): optimize config discovery #1595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

camc314
Copy link
Contributor

@camc314 camc314 commented Aug 17, 2025

Refactor hot paths in project setup for better performance and simplicity.

  • Config acquisition and parsing: Add fast-path for already-parsed configs, deduplicate concurrent parses using a wait group, use narrower locks (read locks for reads, write locks only for updates), avoid full reloads on entry creation, and support targeted reloads (file lists only).
  • Default project discovery: Replace recursive goroutine-based traversal with a single-threaded iterative BFS queue. Track visited paths by (path, loadKind) pair, upgrade loadKind monotonically, and exit early on a good match. This removes recursion depth issues, goroutine overload, and sync overhead, while making resolution deterministic.
  • Result tracking and service wiring: Use a single mutex with plain maps instead of SyncMaps and multiple locks. Ensure monotonic updates to prevent revisits with weaker loadKind. Initialize activeParsing in NewService and make cleanup consistent with BFS using plain maps under mutex.

These changes eliminate duplicate parsing, cut CPU spikes, and avoid goroutine storms in large reference graphs. The design now scales linearly with project size and offers more predictable latency under load.

Performance:

Old design: O(E) goroutines (E = edges in tsconfig graph), with duplicate work on converging paths. Parsing was O(N * c) worst-case (N = configs, c = concurrent requests per config).

New design: O(V + E) traversal (V = tsconfigs), visiting each config once per strongest loadKind. Parsing is O(N) amortized, as each config parses only once.

See oxc-project/tsgolint#99 for before/after analysis.

@Copilot Copilot AI review requested due to automatic review settings August 17, 2025 00:21
Copy link
Contributor

@Copilot Copilot AI left a comment

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 refactors the configuration file discovery system to eliminate performance bottlenecks through several optimizations. The changes move from a concurrent goroutine-based approach to a deterministic iterative design that avoids duplicate work and scales linearly with project size.

Key changes include:

  • Optimized config parsing: Fast-path for already-parsed configs, deduplication of concurrent parses using wait groups, and more granular locking
  • Replaced recursive traversal: Single-threaded BFS queue instead of goroutine-based recursive search with better visited state tracking
  • Simplified synchronization: Single mutex with plain maps replacing multiple sync.Map instances and separate locks

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
internal/project/configfileregistry.go Adds deduplication logic for concurrent config parsing and fast-path returns for already-parsed configs
internal/project/defaultprojectfinder.go Replaces recursive goroutine-based project discovery with iterative BFS queue and consolidated mutex usage
internal/project/service.go Initializes activeParsing map and updates cleanup logic to use plain maps instead of sync.Map

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@camc314 camc314 force-pushed the c/optimize-config-file-registry branch from 052306b to 46d7823 Compare August 17, 2025 00:26
Refactor hot paths in project setup for better performance and simplicity.

- Config acquisition and parsing: Add fast-path for already-parsed configs, deduplicate concurrent parses using a wait group, use narrower locks (read locks for reads, write locks only for updates), avoid full reloads on entry creation, and support targeted reloads (file lists only).
- Default project discovery: Replace recursive goroutine-based traversal with a single-threaded iterative BFS queue. Track visited paths by (path, loadKind) pair, upgrade loadKind monotonically, and exit early on a good match. This removes recursion depth issues, goroutine overload, and sync overhead, while making resolution deterministic.
- Result tracking and service wiring: Use a single mutex with plain maps instead of SyncMaps and multiple locks. Ensure monotonic updates to prevent revisits with weaker loadKind. Initialize activeParsing in NewService and make cleanup consistent with BFS using plain maps under mutex.

These changes eliminate duplicate parsing, cut CPU spikes, and avoid goroutine storms in large reference graphs. The design now scales linearly with project size and offers more predictable latency under load.

Performance:

Old design: O(E) goroutines (E = edges in tsconfig graph), with duplicate work on converging paths. Parsing was O(N * c) worst-case (N = configs, c = concurrent requests per config).

New design: O(V + E) traversal (V = tsconfigs), visiting each config once per strongest loadKind. Parsing is O(N) amortized, as each config parses only once.

See oxc-project/tsgolint#99 for before/after analysis.
@camc314 camc314 force-pushed the c/optimize-config-file-registry branch from 46d7823 to b7e764a Compare August 17, 2025 00:45
@jakebailey
Copy link
Member

  • Default project discovery: Replace recursive goroutine-based traversal with a single-threaded iterative BFS queue.

I suspect this may already be done in #1505 (or at least, a similar concept); a lot of this I think may end up changing in ways to make it work in snapshots.

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