Repository navigation
fix(core): start fff search lazily on first find - #58959
Conversation
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The change looks sound. I checked it with a throwaway bench that built fffLayer for 10 git repos in one process. On the base, threads went from 31 to 79 as soon as the layers were built. With this PR they stay at 31 until the first search in each repo, and every first search found its file. search.test.ts passes (6 tests), and typecheck:tsc for packages/core is clean. The picker is created in one synchronous step, so concurrent first searches can't create two pickers. A creation failure is still logged once and returns [], and the picker is still destroyed with the layer scope.
Public API: unchanged. isScanning() is added only to the internal Picker interface.
One small suggestion on the new test (inline comment).
|
|
||
| expect(yield* paths("existing-alpha")).toContain(RelativePath.make("src/existing-alpha.ts")) | ||
|
|
||
| yield* Effect.promise(() => Bun.write(path.join(directory, "src/created-beta.ts"), "beta")) |
There was a problem hiding this comment.
The first-search assertion above is the one that covers this change. The create and delete steps below mostly test fff's own watcher, using real filesystem events, polling and 10-second timeouts. That makes the test slower and more likely to be flaky on loaded CI machines, without checking anything new about lazy startup. Could you drop them and keep just the first search?
There was a problem hiding this comment.
The new commit looks sound. A search that is cancelled midway can no longer leave a picker that nothing cleans up: the layer registers its cleanup before the picker exists and creates it in one step that can't be interrupted. The failure warning is still logged only once, and a search after shutdown returns [] without creating anything. The three tests that use a fake backend run on TestClock, so they don't depend on real timing.
I re-ran my bench with this commit: 31 threads after building 10 layers, 79 after one search each, and all 10 searches found their file. search.test.ts passed 9 of 9 in three runs, and typecheck:tsc is clean.
Public API: unchanged. makeFffLayer and isScanning() are internal to packages/core.
My earlier comment about the live create/delete test is still open.
There was a problem hiding this comment.
My earlier comment is addressed. The live fff test now only checks that the first search finds an existing file, and the create/delete steps with 10-second timeouts are gone. This commit changes only the test file. search.test.ts passes (9 tests) and typecheck:tsc is clean. Public API: unchanged.
What it does. Every git Location used to start its own
ffffile finder as soon as it booted, even if nobody ever searched in it. Each finder brings its own scanner, watcher and git threads, so a server with a few dozen open projects carried about 150 extra threads from the start. Now the finder starts on the first file search in that Location.How it works.
fffLayerno longer callsFff.createwhile the layer is built. The firstfind()creates the picker. The layer registers its finalizer before the picker exists, and creating and publishing the picker is one synchronous, uninterruptible step, so an interrupted search can't leave a native picker without an owner. A search after shutdown returns[]and creates nothing.find()waits for the scan to finish, up to 3 seconds, before searching. Before this change the boot-time head start hid that wait. After a timeout it returns whatever has been indexed so far, the same as searching during a scan used to.[], as before.PickergainsisScanning(). Both the bun and nodefffpackages already have it.makeFffLayer(create)takes the backend so tests can drive scanning by hand;fffLayerismakeFffLayer(Fff.create).Risk. Server only, file search (
@mentions, file pickers). The first search in each project now pays for the initial scan (milliseconds for a normal repo, capped at 3 s for a huge one). After that, search behaves as before:fffstill runs its own watcher once started. Finders aren't evicted when idle yet, so projects you've searched in keep their threads until the Location shuts down.Proof.
A bench script (not committed) builds
fffLayerfor N temporary git repos (500 files each) in one process. It samplesps -Mthread count and RSS after the layers are built ("boot"), then again after one search per repo:The process baseline with no Locations is 30–31 threads and about 49 MB, so after this change boot adds no
fffthreads. Every search found its file in both runs.bun test test/filesystem/search.test.ts: 9 pass. Three tests use a fake backend withTestClock: no picker until the first search, which stays pending until the scan finishes; partial results at the 3 s timeout, and later searches don't wait; an interrupted first search leaves the picker owned and destroyed exactly once when the layer closes. One live test with realfffchecks that the very first search finds an existing file. With the scan wait removed, 3 of these fail.bun run typecheck(packages/core): no type errors.oxlinton changed files: clean.An independent review of this PR asked for two fixes: tie picker ownership to the layer so an interrupted search can't leak it, and replace the timing-dependent scan test with
TestClocktests. Both are done.Added by the opencode bot (AI-generated).
Related: #54631, #37794