perf(browsers): reduce synchronous existsSync calls in file discovery#495
Merged
perf(browsers): reduce synchronous existsSync calls in file discovery#495
Conversation
There was a problem hiding this comment.
PR Review: fast-glob refactor for profile discovery. Clean performance improvement replacing up to 12 sequential existsSync calls with single glob patterns, plus module-level installation caching. Key positives: behavior improvement (old code silently capped Chromium profiles at 10, Profile * glob now finds all profiles correctly); guard checks ensure fg.sync is never called with non-existent cwd; installCache invariant is well-documented; tests cleanly mock fast-glob; fast-glob already in package.json. Minor non-blocking note: the Firefox path in EnhancedCookieQueryService lost a debug log on readdirSync failure, but propagating unexpected errors is arguably better than silent swallowing. LGTM.
- Replace iterative existsSync calls with single fg.sync() glob patterns in profile discovery (BrowserAvailability.ts and EnhancedCookieQueryService.ts) - Cache browser installation checks at module level since installations don't change during process lifetime - Update tests to mock fast-glob instead of readdirSync Fixes #489
4cf6bba to
8303f8b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
existsSynccalls with singlefg.sync()glob patterns in Chromium and Firefox profile discoveryinstallCacheMap) since installations don't change during process lifetimeEnhancedCookieQueryServicetests to mockfast-globinstead ofreaddirSyncReduces 20-40 synchronous stat syscalls per query lifecycle to ~3-5 glob calls.
Closes #489
Test plan
pnpm type-checkpassespnpm lintpasses (Biome, 225 files)