Open LaraMateo opened 3 months ago
@LaraMateo Could you attach the file mentioned in the generated report?
I have been seeing this issue since the last minor release, here is the file
@AhmedBaset Thanks, that helps a lot.
@yoavbls I used https://lahmatiy.github.io/cpupro as it gives a pretty good overview of cpuprofile's.
I also ran a production build on the v0.6.0
with the sourcemaps enabled. Then using npx source-map-cli resolve <line> <col> --context 3
(see https://www.npmjs.com/package/source-map-cli) to resolve it to the source code, we finally have some way of figuring out where the extension seems to stall:
NOTE: the sourcemaps generated vs the cpuprofile seems to be off by 1 line, not sure why, but adding 1 to the line number (243 instead of 242). I'm not 100% sure if thats correct, if i try to apply the same idea to #118 it doesnt really seem to map correctly. But for the provided cpu profile I seem to be able to realiably map the whole stack trace up the call chain, so I am pretty confident.
If my attempt of mapping the cpuprofile to the sourcemap is correct, it seems like an abnormal amount of time is spent during the function convertToValidType
, NQ
in the sourcemap:
> npx source-map-cli resolve dist/extension.js.map --context 3 243 146
Maps to ../src/format/formatTypeBlock.ts:54:28 (type)
}
}
const convertToValidType = (type: string) =>
^
`type x = ${type
// Add missing parentheses when the type ends with "...""
.replace(/(.*)\.\.\.$/, (_, p1) => addMissingParentheses(p1))
The source of the function:
const convertToValidType = (type: string) =>
`type x = ${type
// Add missing parentheses when the type ends with "...""
.replace(/(.*)\.\.\.$/, (_, p1) => addMissingParentheses(p1))
// Replace single parameter function destructuring because it's not a valid type
// .replaceAll(/\((\{.*\})\:/g, (_, p1) => `(param: /* ${p1} */`)
// Change `(...): return` which is invalid to `(...) => return`
.replace(/^(\(.*\)): /, (_, p1) => `${p1} =>`)
.replaceAll(/... (\d{0,}) more .../g, (_, p1) => `___${p1}MORE___`)
.replaceAll(/... (\d{0,}) more ...;/g, (_, p1) => `___MORE___: ${p1};`)
.replaceAll("...;", "___KEY___: ___THREE_DOTS___;")
.replaceAll("...", "__THREE_DOTS__")};`;
I did some manual testing to see if there is a regex DOS somewhere hiding in here, but I can't really find a reproducible case. The only somewhat shady regex might be the first one.
It uses the replacer function addMissingParentheses
, which uses a stack and a while loop that doesnt end until the stack is empty. Maybe that loop never ends in some edge case? But I'm not sure since function call doesn't seem to show on the profiler.
As I mentioned, i tried to do the same method to #102 but for some reason the generated source map does not seem to work for the build at the v0.5.4 branch (theres no tag btw, its hidden in this commit 2531440478077fd3f3335e3e3d1b92c9679eb1bc). It does seem that the cases are unrelated though, the stalling function XX
in that cpuprofile seems to point the prettier prettify
function that uses the typescript parser.
Lots of text and research, but not really anywhere yet, I feel like these issues are going to be impossible to solve without proper reproducible cases 😞 .
Great work Kevin! Thank you!
I agree it'll be impossible to know for sure unless we'll have reproduction but the addMissingParentheses
is a pretty direction to investigate.
I'm not sure if it can get stuck in the while loop because it's popping the stack of the open parentheses but there are also some regexes that could be expansive, it may be that
Performance
pretty-ts-errors
0.5.4
Darwin arm64 23.5.0
1.91.1
:warning: Make sure to attach this file from your home-directory: :warning:
file:///var/folders/mv/15wtxvc57854ydjbmr1p670r0000gq/T/YoavBls.pretty-ts-errors-unresponsive.cpuprofile.txt
Find more details here: https://github.com/microsoft/vscode/wiki/Explain-extension-causes-high-cpu-load