Fix regression in incremental Watchman queries on Windows #1231
Conversation
This file contains 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
Summary: D52513808 introduced a startup regression in Watchman crawling on Windows where a clock key was incorrectly calculated due to more strict behaviour around path separators. This meant the clock was not matched with cached state, and the crawler would fall back to a "glob" query, rather than the (typically much cheaper) incremental "since" query. The bug isn't in the new `RootPathUtils` itself, which already states its assumptions (including `"All input path separators must be system-native."`), but rather in the failure to properly normalise a particular response from Watchman (which returns posix separators, even on Windows) to Metro's system-native paths, before using `pathUtils.absoluteToNormal`. Note that we already normalise other responses from Watchman to system separators. Before D52513808 we would've deemed the path too complex to handle and fallen back to `path.relative`, which is much slower but more forgiving. Changelog ``` * **[Fix]**: Fix startup time regression when using Watchman on Windows with a warm cache. ``` Differential Revision: D54592646
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D54592646 |
|
This pull request has been merged in d661377. |
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:
D52513808 introduced a startup regression in Watchman crawling on Windows where a clock key was incorrectly calculated due to more strict behaviour around path separators.
This meant the clock was not matched with cached state, and the crawler would fall back to a "glob" query, rather than the (typically much cheaper) incremental "since" query.
The bug isn't in the new
RootPathUtilsitself, which already states its assumptions (including"All input path separators must be system-native."), but rather in the failure to properly normalise a particular response from Watchman (which returns posix separators, even on Windows) to Metro's system-native paths, before usingpathUtils.absoluteToNormal. Note that we already normalise other responses from Watchman to system separators.Before D52513808 we would've deemed the path too complex to handle and fallen back to
path.relative, which is much slower but more forgiving.Changelog
Differential Revision: D54592646