Skip to content

Commit

Permalink
Improve Fast Refresh responsiveness when watching a large number of f…
Browse files Browse the repository at this point in the history 
…iles (#913)

Summary:
Pull Request resolved: #913

Following D40829941 (7191173), where we changed the output of `metro-file-map`'s `build()` to return "live" references to the file system and the Haste module map (rather than snapshots), we no longer require further snapshots to be emitted as part of a change event payload. In fact, `ChangeEvent['snapshotFS']` and `ChangeEvent['moduleMap']` are currently only referenced in test code.

Removing them means we don't have to copy potentially large data structures on each emitted change.

With ~300k files and ~150k Haste map entries, this amounts to ~200ms more responsive fast refresh - in general this boost will be proportional to the total number of files+Haste modules watched.

After this, a `HasteFS` instance is only constructed once, on Metro startup. That clears the way for an alternative (more expensive to initialise) implementation capable of performing lookups through symlinks.

Changelog: [Performance] Improve Fast Refresh responsiveness when watching a large number of files.

Reviewed By: motiz88

Differential Revision: D42303139

fbshipit-source-id: b158ca2fdcddf02ecb2f2d17bc5b35e448fed0ae
  • Loading branch information
@robhogan @facebook-github-bot
robhogan authored and facebook-github-bot committed Jan 4, 2023
1 parent 7d89ee4 commit b942eca
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 55 deletions.
67 changes: 34 additions & 33 deletions packages/metro-file-map/src/__tests__/index-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1364,13 +1364,13 @@ describe('HasteMap', () => {
}

hm_it('build returns a "live" fileSystem and hasteModuleMap', async hm => {
const initialResult = await hm.build();
const {fileSystem, hasteModuleMap} = await hm.build();
const filePath = path.join('/', 'project', 'fruits', 'Banana.js');
expect(initialResult.fileSystem.getModuleName(filePath)).toBeDefined();
expect(initialResult.hasteModuleMap.getModule('Banana')).toBe(filePath);
expect(fileSystem.getModuleName(filePath)).toBeDefined();
expect(hasteModuleMap.getModule('Banana')).toBe(filePath);
mockDeleteFile(path.join('/', 'project', 'fruits'), 'Banana.js');
mockDeleteFile(path.join('/', 'project', 'fruits'), 'Banana.js');
const {eventsQueue, snapshotFS, moduleMap} = await waitForItToChange(hm);
const {eventsQueue} = await waitForItToChange(hm);
expect(eventsQueue).toHaveLength(1);
const deletedBanana = {
filePath,
Expand All @@ -1379,10 +1379,8 @@ describe('HasteMap', () => {
};
expect(eventsQueue).toEqual([deletedBanana]);
// Verify that the initial result has been updated
expect(initialResult.fileSystem.getModuleName(filePath)).toBeNull();
expect(initialResult.hasteModuleMap.getModule('Banana')).toBeNull();
expect(snapshotFS.getModuleName(filePath)).toBeNull();
expect(moduleMap.getModule('Banana')).toBeNull();
expect(fileSystem.getModuleName(filePath)).toBeNull();
expect(hasteModuleMap.getModule('Banana')).toBeNull();
});

const MOCK_CHANGE_FILE = {
Expand All @@ -1398,6 +1396,7 @@ describe('HasteMap', () => {
};

hm_it('handles several change events at once', async hm => {
const {fileSystem, hasteModuleMap} = await hm.build();
mockFs[path.join('/', 'project', 'fruits', 'Tomato.js')] = `
// Tomato!
`;
Expand All @@ -1419,7 +1418,7 @@ describe('HasteMap', () => {
path.join('/', 'project', 'fruits'),
MOCK_CHANGE_FILE,
);
const {eventsQueue, snapshotFS, moduleMap} = await waitForItToChange(hm);
const {eventsQueue} = await waitForItToChange(hm);
expect(eventsQueue).toEqual([
{
filePath: path.join('/', 'project', 'fruits', 'Tomato.js'),
Expand All @@ -1433,12 +1432,12 @@ describe('HasteMap', () => {
},
]);
expect(
snapshotFS.getModuleName(
fileSystem.getModuleName(
path.join('/', 'project', 'fruits', 'Tomato.js'),
),
).not.toBeNull();
expect(moduleMap.getModule('Tomato')).toBeDefined();
expect(moduleMap.getModule('Pear')).toBe(
expect(hasteModuleMap.getModule('Tomato')).toBeDefined();
expect(hasteModuleMap.getModule('Pear')).toBe(
path.join('/', 'project', 'fruits', 'Pear.js'),
);
});
Expand Down Expand Up @@ -1466,6 +1465,7 @@ describe('HasteMap', () => {
hm_it(
'emits a change even if a file in node_modules has changed',
async hm => {
const {fileSystem} = await hm.build();
const e = mockEmitters[path.join('/', 'project', 'fruits')];
e.emit(
'all',
Expand All @@ -1474,7 +1474,7 @@ describe('HasteMap', () => {
path.join('/', 'project', 'fruits', 'node_modules', ''),
MOCK_CHANGE_FILE,
);
const {eventsQueue, snapshotFS} = await waitForItToChange(hm);
const {eventsQueue} = await waitForItToChange(hm);
const filePath = path.join(
'/',
'project',
Expand All @@ -1486,16 +1486,16 @@ describe('HasteMap', () => {
expect(eventsQueue).toEqual([
{filePath, metadata: MOCK_CHANGE_FILE, type: 'add'},
]);
expect(snapshotFS.getModuleName(filePath)).toBeDefined();
expect(fileSystem.getModuleName(filePath)).toBeDefined();
},
);

hm_it(
'correctly tracks changes to both platform-specific versions of a single module name',
async hm => {
const {hasteModuleMap: initMM} = await hm.build();
expect(initMM.getModule('Orange', 'ios')).toBeTruthy();
expect(initMM.getModule('Orange', 'android')).toBeTruthy();
const {hasteModuleMap, fileSystem} = await hm.build();
expect(hasteModuleMap.getModule('Orange', 'ios')).toBeTruthy();
expect(hasteModuleMap.getModule('Orange', 'android')).toBeTruthy();
const e = mockEmitters[path.join('/', 'project', 'fruits')];
e.emit(
'all',
Expand All @@ -1511,9 +1511,7 @@ describe('HasteMap', () => {
path.join('/', 'project', 'fruits'),
MOCK_CHANGE_FILE,
);
const {eventsQueue, snapshotFS, moduleMap} = await waitForItToChange(
hm,
);
const {eventsQueue} = await waitForItToChange(hm);
expect(eventsQueue).toHaveLength(2);
expect(eventsQueue).toEqual([
{
Expand All @@ -1528,20 +1526,20 @@ describe('HasteMap', () => {
},
]);
expect(
snapshotFS.getModuleName(
fileSystem.getModuleName(
path.join('/', 'project', 'fruits', 'Orange.ios.js'),
),
).toBeTruthy();
expect(
snapshotFS.getModuleName(
fileSystem.getModuleName(
path.join('/', 'project', 'fruits', 'Orange.android.js'),
),
).toBeTruthy();
const iosVariant = moduleMap.getModule('Orange', 'ios');
const iosVariant = hasteModuleMap.getModule('Orange', 'ios');
expect(iosVariant).toBe(
path.join('/', 'project', 'fruits', 'Orange.ios.js'),
);
const androidVariant = moduleMap.getModule('Orange', 'android');
const androidVariant = hasteModuleMap.getModule('Orange', 'android');
expect(androidVariant).toBe(
path.join('/', 'project', 'fruits', 'Orange.android.js'),
);
Expand All @@ -1560,6 +1558,7 @@ describe('HasteMap', () => {

describe('recovery from duplicate module IDs', () => {
async function setupDuplicates(hm) {
const {fileSystem, hasteModuleMap} = await hm.build();
mockFs[path.join('/', 'project', 'fruits', 'Pear.js')] = `
// Pear!
`;
Expand All @@ -1581,14 +1580,14 @@ describe('HasteMap', () => {
path.join('/', 'project', 'fruits', 'another'),
MOCK_CHANGE_FILE,
);
const {snapshotFS, moduleMap} = await waitForItToChange(hm);
await waitForItToChange(hm);
expect(
snapshotFS.exists(
fileSystem.exists(
path.join('/', 'project', 'fruits', 'another', 'Pear.js'),
),
).toBe(true);
try {
moduleMap.getModule('Pear');
hasteModuleMap.getModule('Pear');
throw new Error('should be unreachable');
} catch (error) {
const {
Expand All @@ -1612,6 +1611,7 @@ describe('HasteMap', () => {
hm_it(
'recovers when the oldest version of the duplicates is fixed',
async hm => {
const {hasteModuleMap} = await hm.build();
await setupDuplicates(hm);
mockFs[path.join('/', 'project', 'fruits', 'Pear.js')] = null;
mockFs[path.join('/', 'project', 'fruits', 'Pear2.js')] = `
Expand All @@ -1632,17 +1632,18 @@ describe('HasteMap', () => {
path.join('/', 'project', 'fruits'),
MOCK_CHANGE_FILE,
);
const {moduleMap} = await waitForItToChange(hm);
expect(moduleMap.getModule('Pear')).toBe(
await waitForItToChange(hm);
expect(hasteModuleMap.getModule('Pear')).toBe(
path.join('/', 'project', 'fruits', 'another', 'Pear.js'),
);
expect(moduleMap.getModule('Pear2')).toBe(
expect(hasteModuleMap.getModule('Pear2')).toBe(
path.join('/', 'project', 'fruits', 'Pear2.js'),
);
},
);

hm_it('recovers when the most recent duplicate is fixed', async hm => {
const {hasteModuleMap} = await hm.build();
await setupDuplicates(hm);
mockFs[path.join('/', 'project', 'fruits', 'another', 'Pear.js')] =
null;
Expand All @@ -1664,11 +1665,11 @@ describe('HasteMap', () => {
path.join('/', 'project', 'fruits', 'another'),
MOCK_CHANGE_FILE,
);
const {moduleMap} = await waitForItToChange(hm);
expect(moduleMap.getModule('Pear')).toBe(
await waitForItToChange(hm);
expect(hasteModuleMap.getModule('Pear')).toBe(
path.join('/', 'project', 'fruits', 'Pear.js'),
);
expect(moduleMap.getModule('Pear2')).toBe(
expect(hasteModuleMap.getModule('Pear2')).toBe(
path.join('/', 'project', 'fruits', 'another', 'Pear2.js'),
);
});
Expand Down
2 changes: 0 additions & 2 deletions packages/metro-file-map/src/flow-types.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ export type CacheManagerFactory = (
export type ChangeEvent = {
logger: ?RootPerfLogger,
eventsQueue: EventsQueue,
snapshotFS: FileSystem,
moduleMap: ModuleMap,
};

export type ChangeEventMetadata = {
Expand Down
20 changes: 0 additions & 20 deletions packages/metro-file-map/src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -799,25 +799,6 @@ export default class HasteMap extends EventEmitter {
return this._worker;
}

_getSnapshot(data: InternalData): {
snapshotFS: FileSystem,
moduleMap: HasteModuleMap,
} {
const rootDir = this._options.rootDir;
return {
snapshotFS: new HasteFS({
files: new Map(data.files),
rootDir,
}),
moduleMap: new HasteModuleMap({
duplicates: new Map(data.duplicates),
map: new Map(data.map),
mocks: new Map(data.mocks),
rootDir,
}),
};
}

_removeIfExists(data: InternalData, relativeFilePath: Path) {
const fileMetadata = data.files.get(relativeFilePath);
if (!fileMetadata) {
Expand Down Expand Up @@ -900,7 +881,6 @@ export default class HasteMap extends EventEmitter {
const changeEvent: ChangeEvent = {
logger: hmrPerfLogger,
eventsQueue,
...this._getSnapshot(data),
};
this.emit('change', changeEvent);
eventsQueue = [];
Expand Down

0 comments on commit b942eca

Please sign in to comment.