Skip to content

Commit 7e67143

Browse files
Simple file I/O optimizations (#48)
* Add top-level test target. * Two disk I/O optimizations to prevent unnecessary repeated reads, and batching reads to avoid back-and-forth in an our of JS. * Next step: switch to async/promises and parallelize bulk reads (e.g. scanning node_modules or walking up for package.json) to fs.promises + Promise.all(). That lets the OS reorder reads and overlap them. Co-authored-by: Kræn Hansen <mail@kraenhansen.dk>
1 parent bd1a809 commit 7e67143

File tree

3 files changed

+134
-54
lines changed

3 files changed

+134
-54
lines changed

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
"build": "tsc --build",
1313
"clean": "tsc --build --clean",
1414
"dev": "tsc --build --watch",
15-
"lint": "eslint ."
15+
"lint": "eslint .",
16+
"test": "npm run test --workspace react-native-node-api-modules --workspace react-native-node-api-cmake --workspace gyp-to-cmake --workspace node-addon-examples"
1617
},
1718
"author": {
1819
"name": "Callstack",

packages/react-native-node-api-modules/src/node/path-utils.test.ts

Lines changed: 87 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import assert from "node:assert/strict";
22
import { describe, it } from "node:test";
33
import path from "node:path";
4+
import fs from "node:fs";
45

56
import {
67
determineModuleContext,
@@ -23,6 +24,66 @@ describe("isNodeApiModule", () => {
2324
assert(isNodeApiModule(path.join(tempDirectoryPath, "addon")));
2425
assert(isNodeApiModule(path.join(tempDirectoryPath, "addon.node")));
2526
});
27+
28+
it("returns false when directory cannot be read due to permissions", (context) => {
29+
const tempDirectoryPath = setupTempDirectory(context, {
30+
"addon.android.node": "",
31+
});
32+
// remove read permissions on directory
33+
fs.chmodSync(tempDirectoryPath, 0);
34+
try {
35+
assert.equal(
36+
isNodeApiModule(path.join(tempDirectoryPath, "addon")),
37+
false
38+
);
39+
} finally {
40+
fs.chmodSync(tempDirectoryPath, 0o700);
41+
}
42+
});
43+
44+
it("throws when module file exists but is not readable", (context) => {
45+
const tempDirectoryPath = setupTempDirectory(context, {
46+
"addon.android.node": "",
47+
});
48+
const candidate = path.join(tempDirectoryPath, "addon.android.node");
49+
// remove read permission on file
50+
fs.chmodSync(candidate, 0);
51+
try {
52+
assert.throws(() => isNodeApiModule(path.join(tempDirectoryPath, "addon")), /Found an unreadable module addon\.android\.node/);
53+
} finally {
54+
fs.chmodSync(candidate, 0o600);
55+
}
56+
});
57+
58+
it("returns false when parent directory does not exist", () => {
59+
// Path to a non-existent directory
60+
const fakePath = path.join(process.cwd(), "no-such-dir", "addon");
61+
assert.equal(isNodeApiModule(fakePath), false);
62+
});
63+
64+
it("recognize .xcframeworks", (context) => {
65+
const tempDirectoryPath = setupTempDirectory(context, {
66+
"addon.xcframework/addon.node": "// This is supposed to be a binary file",
67+
});
68+
assert.equal(isNodeApiModule(path.join(tempDirectoryPath, "addon")), true);
69+
assert.equal(
70+
isNodeApiModule(path.join(tempDirectoryPath, "addon.node")),
71+
true
72+
);
73+
assert.equal(isNodeApiModule(path.join(tempDirectoryPath, "nope")), false);
74+
});
75+
76+
it("throws when one module unreadable but another readable", (context) => {
77+
const tempDirectoryPath = setupTempDirectory(context, {
78+
"addon.android.node": "",
79+
"addon.xcframework": "",
80+
});
81+
const unreadable = path.join(tempDirectoryPath, "addon.android.node");
82+
// only android module is unreadable
83+
fs.chmodSync(unreadable, 0);
84+
assert.throws(() => isNodeApiModule(path.join(tempDirectoryPath, "addon")), /Found an unreadable module addon\.android\.node/);
85+
fs.chmodSync(unreadable, 0o600);
86+
});
2687
});
2788

2889
describe("stripExtension", () => {
@@ -46,20 +107,6 @@ describe("replaceExtensionWithNode", () => {
46107
});
47108
});
48109

