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

Repeated package.json reads in ts-loader ts api wrapper compared to tsc --build #42670

Closed
JasonKleban opened this issue Feb 5, 2021 · 4 comments · Fixed by #43700
Closed
Assignees
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.

Comments

@JasonKleban
Copy link

I'm noticing extreme performance problems comparing a tsc --build to webpack with ts-loader of the same project. Sysinternals Process Monitor is showing about 11k file reads from a 1 minute tsc --build and nearly 10x ReadFile events through ts-loader, with matchingly awful CPU and memory work before an ultimately successful build hours later. I am pursuing this as a ts-loader optimization problem, but I'm not sure if the ts api gives enough control to succeed.

const { path: candidate, parts } = normalizePathAndParts(combinePaths(containingDirectory, moduleName));
const resolved = nodeLoadModuleByRelativeName(extensions, candidate, /*onlyRecordFailures*/ false, state, /*considerPackageJson*/ true);
// Treat explicit "node_modules" import as an external library import.
return resolved && toSearchResult({ resolved, isExternalLibraryImport: contains(parts, "node_modules") });

Perhaps it's exactly what it needs to do, but this /*considerPackageJson*/ true here is causing repeated reads/parsing of package.json files from dependencies in a row. I can override the injected readFile() and possibly avoid the read with a cache, but I don't think we can avoid the json parse because that is done internal to typescript.

image

image

Common dependencies with many internal .d.ts files are costing 6000+ read/parse of their package.json files in a run, with a long tail down to single reads.

Here is the call to compiler.resolveModuleName()

https://github.com/TypeStrong/ts-loader/blob/b8a70f91aa4a450603342e62b8c03bdd09c7a979/src/servicesHost.ts#L1175-L1199

Here is the injected readFile that is called back to read the contents of many package.json again and again, with each relative path import of a .d.ts file within its library:

https://github.com/TypeStrong/ts-loader/blob/b8a70f91aa4a450603342e62b8c03bdd09c7a979/src/servicesHost.ts#L96-L104

Once the packageJsonInfo is extracted, I suspect additional waste but one step at a time.

CC @sheetalkamat

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Feb 5, 2021
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.3.0 milestone Feb 5, 2021
@JasonKleban
Copy link
Author

@sheetalkamat @RyanCavanaugh I created a self-contained repro for demonstrating this behavior here: https://github.com/JasonKleban/practices-2021

@JasonKleban
Copy link
Author

JasonKleban commented Feb 18, 2021

When comparing the output of traceResolutions: true between the two build methods of the linked sample repo, tsc has 898 lines of output while the webpack & ts=loader version generates 6201 lines. Here is the first significant divergence and it may contain a clue?

image

and this is the next difference suggesting that tsc is getting a cache hit whereas ts-loader way is not attempting that (maybe, but perhaps the diff proximity is misleading here):

image

@sheetalkamat
Copy link
Member

sheetalkamat commented Apr 9, 2021

This is a multi repo issue and i suggest we fix this with:

  1. Module resolution to cache readJson as well as getPackageJsonInfo and handle those things. Need to see when and which kind of invalidation should happen in watch and tsserver mode.
  2. ts-loader could use module resolution cache that gets cleared like its other cache.
  3. API in typescript to provide a way for ts-loader to use efficient module resolution cache we have in ts build state where if module resolution options dont change the module resolution cache is reused.

Also 1 might not be needed with 2 and 3

@sheetalkamat
Copy link
Member

@JasonKleban i have PR #43700 and TypeStrong/ts-loader#1287 which i verified with your sample repro.
Can you check if the builds from these two help with your scenario and report. Thanks

@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
5 participants