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

Extension causes high cpu load #118

Open
LaraMateo opened this issue Jul 23, 2024 · 4 comments
Open

Extension causes high cpu load #118

LaraMateo opened this issue Jul 23, 2024 · 4 comments

Comments

@LaraMateo
Copy link

  • Issue Type: Performance
  • Extension Name: pretty-ts-errors
  • Extension Version: 0.5.4
  • OS Version: Darwin arm64 23.5.0
  • VS Code version: 1.91.1

⚠️ Make sure to attach this file from your home-directory:
⚠️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

@kevinramharak
Copy link
Contributor

@LaraMateo Could you attach the file mentioned in the generated report?

@AhmedBaset
Copy link

I have been seeing this issue since the last minor release, here is the file

YoavBls.pretty-ts-errors-unresponsive.cpuprofile.txt

@kevinramharak
Copy link
Contributor

@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:

image

> 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 2531440). 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 😞 .

@yoavbls
Copy link
Owner

yoavbls commented Aug 19, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants