Skip to content

Commit

Permalink
Fix regression in incremental Watchman queries on Windows (#1231)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #1231

D52513808 / #1210 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 / #1210 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.
```

Reviewed By: motiz88

Differential Revision: D54592646

fbshipit-source-id: de91b1a724fcec470e93650ec9c6adce8c148aac
  • Loading branch information
robhogan authored and facebook-github-bot committed Mar 7, 2024
1 parent 4346825 commit d661377
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions packages/metro-file-map/src/crawlers/watchman/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {performance} from 'perf_hooks';
const watchman = require('fb-watchman');

type WatchmanRoots = Map<
string,
string, // Posix-separated absolute path
$ReadOnly<{directoryFilters: Array<string>, watcher: string}>,
>;

Expand Down Expand Up @@ -181,7 +181,7 @@ module.exports = async function watchmanCrawl({

await Promise.all(
Array.from(rootProjectDirMappings).map(
async ([root, {directoryFilters, watcher}], index) => {
async ([posixSeparatedRoot, {directoryFilters, watcher}], index) => {
// Jest is only going to store one type of clock; a string that
// represents a local clock. However, the Watchman crawler supports
// a second type of clock that can be written by automation outside of
Expand All @@ -191,7 +191,11 @@ module.exports = async function watchmanCrawl({
// By using scm queries, we can create the haste map on a different
// system and import it, transforming the clock into a local clock.
const since = previousState.clocks.get(
normalizePathSeparatorsToPosix(pathUtils.absoluteToNormal(root)),
normalizePathSeparatorsToPosix(
pathUtils.absoluteToNormal(
normalizePathSeparatorsToSystem(posixSeparatedRoot),
),
),
);

perfLogger?.annotate({
Expand All @@ -218,7 +222,7 @@ module.exports = async function watchmanCrawl({
perfLogger?.point(`watchmanCrawl/query_${index}_start`);
const response = await cmd<WatchmanQueryResponse>(
'query',
root,
posixSeparatedRoot,
query,
);
perfLogger?.point(`watchmanCrawl/query_${index}_end`);
Expand All @@ -232,7 +236,7 @@ module.exports = async function watchmanCrawl({
isFresh = isFresh || response.is_fresh_instance;
}

results.set(root, response);
results.set(posixSeparatedRoot, response);
},
),
);
Expand Down

0 comments on commit d661377

Please sign in to comment.