Skip to content

Commit

Permalink
Watcher backends: merge 'add' and 'change' events into 'touch'
Browse files Browse the repository at this point in the history
Summary:
Refactor watcher backends so that they're not required to disambiguate "add" file events from "change" file events, instead emitting ambiguous "touch" events.

This distinction isn't typically available from the host OS (`fsevents`, `ReadDirectoryChangesW`, `inotify`), so for the backends to supply it, they must track all files to know whether they existed prior to the OS event. Watchman does this by design anyway, and typically only once per session, but both our `FSEventsWatcher` and `NodeWatcher` currently crawl the whole directory tree and keep an in-memory list of files, and must do so on each Metro start.

We don't need this information from the watchers anyway, because we can already infer new vs modified according to whether the file is present in `TreeFS` (ie, from the crawl, or subsequent event). Therefore we *don't* need to reduce the granularity of `metro-file-map`'s *public* events, only the internal ones.

By simplifying this, we can follow up by taking a huge burden off the watcher backends.

Changelog: Internal

Differential Revision: D67579233
  • Loading branch information
robhogan authored and facebook-github-bot committed Dec 22, 2024
1 parent 1deb730 commit cf3533e
Show file tree
Hide file tree
Showing 10 changed files with 86 additions and 91 deletions.
4 changes: 2 additions & 2 deletions packages/metro-file-map/src/Watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import type {AbortSignal} from 'node-abort-controller';

