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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 馃悰: bundleErrors.json is a bad idea for perf #15

Closed
YassinEldeeb opened this issue Apr 28, 2022 · 2 comments
Closed

Bug 馃悰: bundleErrors.json is a bad idea for perf #15

YassinEldeeb opened this issue Apr 28, 2022 · 2 comments

Comments

@YassinEldeeb
Copy link
Contributor

I've noticed that you've added a script in apps/vscode to bundle the errors into one Gigantic JSON file which will eventually be shipped with the extension.

And then you read this file which will eventually be Gigantic in src/extension.ts as the following:

import * as bundleErrors from "./bundleErrors.json";

That is probably going to be problematic in the future, cause the entire JSON file is loaded into memory as a static item that lives till the program exits.

That means that first of all, the extension startup is gonna be relatively slow cause It has to read the entire JSON file which is a lot of I/O, and then parse the content.

Secondly, the parsed JSON data is held in memory for the program's lifetime even though the user just needs a few fields from it at a particular time(avg. ~ 10).

It's best to stick to the previous workflow of just reading/parsing files only when needed (Ex: 1006.md & 1009.md) cause It does have a lot of advantages:

  • Faster startup.
  • Less I/O operations.
  • Less memory usage.
  • When you read a file say 1006.md and parse Its content and pass it to vscode to show it to the user, the held memory for that parsed file content will be automatically cleaned up by the garbage collector at the end of each loop iteration cause It's not a static item as if you use an import statement.
@mattpocock
Copy link
Owner

We can't use the previous flow (using the file system) for the VSCode extension because we don't have access to the file system in all contexts when the VSCode extension runs (for instance, in the browser).

I also think we'll have maybe hundreds of records, which feels perfectly fine to load into memory.

@YassinEldeeb
Copy link
Contributor Author

YassinEldeeb commented Apr 28, 2022

I totally forgot about VSC for the web, fair point!

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

2 participants