Repository navigation
perf: parse a saved controller once instead of once per route - #117
Merged
Merged
Conversation
RoutesScanner.invalidate created a new VirtualFileSystem for each route bound to the changed controller. Each instance had an empty AST cache, so the dev server read and parsed the same controller file once per route on every save. Create the VirtualFileSystem once before the loop, as scan() already does. A fresh instance is still created on each call, so an invalidation always reads the latest file contents.
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.
Hey! 👋
When a controller changes in HMR mode, the dev server calls
RoutesScanner.invalidate()to recompute the request and response types of its routes. This method created a newVirtualFileSystemfor each route bound to the controller. Each instance starts with an empty AST cache, so the same file was read and parsed with ast-grep once per route.This PR creates the
VirtualFileSystemonce before the loop, asscan()already does. Each call still creates a new instance, so an invalidation always reads the latest file contents.Benchmark
Node.js 26.10, single core, public
RoutesScannerAPI:scan(), then timeinvalidate(controllerPath). Generated controllers with and without VineJS validators. Each variant ran in 5 independent processes with 20 warmups and 40 measured calls. Execution order alternated between variants. Values are the median of the per-process medians.There is no change with 1 route. With 3 routes the call is 1.2× to 2× faster, and with 7 routes 1.7× to 3.3× faster.
In a real app created with
create-adonisjs -K=api(HMR mode, TuyaugenerateRegistryhook), on a 36-line controller with 7 routes, the registry is up to date 18 ms after the save instead of 28 ms. The HTTP response is not affected:#reScanRoutesis not awaited and runs in the dev server process, so the hot reload takes about 30 ms in both cases. In watch mode, a controller change restarts the server andinvalidate()is not called.For typical controllers this is a small cleanup, not a visible speedup.
Validation
getScannedRoutes()is identical before and after the change, for 50 controller configurations (50 to 2000 lines, 1 to 30 routes, with and without validators). Compared afterscan(), afterinvalidate(), and after a file edit followed byinvalidate().git diff --checkpassedNot covered: no new test checks how many times the file is parsed. The existing routes scanner tests cover the output.