Skip to content

Commit

Permalink
Remove flat #files map from TreeFS
Browse files Browse the repository at this point in the history
Summary:
Currently we maintain a `#files` `Map` alongside our tree-structured data.

With the previous diffs in this stack, this is now strictly redundant, and only used for more optimised serialisation, and "optimising" lookups when given a normal path exactly matching a map key.

However, when the map is large these hash table lookups become expensive, often having a net-negative performance impact. The next diff prioves a performance improvement over this stack for all use cases, despite removing `#files`.

This diff removes `#files` and all its uses, temporarily replacing serialisation with an equivalent tree traversal, before we switch to serialising the tree directly in the next diff.

Changelog: Internal

Reviewed By: motiz88

Differential Revision: D46687190

fbshipit-source-id: b143f49693bd822f56615a3f5e7373fa12a63725
  • Loading branch information
robhogan authored and facebook-github-bot committed Jun 24, 2023
1 parent ca024d3 commit 9b9711c
Showing 1 changed file with 16 additions and 29 deletions.
45 changes: 16 additions & 29 deletions packages/metro-file-map/src/lib/TreeFS.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,20 @@ type MixedNode = FileNode | DirectoryNode;

export default class TreeFS implements MutableFileSystem {
+#cachedNormalSymlinkTarkets: WeakMap<FileNode, Path> = new WeakMap();
+#files: FileData;
+#rootDir: Path;
+#rootNode: DirectoryNode = new Map();

constructor({rootDir, files}: {rootDir: Path, files: FileData}) {
this.#rootDir = rootDir;
this.#files = files;
this.bulkAddOrModify(files);
}

getSerializableSnapshot(): FileData {
return new Map(
Array.from(this.#files.entries(), ([k, v]: [Path, FileMetaData]) => [
k,
[...v],
]),
Array.from(
this._metadataIterator(this.#rootNode, {includeSymlinks: true}),
({normalPath, metadata}) => [normalPath, [...metadata]],
),
);
}

Expand Down Expand Up @@ -81,7 +79,10 @@ export default class TreeFS implements MutableFileSystem {
} {
const changedFiles: FileData = new Map(files);
const removedFiles: Set<string> = new Set();
for (const [normalPath, metadata] of this.#files) {
for (const {normalPath, metadata} of this._metadataIterator(
this.#rootNode,
{includeSymlinks: true},
)) {
const newMetadata = files.get(normalPath);
if (newMetadata) {
if ((newMetadata[H.SYMLINK] === 0) !== (metadata[H.SYMLINK] === 0)) {
Expand Down Expand Up @@ -227,10 +228,6 @@ export default class TreeFS implements MutableFileSystem {

getRealPath(mixedPath: Path): ?string {
const normalPath = this._normalizePath(mixedPath);
const metadata = this.#files.get(normalPath);
if (metadata && metadata[H.SYMLINK] === 0) {
return fastPath.resolve(this.#rootDir, normalPath);
}
const result = this._lookupByNormalPath(normalPath, {followLeaf: true});
if (!result || result.node instanceof Map) {
return null;
Expand Down Expand Up @@ -258,8 +255,6 @@ export default class TreeFS implements MutableFileSystem {
}

bulkAddOrModify(addedOrModifiedFiles: FileData): void {
const files = this.#files;

// Optimisation: Bulk FileData are typically clustered by directory, so we
// optimise for that case by remembering the last directory we looked up.
// Experiments with large result sets show this to be significantly (~30%)
Expand All @@ -268,10 +263,6 @@ export default class TreeFS implements MutableFileSystem {
let directoryNode: DirectoryNode;

for (const [normalPath, metadata] of addedOrModifiedFiles) {
if (addedOrModifiedFiles !== files) {
files.set(normalPath, metadata);
}

const lastSepIdx = normalPath.lastIndexOf(path.sep);
const dirname = lastSepIdx === -1 ? '' : normalPath.slice(0, lastSepIdx);
const basename =
Expand All @@ -297,22 +288,22 @@ export default class TreeFS implements MutableFileSystem {
remove(mixedPath: Path): ?FileMetaData {
const normalPath = this._normalizePath(mixedPath);
const result = this._lookupByNormalPath(normalPath, {followLeaf: false});
if (!result || result.node instanceof Map) {
if (!result) {
return null;
}
const {parentNode, canonicalPath, node} = result;

// If node is a symlink, get its metadata from the file map. Otherwise, we
// already have it in the lookup result.
const fileMetadata =
typeof node === 'string' ? this.#files.get(canonicalPath) : node;
if (node instanceof Map) {
throw new Error(`TreeFS: remove called on a directory: ${mixedPath}`);
}
// If node is a symlink, get its metadata from the tuple.
const fileMetadata = node;
if (fileMetadata == null) {
throw new Error(`TreeFS: Missing metadata for ${mixedPath}`);
}
if (parentNode == null) {
throw new Error(`TreeFS: Missing parent node for ${mixedPath}`);
}
this.#files.delete(canonicalPath);
parentNode.delete(path.basename(canonicalPath));
return fileMetadata;
}
Expand Down Expand Up @@ -549,16 +540,12 @@ export default class TreeFS implements MutableFileSystem {
opts: {followLeaf: boolean} = {followLeaf: true},
): ?FileMetaData {
const normalPath = this._normalizePath(filePath);
const metadata = this.#files.get(normalPath);
if (metadata && (!opts.followLeaf || metadata[H.SYMLINK] === 0)) {
return metadata;
}
const result = this._lookupByNormalPath(normalPath, {
followLeaf: opts.followLeaf,
});
if (!result || result.node instanceof Map) {
if (result == null || result.node instanceof Map) {
return null;
}
return this.#files.get(result.canonicalPath);
return result.node;
}
}

0 comments on commit 9b9711c

Please sign in to comment.