49-
describe("isNodeApiModule", () => {
50-
it("recognize .xcframeworks", (context) => {
51-
const tempDirectoryPath = setupTempDirectory(context, {
52-
"addon.xcframework/addon.node": "// This is supposed to be a binary file",
53-
});
54-
assert.equal(isNodeApiModule(path.join(tempDirectoryPath, "addon")), true);
55-
assert.equal(
56-
isNodeApiModule(path.join(tempDirectoryPath, "addon.node")),
57-
true
58-
);
59-
assert.equal(isNodeApiModule(path.join(tempDirectoryPath, "nope")), false);
60-
});
61-
});
62-
63110
describe("determineModuleContext", () => {
64111
it("works", (context) => {
65112
const tempDirectoryPath = setupTempDirectory(context, {
@@ -243,3 +290,29 @@ describe("findNodeApiModulePaths", () => {
243290
]);
244291
});
245292
});
293+
294+
describe("determineModuleContext", () => {
295+
it("should read package.json only once across multiple module paths for the same package", (context) => {
296+
const tempDir = setupTempDirectory(context, {
297+
"package.json": `{ "name": "cached-pkg" }`,
298+
"subdir1/file1.node": "",
299+
"subdir2/file2.node": "",
300+
"subdir1/file1.xcframework": ""
301+
});
302+
let readCount = 0;
303+
const orig = fs.readFileSync;
304+
context.mock.method(fs, "readFileSync", (...args: Parameters<typeof fs.readFileSync>) => {
305+
const [pathArg] = args;
306+
if (typeof pathArg === "string" && pathArg.endsWith("package.json")) {
307+
readCount++;
308+
}
309+
return orig(...args);
310+
});
311+
312+
const ctx1 = determineModuleContext(path.join(tempDir, "subdir1/file1.node"));
313+
const ctx2 = determineModuleContext(path.join(tempDir, "subdir2/file2.node"));
314+
assert.equal(ctx1.packageName, "cached-pkg");
315+
assert.equal(ctx2.packageName, "cached-pkg");
316+
assert.equal(readCount, 1);
317+
});
318+
});

packages/react-native-node-api-modules/src/node/path-utils.ts

Lines changed: 45 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import assert from "node:assert/strict";
22
import path from "node:path";
3-
import fs, { readdirSync } from "node:fs";
3+
import fs from "node:fs";
44
import { findDuplicates } from "./duplicates";
55
import chalk from "chalk";
66
import { packageDirectorySync } from "pkg-dir";
@@ -22,21 +22,37 @@ export type NamingStrategy = {
2222
stripPathSuffix: boolean;
2323
};
2424

25+
// Cache mapping package directory to package name across calls
26+
const packageNameCache = new Map<string, string>();
27+
2528
/**
26-
* @param modulePath The path to the module to check (must be extensionless or end in .node)
27-
* @returns True if a platform specific prebuild exists for the module path.
29+
* @param modulePath Batch-scans the path to the module to check (must be extensionless or end in .node)
30+
* @returns True if a platform specific prebuild exists for the module path, warns on unreadable modules.
31+
* @throws If the parent directory cannot be read, or if a detected module is unreadable.
2832
* TODO: Consider checking for a specific platform extension.
2933
*/
3034
export function isNodeApiModule(modulePath: string): boolean {
31-
// Determine if we're trying to load a Node-API module
32-
// Strip optional .node extension
33-
const candidateBasePath = path.resolve(
34-
path.dirname(modulePath),
35-
path.basename(modulePath, ".node")
36-
);
37-
return Object.values(PLATFORM_EXTENSIONS)
38-
.map((extension) => candidateBasePath + extension)
39-
.some(fs.existsSync);
35+
const dir = path.dirname(modulePath);
36+
const baseName = path.basename(modulePath, ".node");
37+
let entries: string[];
38+
try {
39+
entries = fs.readdirSync(dir);
40+
} catch {
41+
// Cannot read directory: treat as no module
42+
return false;
43+
}
44+
return Object.values(PLATFORM_EXTENSIONS).some(extension => {
45+
const fileName = baseName + extension;
46+
if (!entries.includes(fileName)) {
47+
return false;
48+
}
49+
try {
50+
fs.accessSync(path.join(dir, fileName), fs.constants.R_OK);
51+
return true;
52+
} catch (cause) {
53+
throw new Error(`Found an unreadable module ${fileName}: ${cause}`);
54+
}
55+
});
4056
}
4157

4258
/**
@@ -78,34 +94,24 @@ export function determineModuleContext(
7894
modulePath: string,
7995
originalPath = modulePath
8096
): ModuleContext {
81-
const candidatePackageJsonPath = path.join(modulePath, "package.json");
82-
const parentDirectoryPath = path.dirname(modulePath);
83-
if (fs.existsSync(candidatePackageJsonPath)) {
84-
const packageJsonContent = fs.readFileSync(
85-
candidatePackageJsonPath,
86-
"utf8"
87-
);
88-
const packageJson = JSON.parse(packageJsonContent) as unknown;
89-
assert(
90-
typeof packageJson === "object" && packageJson !== null,
91-
"Expected package.json to be an object"
92-
);
93-
assert(
94-
"name" in packageJson && typeof packageJson.name === "string",
95-
"Expected package.json to have a name"
96-
);
97-
return {
98-
packageName: packageJson.name,
99-
relativePath: normalizeModulePath(
100-
path.relative(modulePath, originalPath)
101-
),
102-
};
103-
} else if (parentDirectoryPath === modulePath) {
104-
// We've reached the root of the filesystem
97+
// Locate nearest package directory
98+
const pkgDir = packageDirectorySync({ cwd: modulePath });
99+
if (!pkgDir) {
105100
throw new Error("Could not find containing package");
106-
} else {
107-
return determineModuleContext(parentDirectoryPath, originalPath);
108101
}
102+
// Read and cache package name
103+
let pkgName = packageNameCache.get(pkgDir);
104+
if (!pkgName) {
105+
const pkg = readPackageSync({ cwd: pkgDir });
106+
assert(typeof pkg.name === "string", "Expected package.json to have a name");
107+
pkgName = pkg.name;
108+
packageNameCache.set(pkgDir, pkgName);
109+
}
110+
// Compute module-relative path
111+
const relPath = normalizeModulePath(
112+
path.relative(pkgDir, originalPath)
113+
);
114+
return { packageName: pkgName, relativePath: relPath };
109115
}
110116

111117
export function normalizeModulePath(modulePath: string) {
@@ -246,7 +252,7 @@ export function findNodeApiModulePaths(
246252
return [];
247253
}
248254
const candidatePath = path.join(fromPath, suffix);
249-
return readdirSync(candidatePath, { withFileTypes: true }).flatMap((file) => {
255+
return fs.readdirSync(candidatePath, { withFileTypes: true }).flatMap((file) => {
250256
if (
251257
file.isFile() &&
252258
file.name === MAGIC_FILENAME &&

0 commit comments

Comments
 (0)