Skip to content

Commit 877b64e

Browse files
robhoganmeta-codesync[bot]
authored andcommitted
Optimize PackageCache: use Map/Set, cache null results, eliminate per-hit allocations
Summary: Optimize PackageCache: use Map/Set, cache null results, eliminate per-hit allocations ## Summary Refactors `PackageCache` with three optimizations that together yield a **19% speedup** (median) on `getPackageForModule` calls during `js1 build buckfiles`, despite switching from `Object.create(null)` to `Map`/`Set`. ### Changes 1. **Use `Map`/`Set` and `#private` fields** — Replaces `Object.create(null)` dict objects and `_`-prefixed fields with `Map`/`Set` and `#` private class fields for proper encapsulation. In isolation this is ~8-12% slower per-operation than null-prototype objects, but enables the optimizations below. 2. **Single-map result cache** — Replaces the two-map lookup pattern (`_packagePathAndSubpathByModulePath` → `_packageCache`) with a single `#resultByModulePath` map that caches the pre-built result object. Cache hits go from two `Map.get` calls + one object allocation to a single `Map.get` returning the cached object directly. This eliminates ~2.5M object allocations per build. 3. **Cache null results** — Previously, when `getClosestPackage`/`hierarchicalLookup` returned `null` (no enclosing `package.json`), the result was not cached, causing the same expensive tree walk on every repeat query. Trace analysis revealed that 123,017 unique paths (76% of all unique paths) have no enclosing `package.json`, and they account for 3,048,485 total calls (99% of call volume). Caching null results eliminates 413,138 redundant `hierarchicalLookup` calls, boosting the hit rate from 81% to 95%. 4. **Readonly return types** — `getPackage` and `getPackageForModule` now return `Readonly<>` types, preventing consumers from accidentally mutating cached objects. Reviewed By: huntie Differential Revision: D98745578 fbshipit-source-id: 3247bf39233c7fc556708fd8e2a0afd155b7a991
1 parent 421a4e6 commit 877b64e

File tree

3 files changed

+502
-110
lines changed

3 files changed

+502
-110
lines changed

packages/metro/src/node-haste/PackageCache.js

Lines changed: 88 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -12,137 +12,138 @@
1212
import type {PackageJson} from 'metro-resolver/private/types';
1313

1414
import {readFileSync} from 'fs';
15-
import {dirname} from 'path';
15+
import {dirname, sep} from 'path';
1616

1717
type GetClosestPackageFn = (absoluteFilePath: string) => ?{
1818
packageJsonPath: string,
1919
packageRelativePath: string,
2020
};
2121

