Skip to content

Commit

Permalink
Use async IO in DiskCacheManager
Browse files Browse the repository at this point in the history
Summary:
Switch from `graceful-fs` synchronous IO in `metro-file-map`'s `DiskCacheManager` to native promise IO.

[`graceful-fs`](https://github.com/isaacs/node-graceful-fs/blob/main/graceful-fs.js) [does not patch](https://github.com/isaacs/node-graceful-fs/tree/main?tab=readme-ov-file#sync-methods) `readFileSync`/`writeFileSync`, so we're not deriving any benefit from `graceful-fs` over `fs` in this instance.

Using async IO is just good practice especially during startup when Metro is doing various things at once.

This sets up lazy writes triggered by events, in the next diff.

Modification to `server-torn-down-test` required because `fs.promises` open differently-labelled handles, but these are unreffed and don't block shutdown.

Changelog: Internal

Reviewed By: vzaidman

Differential Revision: D68722144

fbshipit-source-id: 6e070b76a650f43238719e95bdb2d13600deb3d8
  • Loading branch information
robhogan authored and facebook-github-bot committed Jan 31, 2025
1 parent 71b8a54 commit b4d5251
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 4 deletions.
6 changes: 3 additions & 3 deletions packages/metro-file-map/src/cache/DiskCacheManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type {
} from '../flow-types';

import rootRelativeCacheKeys from '../lib/rootRelativeCacheKeys';
import {readFileSync, writeFileSync} from 'graceful-fs';
import {promises as fsPromises} from 'fs';
import {tmpdir} from 'os';
import path from 'path';
import {deserialize, serialize} from 'v8';
Expand Down Expand Up @@ -67,7 +67,7 @@ export class DiskCacheManager implements CacheManager {

async read(): Promise<?CacheData> {
try {
return deserialize(readFileSync(this._cachePath));
return deserialize(await fsPromises.readFile(this._cachePath));
} catch (e) {
if (e?.code === 'ENOENT') {
// Cache file not found - not considered an error.
Expand All @@ -83,7 +83,7 @@ export class DiskCacheManager implements CacheManager {
{changed, removed}: CacheDelta,
): Promise<void> {
if (changed.size > 0 || removed.size > 0) {
writeFileSync(this._cachePath, serialize(dataSnapshot));
await fsPromises.writeFile(this._cachePath, serialize(dataSnapshot));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,21 @@
* @oncall react_native
*/

import type {BuildParameters} from '../../flow-types';
import type {BuildParameters, CacheData} from '../../flow-types';

import {DiskCacheManager} from '../DiskCacheManager';
import * as path from 'path';
import {serialize} from 'v8';

const mockReadFile = jest.fn();
const mockWriteFile = jest.fn();

jest.mock('fs', () => ({
promises: {
readFile: (...args) => mockReadFile(...args),
writeFile: (...args) => mockWriteFile(...args),
},
}));

const buildParameters: BuildParameters = {
cacheBreaker: '',
Expand Down Expand Up @@ -42,6 +53,10 @@ const defaultConfig = {
};

describe('cacheManager', () => {
beforeEach(() => {
jest.clearAllMocks();
});

test('creates valid cache file paths', () => {
expect(
DiskCacheManager.getCacheFilePath(buildParameters, 'file-prefix', '/'),
Expand Down Expand Up @@ -158,4 +173,43 @@ describe('cacheManager', () => {
cacheManager2.getCacheFilePath(),
);
});

test('reads a cache file and deserialises its contents', async () => {
const cacheManager = new DiskCacheManager({buildParameters}, defaultConfig);
mockReadFile.mockResolvedValueOnce(serialize({foo: 'bar'}));
const cache = await cacheManager.read();
expect(mockReadFile).toHaveBeenCalledWith(cacheManager.getCacheFilePath());
expect(cache).toEqual({foo: 'bar'});
});

test('serialises and writes a cache file', async () => {
const cacheManager = new DiskCacheManager({buildParameters}, defaultConfig);
const data: CacheData = {
clocks: new Map([['foo', 'bar']]),
fileSystemData: new Map(),
plugins: new Map(),
};
await cacheManager.write(data, {
changed: new Map(),
removed: new Set(['foo']),
});
expect(mockWriteFile).toHaveBeenCalledWith(
cacheManager.getCacheFilePath(),
serialize(data),
);
});

test('does not write when there have been no changes', async () => {
const cacheManager = new DiskCacheManager({buildParameters}, defaultConfig);
await cacheManager.write(
{
clocks: new Map([['foo', 'bar']]),
fileSystemData: new Map(),
plugins: new Map(),
},
// Empty delta
{changed: new Map(), removed: new Set()},
);
expect(mockWriteFile).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ describe('Server torn down test', () => {
'RANDOMBYTESREQUEST',
'Timeout',
'TickObject',
'FILEHANDLE',
'FILEHANDLECLOSEREQ',
'FSREQCALLBACK',
'FSREQPROMISE',
'FSEVENTWRAP',
Expand Down

0 comments on commit b4d5251

Please sign in to comment.