import nodeCrawl from './crawlers/node';
import watchmanCrawl from './crawlers/watchman';
import {ADD_EVENT, CHANGE_EVENT} from './watchers/common';
import {TOUCH_EVENT} from './watchers/common';
import FSEventsWatcher from './watchers/FSEventsWatcher';
import NodeWatcher from './watchers/NodeWatcher';
import WatchmanWatcher from './watchers/WatchmanWatcher';
Expand Down Expand Up @@ -210,7 +210,7 @@ export class Watcher extends EventEmitter {
watcher.on('all', (change: WatcherBackendChangeEvent) => {
const basename = path.basename(change.relativePath);
if (basename.startsWith(this._options.healthCheckFilePrefix)) {
if (change.event === ADD_EVENT || change.event === CHANGE_EVENT) {
if (change.event === TOUCH_EVENT) {
debug(
'Observed possible health check cookie: %s in %s',
change.relativePath,
Expand Down
62 changes: 28 additions & 34 deletions packages/metro-file-map/src/__tests__/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1621,13 +1621,13 @@ describe('FileMap', () => {
`;
const e = mockEmitters[path.join('/', 'project', 'fruits')];
e.emit('all', {
event: 'add',
event: 'touch',
relativePath: 'Tomato.js',
root: path.join('/', 'project', 'fruits'),
metadata: MOCK_CHANGE_FILE,
});
e.emit('all', {
event: 'change',
event: 'touch',
relativePath: 'Pear.js',
root: path.join('/', 'project', 'fruits'),
metadata: MOCK_CHANGE_FILE,
Expand Down Expand Up @@ -1662,13 +1662,13 @@ describe('FileMap', () => {
// Tomato!
`;
e.emit('all', {
event: 'change',
event: 'touch',
relativePath: 'Tomato.js',
root: path.join('/', 'project', 'fruits'),
metadata: MOCK_CHANGE_FILE,
});
e.emit('all', {
event: 'change',
event: 'touch',
relativePath: 'Tomato.js',
root: path.join('/', 'project', 'fruits'),
metadata: MOCK_CHANGE_FILE,
Expand All @@ -1683,25 +1683,22 @@ describe('FileMap', () => {
const {fileSystem} = await hm.build();
const fruitsRoot = path.join('/', 'project', 'fruits');
const e = mockEmitters[fruitsRoot];
mockFs[path.join(fruitsRoot, 'Tomato.js')] = `
// Tomato!
`;
e.emit('all', {
event: 'change',
relativePath: 'Tomato.js',
event: 'touch',
relativePath: 'Strawberry.js',
root: fruitsRoot,
metadata: MOCK_CHANGE_FILE,
});
e.emit('all', {
event: 'change',
event: 'touch',
relativePath: 'LinkToStrawberry.js',
root: fruitsRoot,
metadata: MOCK_CHANGE_LINK,
});
const {eventsQueue} = await waitForItToChange(hm);
expect(eventsQueue).toEqual([
{
filePath: path.join(fruitsRoot, 'Tomato.js'),
filePath: path.join(fruitsRoot, 'Strawberry.js'),
metadata: MOCK_CHANGE_FILE,
type: 'change',
},
Expand All @@ -1718,25 +1715,22 @@ describe('FileMap', () => {
const {fileSystem} = await hm.build();
const fruitsRoot = path.join('/', 'project', 'fruits');
const e = mockEmitters[fruitsRoot];
mockFs[path.join(fruitsRoot, 'Tomato.js')] = `
// Tomato!
`;
e.emit('all', {
event: 'change',
relativePath: 'Tomato.js',
event: 'touch',
relativePath: 'Strawberry.js',
root: fruitsRoot,
metadata: MOCK_CHANGE_FILE,
});
e.emit('all', {
event: 'change',
event: 'touch',
relativePath: 'LinkToStrawberry.js',
root: fruitsRoot,
metadata: MOCK_CHANGE_LINK,
});
const {eventsQueue} = await waitForItToChange(hm);
expect(eventsQueue).toEqual([
{
filePath: path.join(fruitsRoot, 'Tomato.js'),
filePath: path.join(fruitsRoot, 'Strawberry.js'),
metadata: MOCK_CHANGE_FILE,
type: 'change',
},
Expand All @@ -1759,7 +1753,7 @@ describe('FileMap', () => {
const {fileSystem} = await hm.build();
const e = mockEmitters[path.join('/', 'project', 'fruits')];
e.emit('all', {
event: 'add',
event: 'touch',
relativePath: 'apple.js',
root: path.join('/', 'project', 'fruits', 'node_modules', ''),
metadata: MOCK_CHANGE_FILE,
Expand Down Expand Up @@ -1788,13 +1782,13 @@ describe('FileMap', () => {

const e = mockEmitters[path.join('/', 'project', 'fruits')];
e.emit('all', {
event: 'add',
event: 'touch',
relativePath: 'Banana.js',
root: path.join('/', 'project', 'fruits', ''),
metadata: MOCK_CHANGE_FILE,
});
e.emit('all', {
event: 'add',
event: 'touch',
relativePath: 'Banana.unwatched',
root: path.join('/', 'project', 'fruits', ''),
metadata: MOCK_CHANGE_FILE,
Expand All @@ -1803,7 +1797,7 @@ describe('FileMap', () => {
const filePath = path.join('/', 'project', 'fruits', 'Banana.js');
expect(eventsQueue).toHaveLength(1);
expect(eventsQueue).toEqual([
{filePath, metadata: MOCK_CHANGE_FILE, type: 'add'},
{filePath, metadata: MOCK_CHANGE_FILE, type: 'change'},
]);
expect(fileSystem.getModuleName(filePath)).toBeDefined();
},
Expand Down Expand Up @@ -1844,7 +1838,7 @@ describe('FileMap', () => {
link: 'Strawberry.js',
};
e.emit('all', {
event: 'add',
event: 'touch',
relativePath: 'LinkToStrawberry.ext',
root: path.join('/', 'project', 'fruits', ''),
metadata: MOCK_CHANGE_LINK,
Expand Down Expand Up @@ -1921,13 +1915,13 @@ describe('FileMap', () => {
expect(hasteMap.getModule('Orange', 'android')).toBeTruthy();
const e = mockEmitters[path.join('/', 'project', 'fruits')];
e.emit('all', {
event: 'change',
event: 'touch',
relativePath: 'Orange.ios.js',
root: path.join('/', 'project', 'fruits'),
metadata: MOCK_CHANGE_FILE,
});
e.emit('all', {
event: 'change',
event: 'touch',
relativePath: 'Orange.android.js',
root: path.join('/', 'project', 'fruits'),
metadata: MOCK_CHANGE_FILE,
Expand Down Expand Up @@ -1994,7 +1988,7 @@ describe('FileMap', () => {
root: path.join('/', 'project', 'vegetables'),
});
mockEmitters[path.join('/', 'project', 'fruits')].emit('all', {
event: 'add',
event: 'touch',
relativePath: 'Melon.js',
root: path.join('/', 'project', 'fruits'),
metadata: MOCK_CHANGE_FILE,
Expand Down Expand Up @@ -2033,13 +2027,13 @@ describe('FileMap', () => {
`;
const e = mockEmitters[path.join('/', 'project', 'fruits')];
e.emit('all', {
event: 'change',
event: 'touch',
relativePath: 'Pear.js',
root: path.join('/', 'project', 'fruits'),
metadata: MOCK_CHANGE_FILE,
});
e.emit('all', {
event: 'add',
event: 'touch',
relativePath: 'Pear.js',
root: path.join('/', 'project', 'fruits', 'another'),
metadata: MOCK_CHANGE_FILE,
Expand Down Expand Up @@ -2081,13 +2075,13 @@ describe('FileMap', () => {
const {fileSystem} = await hm.build();
const e = mockEmitters[path.join('/', 'project', 'fruits')];
e.emit('all', {
event: 'change',
event: 'touch',
relativePath: 'Pear.js',
root: path.join('/', 'project', 'fruits'),
metadata: MOCK_CHANGE_FILE,
});
e.emit('all', {
event: 'add',
event: 'touch',
relativePath: 'Pear.js',
root: path.join('/', 'project', 'fruits', 'another'),
metadata: MOCK_CHANGE_FILE,
Expand Down Expand Up @@ -2139,7 +2133,7 @@ describe('FileMap', () => {
metadata: MOCK_CHANGE_FILE,
});
e.emit('all', {
event: 'add',
event: 'touch',
relativePath: 'Pear2.js',
root: path.join('/', 'project', 'fruits'),
metadata: MOCK_CHANGE_FILE,
Expand All @@ -2164,7 +2158,7 @@ describe('FileMap', () => {
`;
const e = mockEmitters[path.join('/', 'project', 'fruits')];
e.emit('all', {
event: 'add',
event: 'touch',
relativePath: 'Pear2.js',
root: path.join('/', 'project', 'fruits', 'another'),
metadata: MOCK_CHANGE_FILE,
Expand All @@ -2189,13 +2183,13 @@ describe('FileMap', () => {
// Tomato!
`;
e.emit('all', {
event: 'change',
event: 'touch',
relativePath: 'tomato.js',
root: path.join('/', 'project', 'fruits'),
metadata: MOCK_CHANGE_FOLDER,
});
e.emit('all', {
event: 'change',
event: 'touch',
relativePath: path.join('tomato.js', 'index.js'),
root: path.join('/', 'project', 'fruits'),
metadata: MOCK_CHANGE_FILE,
Expand Down
2 changes: 1 addition & 1 deletion packages/metro-file-map/src/flow-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ export type ReadOnlyRawMockMap = $ReadOnly<{

export type WatcherBackendChangeEvent =
| $ReadOnly<{
event: 'change' | 'add',
event: 'touch',
relativePath: string,
root: string,
metadata: ChangeEventMetadata,
Expand Down
19 changes: 14 additions & 5 deletions packages/metro-file-map/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -898,14 +898,23 @@ export default class FileMap extends EventEmitter {
// null, then it is assumed that the watcher does not have capabilities
// to detect modified time, and change processing proceeds.
if (
change.event === 'change' &&
change.event === 'touch' &&
linkStats != null &&
change.metadata.modifiedTime != null &&
linkStats.modifiedTime === change.metadata.modifiedTime
) {
return;
}

// Emitted events, unlike memoryless backend events, specify 'add' or
// 'change' instead of 'touch'.
const eventTypeToEmit =
change.event === 'touch'
? linkStats == null
? 'add'
: 'change'
: 'delete';

const onChangeStartTime = performance.timeOrigin + performance.now();

changeQueue = changeQueue
Expand All @@ -915,7 +924,7 @@ export default class FileMap extends EventEmitter {
nextEmit != null &&
nextEmit.eventsQueue.find(
event =>
event.type === change.event &&
event.type === eventTypeToEmit &&
event.filePath === absoluteFilePath &&
((!event.metadata && !change.metadata) ||
(event.metadata &&
Expand All @@ -935,7 +944,7 @@ export default class FileMap extends EventEmitter {
const event = {
filePath: absoluteFilePath,
metadata,
type: change.event,
type: eventTypeToEmit,
};
if (nextEmit == null) {
nextEmit = {
Expand All @@ -960,9 +969,9 @@ export default class FileMap extends EventEmitter {
);
}

// If the file was added or changed,
// If the file was added or modified,
// parse it and update the haste map.
if (change.event === 'add' || change.event === 'change') {
if (change.event === 'touch') {
invariant(
change.metadata.size != null,
'since the file exists or changed, it should have known size',
Expand Down
20 changes: 11 additions & 9 deletions packages/metro-file-map/src/watchers/FSEventsWatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ try {
// Optional dependency, only supported on Darwin.
}

const CHANGE_EVENT = 'change';
const TOUCH_EVENT = 'touch';
const DELETE_EVENT = 'delete';
const ADD_EVENT = 'add';
const ALL_EVENT = 'all';

/**
Expand Down Expand Up @@ -94,7 +93,8 @@ export default class FSEventsWatcher extends EventEmitter {
});
});

debug(`Watching ${this.root}`);
const startTime = performance.now();
debug('Watching %s', this.root);

this._tracked = new Set();
const trackPath = (filePath: string) => {
Expand All @@ -108,6 +108,11 @@ export default class FSEventsWatcher extends EventEmitter {
trackPath,
() => {
this.emit('ready');
debug(
'Scanned %s in %d',
this.root,
(performance.now() - startTime) / 1000,
);
resolve();
},
(...args) => {
Expand Down Expand Up @@ -141,6 +146,7 @@ export default class FSEventsWatcher extends EventEmitter {

async _handleEvent(filepath: string) {
const relativePath = path.relative(this.root, filepath);
debug('Handling event on %s (root: %s)', relativePath, this.root);

try {
const stat = await fsPromises.lstat(filepath);
Expand All @@ -161,12 +167,8 @@ export default class FSEventsWatcher extends EventEmitter {
size: stat.size,
};

if (this._tracked.has(filepath)) {
this._emit({event: CHANGE_EVENT, relativePath, metadata});
} else {
this._tracked.add(filepath);
this._emit({event: ADD_EVENT, relativePath, metadata});
}
this._emit({event: TOUCH_EVENT, relativePath, metadata});
this._tracked.add(filepath);
} catch (error) {
if (error?.code !== 'ENOENT') {
this.emit('error', error);
Expand Down
Loading

0 comments on commit cf3533e

Please sign in to comment.