22+
type PackageForModule = Readonly<{
23+
packageJson: PackageJson,
24+
rootPath: string,
25+
packageRelativePath: string,
26+
}>;
27+
2228
export class PackageCache {
23-
_getClosestPackage: GetClosestPackageFn;
24-
_packageCache: {
25-
[filePath: string]: {
29+
#getClosestPackage: GetClosestPackageFn;
30+
#packageCache: Map<
31+
string,
32+
{
2633
rootPath: string,
2734
packageJson: PackageJson,
2835
},
29-
__proto__: null,
30-
...
31-
};
32-
// Cache for "closest package.json" queries by module path.
33-
_packagePathAndSubpathByModulePath: {
34-
[filePath: string]: ?{
35-
packageJsonPath: string,
36-
packageRelativePath: string,
37-
},
38-
__proto__: null,
39-
...
40-
};
41-
// The inverse of _packagePathByModulePath.
42-
_modulePathsByPackagePath: {
43-
[filePath: string]: Set<string>,
44-
__proto__: null,
45-
...
46-
};
36+
>;
37+
// Single cache: module path → pre-built result object, or null (no allocation on hit)
38+
#resultByModulePath: Map<string, PackageForModule | null>;
39+
// Reverse index for invalidation: package.json path → set of module paths
40+
#modulePathsByPackagePath: Map<string, Set<string>>;
41+
// Module paths that resolved to no package.json (null), for invalidation
42+
#modulePathsWithNoPackage: Set<string>;
4743

4844
constructor(options: {getClosestPackage: GetClosestPackageFn, ...}) {
49-
this._getClosestPackage = options.getClosestPackage;
50-
this._packageCache = Object.create(null);
51-
this._packagePathAndSubpathByModulePath = Object.create(null);
52-
this._modulePathsByPackagePath = Object.create(null);
45+
this.#getClosestPackage = options.getClosestPackage;
46+
this.#packageCache = new Map();
47+
this.#resultByModulePath = new Map();
48+
this.#modulePathsByPackagePath = new Map();
49+
this.#modulePathsWithNoPackage = new Set();
5350
}
5451

55-
getPackage(filePath: string): {
52+
getPackage(filePath: string): Readonly<{
5653
rootPath: string,
5754
packageJson: PackageJson,
58-
} {
59-
if (!this._packageCache[filePath]) {
60-
this._packageCache[filePath] = {
55+
}> {
56+
let cached = this.#packageCache.get(filePath);
57+
if (cached == null) {
58+
cached = {
6159
rootPath: dirname(filePath),
6260
packageJson: JSON.parse(readFileSync(filePath, 'utf8')),
6361
};
62+
this.#packageCache.set(filePath, cached);
6463
}
65-
return this._packageCache[filePath];
64+
return cached;
6665
}
6766

68-
getPackageForModule(absoluteModulePath: string): ?{
69-
packageJson: PackageJson,
70-
rootPath: string,
71-
packageRelativePath: string,
72-
} {
73-
let packagePathAndSubpath =
74-
this._packagePathAndSubpathByModulePath[absoluteModulePath];
75-
if (
76-
packagePathAndSubpath &&
77-
this._packageCache[packagePathAndSubpath.packageJsonPath]
78-
) {
79-
const {rootPath, packageJson} =
80-
this._packageCache[packagePathAndSubpath.packageJsonPath];
81-
return {
82-
packageJson,
83-
rootPath,
84-
packageRelativePath: packagePathAndSubpath.packageRelativePath,
85-
};
67+
getPackageForModule(absoluteModulePath: string): ?PackageForModule {
68+
const cached = this.#resultByModulePath.get(absoluteModulePath);
69+
70+
// Distinguish between `null` (positively no closest package) and
71+
// `undefined` (no cached result yet)
72+
// eslint-disable-next-line lint/strictly-null
73+
if (cached !== undefined) {
74+
return cached;
8675
}
8776

88-
packagePathAndSubpath = this._getClosestPackage(absoluteModulePath);
89-
if (!packagePathAndSubpath) {
77+
const closest = this.#getClosestPackage(absoluteModulePath);
78+
if (closest == null) {
79+
this.#resultByModulePath.set(absoluteModulePath, null);
80+
this.#modulePathsWithNoPackage.add(absoluteModulePath);
9081
return null;
9182
}
9283

93-
const packagePath = packagePathAndSubpath.packageJsonPath;
84+
const packagePath = closest.packageJsonPath;
9485

95-
this._packagePathAndSubpathByModulePath[absoluteModulePath] =
96-
packagePathAndSubpath;
97-
const modulePaths =
98-
this._modulePathsByPackagePath[packagePath] ?? new Set();
86+
// Track module→package for invalidation
87+
let modulePaths = this.#modulePathsByPackagePath.get(packagePath);
88+
if (modulePaths == null) {
89+
modulePaths = new Set();
90+
this.#modulePathsByPackagePath.set(packagePath, modulePaths);
91+
}
9992
modulePaths.add(absoluteModulePath);
100-
this._modulePathsByPackagePath[packagePath] = modulePaths;
10193

10294
const pkg = this.getPackage(packagePath);
103-
10495
if (pkg == null) {
10596
return null;
10697
}
10798

108-
const {rootPath, packageJson} = pkg;
109-
110-
return {
111-
packageJson,
112-
packageRelativePath: packagePathAndSubpath.packageRelativePath,
113-
rootPath,
99+
// Cache the pre-built result object — no allocation on future hits
100+
const result: PackageForModule = {
101+
packageJson: pkg.packageJson,
102+
packageRelativePath: closest.packageRelativePath,
103+
rootPath: pkg.rootPath,
114104
};
105+
this.#resultByModulePath.set(absoluteModulePath, result);
106+
return result;
115107
}
116108

117109
invalidate(filePath: string) {
118-
if (this._packageCache[filePath]) {
119-
delete this._packageCache[filePath];
120-
}
121-
const packagePathAndSubpath =
122-
this._packagePathAndSubpathByModulePath[filePath];
123-
if (packagePathAndSubpath) {
124-
// filePath is a module inside a package.
125-
const packagePath = packagePathAndSubpath.packageJsonPath;
126-
delete this._packagePathAndSubpathByModulePath[filePath];
127-
// This change doesn't invalidate any cached "closest package.json"
128-
// queries for the package's other modules. Clean up only this module.
129-
const modulePaths = this._modulePathsByPackagePath[packagePath];
130-
if (modulePaths) {
131-
modulePaths.delete(filePath);
132-
if (modulePaths.size === 0) {
133-
delete this._modulePathsByPackagePath[packagePath];
110+
this.#packageCache.delete(filePath);
111+
112+
// Clean up any cached result for this module path (including null).
113+
// Derive the package.json path from the cached result to clean up the
114+
// reverse index.
115+
const cachedResult = this.#resultByModulePath.get(filePath);
116+
this.#resultByModulePath.delete(filePath);
117+
this.#modulePathsWithNoPackage.delete(filePath);
118+
119+
if (cachedResult != null) {
120+
const packagePath = cachedResult.rootPath + sep + 'package.json';
121+
const modules = this.#modulePathsByPackagePath.get(packagePath);
122+
if (modules != null) {
123+
modules.delete(filePath);
124+
if (modules.size === 0) {
125+
this.#modulePathsByPackagePath.delete(packagePath);
134126
}
135127
}
136128
}
137-
if (this._modulePathsByPackagePath[filePath]) {
138-
// filePath is a package. This change invalidates all cached "closest
139-
// package.json" queries for modules inside this package.
140-
const modulePaths = this._modulePathsByPackagePath[filePath];
129+
130+
// If filePath is a package.json, invalidate all module lookups pointing to it
131+
const modulePaths = this.#modulePathsByPackagePath.get(filePath);
132+
if (modulePaths != null) {
141133
for (const modulePath of modulePaths) {
142-
delete this._packagePathAndSubpathByModulePath[modulePath];
134+
this.#resultByModulePath.delete(modulePath);
135+
}
136+
this.#modulePathsByPackagePath.delete(filePath);
137+
}
138+
139+
// If a package.json was created, modified, or deleted, invalidate all
140+
// null-cached module results, since modules that previously had no
141+
// enclosing package.json may now resolve to this one.
142+
if (filePath.endsWith(sep + 'package.json')) {
143+
for (const modulePath of this.#modulePathsWithNoPackage) {
144+
this.#resultByModulePath.delete(modulePath);
143145
}
144-
modulePaths.clear();
145-
delete this._modulePathsByPackagePath[filePath];
146+
this.#modulePathsWithNoPackage.clear();
146147
}
147148
}
148149
}

0 commit comments

Comments
 (0)