Skip to content

Conversation

kevinramharak
Copy link
Collaborator

@kevinramharak kevinramharak commented Oct 10, 2025

I have had this idea for a long time, finally got around to implementing it.

This implements:

  • Addition of a logger and a LogOutputChannel named Pretty TypeScript Errors
  • A measure method for the logger to measure the execution time of certain code paths
  • changes uriStore to Map instead of a Record.
  • changes .filter.forEach(() => items.push()) to a single reduce to avoid the double iteration
  • a few early returns and moving of statements beyond those returns to do slightly less work if its not needed.
  • clear the cache and uriStore when the extension is deactivated to clean up resources.
  • increase cache size from 100 to 1000
  • move cache size and supported diagnostic sources to constants

Notes:

  • Most insignificant logs happen on the trace log level, VSCode has to be told to set its log level to trace to collect all logs. Since most of these measurements are < 1ms they are very unlikely to be the cause of our performance issues.
  • During development and testing I noticed onDidChangeDiagnostics fires a lot, most of the time with 0 uri's. No idea why vscode triggers the event so often, when nothing actually changed. The execution time is < 1ms so its not the cause of the issue.
  • The 'focussed' file triggers onDidChangeDiagnostics with the uri of the focussed file very often. Even if there are no difference in diagnostics. This could be on the side of the TS language server where they alter the diagnostics collection, even if the collection stays identical. The execution time is < 1ms so this is also not the cause of the issue.
  • We may have to also call dispose on the logger, but since we want to preserve the logs in case of a crash or when the extension hangs, I'm not sure if that will also wipe the log.
  • After doing a bunch of profiling on large repo's and test repo's I increased the cache size, as far as I can observe the performance issues are caused by vscode triggering the event with 0 actual changes to the diagnostics and the cache not being used because of formattedMessage cache is obsolete when size > 100 #104. This would only happen when there are a large amount of diagnostics, so its kind of hard to emulate, as that does not happen often.
  • We can also try and add configuration options for users to change the CACHE_SIZE_MAX and supportedDiagnosticSources. This could be useful to recommend to users, and see if that fixes their performance issues.

Some screenshots:
image

image

@kevinramharak kevinramharak force-pushed the feature/add-logging-and-performance-measurement branch from 24d2aca to 153e11c Compare October 13, 2025 09:37
@kevinramharak kevinramharak marked this pull request as draft October 13, 2025 11:53
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

Successfully merging this pull request may close these issues.

1 participant