Set stackTraceLimit to 0 in fileSystemEntryExists#40444
Merged
amcasey merged 1 commit intomicrosoft:release-4.0from Sep 16, 2020
Merged
Set stackTraceLimit to 0 in fileSystemEntryExists#40444amcasey merged 1 commit intomicrosoft:release-4.0from
amcasey merged 1 commit intomicrosoft:release-4.0from
Conversation
The exception thrown by Node.js's fs.statSync function contains a stack trace that can be expensive to compute. Since this exception isn't used by fileSystemEntryExists, we can safely set Error.stackTraceLimit to 0 without a change in behavior. --- A significant performance improvement was noticed with this change while profiling tsserver on packages within a proprietary monorepo. Specifically, my team saw high self time percentages for Node.js's uvException and handleErrorFromBinding internal functions. These functions are executed within fs.statSync when it fails to find the given path. https://user-images.githubusercontent.com/906558/90183227-220cb800-dd81-11ea-8d61-f41f89481f46.png fs.statSync: https://github.com/nodejs/node/blob/v14.4.0/lib/fs.js#L1030-L1037 handleErrorFromBinding: https://github.com/nodejs/node/blob/v14.4.0/lib/internal/fs/utils.js#L254-L269 uvException: https://github.com/nodejs/node/blob/v14.4.0/lib/internal/errors.js#L390-L443 ## Measurements After adding Error.stackTraceLimit = 0, we saw: - For a large configured project with 12,565 files, tsserver reached the projectLoadingFinish event 48.78% faster. (~46.786s vs ~31.447s) - For a medium project with 7,064 files, tsserver was 25.75% faster. (~20.897s vs ~16.618s) - For a small project with 796 files, tsserver was only a negligible 3.00% faster. (~3.545s vs ~3.442) Measurements were taken on macOS 10.15.6, Node.js 14.4.0, and a recent master commit of TypeScript (610fa28). The average of 3 runs before and after this change were taken. I would normally include .cpuprofile and isolate-*-*-*.log files, but can't post them publicly in this case. If there's any other summaries the TypeScript team would be curious about I can report them. ## fs.statSync Misses Within our monorepo, the fs.statSync misses were mostly searches for alternative file extensions of module imports. - For node_modules imports, a lot of .ts/.tsx lookups failed until the .d.ts file was found. - Within projects with a lot of JSX files, .ts files were looked for before finding the .tsx version. - In the medium scale project mentioned above, a total of 38,515 non-existent files were queried during createProgram.
Member
Author
DanielRosenwasser
approved these changes
Sep 16, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[This is a cherry-pick of #40043]
The exception thrown by Node.js's fs.statSync function contains a stack
trace that can be expensive to compute. Since this exception isn't used
by fileSystemEntryExists, we can safely set Error.stackTraceLimit to 0
without a change in behavior.
A significant performance improvement was noticed with this change while
profiling tsserver on packages within a proprietary monorepo.
Specifically, my team saw high self time percentages for Node.js's
uvException and handleErrorFromBinding internal functions. These
functions are executed within fs.statSync when it fails to find the
given path.
fs.statSync: https://github.com/nodejs/node/blob/v14.4.0/lib/fs.js#L1030-L1037
handleErrorFromBinding: https://github.com/nodejs/node/blob/v14.4.0/lib/internal/fs/utils.js#L254-L269
uvException: https://github.com/nodejs/node/blob/v14.4.0/lib/internal/errors.js#L390-L443
Measurements
After adding Error.stackTraceLimit = 0, we saw:
projectLoadingFinish event 48.78% faster. (~46.786s vs ~31.447s)
(~20.897s vs ~16.618s)
3.00% faster. (~3.545s vs ~3.442)
Measurements were taken on macOS 10.15.6, Node.js 14.4.0, and a recent
master commit of TypeScript (610fa28). The average of 3 runs before and
after this change were taken.
I would normally include .cpuprofile and isolate---*.log files, but
can't post them publicly in this case. If there's any other summaries
the TypeScript team would be curious about I can report them.
fs.statSync Misses
Within our monorepo, the fs.statSync misses were mostly searches for
alternative file extensions of module imports.
.d.ts file was found.
before finding the .tsx version.
non-existent files were queried during createProgram.