Skip to content

feat(packagejson): remove RWMutex contention in InfoCache with atomic flag + SyncMap #1576

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

Merged
merged 3 commits into from
Aug 13, 2025

Conversation

camc314
Copy link
Contributor

@camc314 camc314 commented Aug 13, 2025

  • Replaced sync.RWMutex with atomic.Bool for read-only state tracking to eliminate lock contention.
  • Switched underlying storage from map to collections.SyncMap for concurrent, lock-free Get/Set operations.
  • Updated getPackageJsonInfo to call new IsReadonly() method before writes.
  • Added SetReadonly() for efficient state changes without blocking readers.

Diff when running tsgo against vscode repo (top = before, bottom = after)

Screenshot 2025-08-13 at 16 44 23

Diff when running tsgolint against vscode repo (top = before, bottom = after)

Screenshot 2025-08-13 at 17 15 57

VIsible in the flamegraphs, the lock contention when getting package.jsons from the cache is basically non-existant now

… SyncMap

- Replaced sync.RWMutex with atomic.Bool for thread-safe readonly flag
- Switched to collections.SyncMap for concurrent cache access
- Updated getPackageJsonInfo to use new IsReadonly() method
- Added SetReadonly method for efficient readonly state management
- Enhanced Get/Set operations for improved performance and thread safety
@camc314 camc314 force-pushed the c/lock-free-get-pkg-json branch from 41621a5 to 05b3aef Compare August 13, 2025 16:09
@camc314 camc314 changed the title feat: optimize packagejson cache with concurrent-safe sync/atomic and… feat(packagejson): remove RWMutex contention in InfoCache with atomic flag + SyncMap Aug 13, 2025
@camc314 camc314 marked this pull request as ready for review August 13, 2025 16:19
@camc314
Copy link
Contributor Author

camc314 commented Aug 13, 2025

Time to go faster! Thanks @jakebailey and @sheetalkamat, your review comments have shaved off some more time

Screenshot 2025-08-13 at 22 10 38

@jakebailey jakebailey added this pull request to the merge queue Aug 13, 2025
Merged via the queue into microsoft:main with commit 6e6df72 Aug 13, 2025
22 checks passed
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.

3 participants