Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve resolver performance #6328

Merged
merged 1 commit into from
May 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/core/utils/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export type ConfigOutput = {|

export type ConfigOptions = {|
parse?: boolean,
parser?: string => any,
|};

const configCache = new LRU<FilePath, ConfigOutput>({max: 500});
Expand Down Expand Up @@ -87,7 +88,7 @@ export async function loadConfig(
if (parse === false) {
config = configContent;
} else {
let parse = getParser(extname);
let parse = opts?.parser ?? getParser(extname);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That isn't part of the configCache cache key. But I also can't think of a case where you would want to parse a single file with 2 different parsers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's hope not 😉

config = parse(configContent);
}

Expand Down
61 changes: 42 additions & 19 deletions packages/utils/node-resolver-core/src/NodeResolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
normalizeSeparators,
findAlternativeNodeModules,
findAlternativeFiles,
loadConfig,
} from '@parcel/utils';
import ThrowableDiagnostic, {
generateJSONCodeHighlights,
Expand Down Expand Up @@ -645,6 +646,13 @@ export default class NodeResolver {
ctx.invalidateOnFileChange.add(file);
let pkg = JSON.parse(json);

await this.processPackage(pkg, file, dir);

this.packageCache.set(file, pkg);
return pkg;
}

async processPackage(pkg: InternalPackageJSON, file: string, dir: string) {
pkg.pkgfile = file;
pkg.pkgdir = dir;

Expand All @@ -656,9 +664,6 @@ export default class NodeResolver {
delete pkg.source;
}
}

this.packageCache.set(file, pkg);
return pkg;
}

getPackageEntries(
Expand Down Expand Up @@ -798,18 +803,27 @@ export default class NodeResolver {
return null;
}

let pkgKeys = ['source', 'alias'];
if (env.isBrowser()) pkgKeys.push('browser');
if (pkg.source && !Array.isArray(pkg.source)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making this function use static references rather than dynamic property accesses seems to improve performance quite a bit

let alias = await this.getAlias(filename, pkg, pkg.source);
if (alias != null) {
return alias;
}
}

if (pkg.alias) {
let alias = await this.getAlias(filename, pkg, pkg.alias);
if (alias != null) {
return alias;
}
}

for (let pkgKey of pkgKeys) {
let pkgKeyValue = pkg[pkgKey];
if (!Array.isArray(pkgKeyValue)) {
let alias = await this.getAlias(filename, pkg, pkgKeyValue);
if (alias != null) {
return alias;
}
if (pkg.browser && env.isBrowser()) {
let alias = await this.getAlias(filename, pkg, pkg.browser);
if (alias != null) {
return alias;
}
}

return null;
}

Expand Down Expand Up @@ -911,7 +925,7 @@ export default class NodeResolver {
return alias;
}

findPackage(
async findPackage(
sourceFile: string,
ctx: ResolverContext,
): Promise<InternalPackageJSON | null> {
Expand All @@ -921,17 +935,26 @@ export default class NodeResolver {
});

// Find the nearest package.json file within the current node_modules folder
let dir = path.dirname(sourceFile);
let pkgFile = this.fs.findAncestorFile(
let res = await loadConfig(
this.fs,
sourceFile,
['package.json'],
dir,
this.projectRoot,
// By default, loadConfig uses JSON5. Use normal JSON for package.json files
// since they don't support comments and JSON.parse is faster.
{parser: JSON.parse},
);
if (pkgFile) {
return this.readPackage(path.dirname(pkgFile), ctx);

if (res != null) {
let file = res.files[0].filePath;
let dir = path.dirname(file);
ctx.invalidateOnFileChange.add(file);
let pkg = res.config;
await this.processPackage(pkg, file, dir);
return pkg;
}

return Promise.resolve(null);
return null;
}

async loadAlias(
Expand Down
3 changes: 3 additions & 0 deletions packages/utils/node-resolver-core/test/resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import path from 'path';
import assert from 'assert';
import nullthrows from 'nullthrows';
import {ncp, overlayFS, outputFS} from '@parcel/test-utils';
import {loadConfig as configCache} from '@parcel/utils';

const rootDir = path.join(__dirname, 'fixture');

Expand Down Expand Up @@ -67,6 +68,8 @@ describe('resolver', function() {
mainFields: ['browser', 'source', 'module', 'main'],
extensions: ['.js', '.json'],
});

configCache.clear();
});

describe('file paths', function() {
Expand Down