Investigate reducing bundle size #11996

Tyriar · 2022-11-11T17:22:43Z

No description provided.

Part of #11996

All the module does is replace ~ with os.homedir(). This change won't reduce bundle size particularly but it simplifies dependencies. Part of #11996

uglify-js doesn't appear to be in the project anymore and it seems that was the only reason it was included based on a comment? Regardless, support is good for named capture group in all browsers/node now. Part of #11996

Support for URL is good in all environments, it was actually only being referenced directly in .node.ts files and it's been available in node since v10. The package itself also has deprecated url.parse with advice to use the standard API instead. It is still imported in tests only via rewiremock. Part of #11996

Tyriar · 2022-11-15T20:22:48Z

A profile when Jupyter activates is interesting, this is on a particularly fast activation (1s) on my fast machine but it's typically around 2-2.5s:

_loadCommonJSModule (load jupyter code) 987.33ms
- readFileSync 97.88ms
_callActivate 50.59ms
activateLegacy 206.24ms - Not sure what the legacy is about here?
- Inversify init 165.33ms Long DI planning #12008
- activateSync 10.20ms
- activate 30.70ms
_loadCommonJSModule (load python code) 126.5ms
activate (python) 13.26ms 👏
refreshEnvironments 879.74ms - What's the spawn about? Is that really locking up the exthost just to read some registry keys?

Tyriar · 2022-11-15T20:35:07Z

I'm thinking this for splitting strategy:

We could create a tsconfig with project references to enforce a strict API to sub-components that can be lazily loaded like interactive-window, this could be done in a more naive way via layer eslint rules
Make all access to interactive window from the main bundle async
Only allow importing interfaces or the strictly defined API from the main bundle. I think these are the main usages we need to worry about wrt the API, plus where ever services from inside interactive-window are injected outside of it
Have a web and a node impl of the object that accesses the interactive window code, the node one would be async as dynamic imports work

DonJayamanne · 2022-11-15T21:21:26Z

refreshEnvironments 879.74ms - What's the spawn about? Is that really locking up the exthost just to read some registry keys?

This should be in the Python extension, they end up spawning Python processes (its part of discovering python environments on the user machine).
One possible speed bump I've thought of in the past is using a worker process for spawning processes , but not sure tha'ts a viable solution or not (ther'es an issue for that).
@karrtikr /cc

& I believe they do end up reading registry keys (at least at one point they did),

Again this is a delay in the Python extension.

karrtikr · 2022-11-16T05:19:15Z

Once Jupyter switches to the new kernel picker, I believe 'refreshEnvironments' need not be called at startup, so this shouldn't be an issue. And yes we do block it on fetching registry keys atm, but asynchronously.

Tyriar · 2022-11-16T16:11:16Z

@karrtikr the profiler shows it's blocking the exthost thread, I'm guessing it's an Electron/Windows thing?

karrtikr · 2022-11-16T16:14:40Z

I think it's a NodeJS thing. Even though it runs in background, practically it seems to be occupying so much clock that it blocks the single threaded node.

DonJayamanne · 2022-11-16T16:35:53Z

think it's a NodeJS thing. Even

based on my knowledge yes this is known , there's a certain part in spawn that is blocking, hence my suggestion to use a node worker, because then the main thread is not blocked at all.
I'm not saying spawn in node is not async, based on my past research I recall that some part of the spawn call is sync (a fraction of it, probably when we're setting up the pipes, or the like, don't remember exactly what it was. I could be wrong, but i did look inot this a few years ago and thatt when I thought of looking into a node worker to aleviate this issue)

msrCrypto is a polyfill for the native APIs which are well supported in browser and node currently. The module also recommends using native APIs when they are available. Fixes #12023 Part of #11996

Support for URL is good in all environments, it was actually only being referenced directly in .node.ts files and it's been available in node since v10. The package itself also has deprecated url.parse with advice to use the standard API instead. It is still imported in tests only via rewiremock. Part of #11996 Co-authored-by: Peng Lyu <penn.lv@gmail.com>

This module was a polyfill which is now well supported so it should be safe to remove it. Part of #11996

This upgrades the node/web packages to use es2020 and downgrades the webviews bundle from esnext because that target changes over time. Part of #11996

It would be great if we could remove this dependency as it modifies global state. Part of #11996

* Explicitly state computeHash type * Remove msrCrypto in favor of native APIs msrCrypto is a polyfill for the native APIs which are well supported in browser and node currently. The module also recommends using native APIs when they are available. Fixes #12023 Part of #11996 * Fix compile issue in webTestReporter.js * Mark node:crypto as an external * require reflect-metadata in smoke tests earlier

* Explicitly state computeHash type * Remove msrCrypto in favor of native APIs msrCrypto is a polyfill for the native APIs which are well supported in browser and node currently. The module also recommends using native APIs when they are available. Fixes #12023 Part of #11996 * Fix compile issue in webTestReporter.js * Mark node:crypto as an external * Introduce a tsconfig.base.json * Move extension tsconfigs into src and extend base * Simplify tsconfig excludes * require reflect-metadata in smoke tests earlier * Fix types in helper.ts when allowJs is removed * Remove allowJs! * Remove unneeded cast

* Explicitly state computeHash type * Remove msrCrypto in favor of native APIs msrCrypto is a polyfill for the native APIs which are well supported in browser and node currently. The module also recommends using native APIs when they are available. Fixes #12023 Part of #11996 * Fix compile issue in webTestReporter.js * Mark node:crypto as an external * Introduce a tsconfig.base.json * Move extension tsconfigs into src and extend base * Simplify tsconfig excludes * require reflect-metadata in smoke tests earlier * Fix types in helper.ts when allowJs is removed * Remove allowJs! * Remove unneeded cast * Support comments in tsconfig telemetry parsing * Dynamic import extension-telemetry module * Add webpack-analyze:node|web npm scripts * Fix node build LinkedMap was removed as tsc was outputting invalid js where the variable map_c was not defined but was used. * Dynamic import svg-to-pdfkit * Skip tests when packaging * Use import for global requires like reflect-metadata * Use commonjs2 again * Remove unneeded webpack resolves in web bundle * Remove resolves in web * Revert "Remove resolves in web" This reverts commit 78a061c. * Dynamic import tcp-port-used * Dynamic import portfinder * Switch back to require for reflect-metadata It may have been already run by another extension in which case it would get clobbered * Move require of reflect metadata to own module This consolidates the any/eslint override/docs in a single file * Fix compile errors in telemetryGenerator.node.ts * Revert "Fix compile errors in telemetryGenerator.node.ts" This reverts commit b6fc84b. * Generate telemetry.csv on mac * Fix lint errors * Fix telemetry test rewiremock usage * Add telemetry reporter is supported check back * Bring back os fallback

DonJayamanne · 2023-10-30T02:30:07Z

~~Closing this as we cannot reduce the bundle size anymore (what ever is shipped is required for now), apart from splitting bundles and making this faster.~~

~Closing this for now.`
Was too premature in closing this issue

We have a few issues for reducing the bundle size:

remove vscode-jsonrpc
remove vsoced-langauage client
remove rxjs
remove port finder, etc

Once we address the above and find a few more, I think then we can close this.

DonJayamanne · 2023-10-31T23:39:25Z

Closing as done, we can remove portfinder separately (there's an issue for that)
rxjs, lodash and a number of other packages have been removed and bundle size reduced significantly.
Overall, compressed VSIX is now more than 300kb smaller

Tyriar added the feature-request Request for new features or functionality label Nov 11, 2022

Tyriar added this to the November 2022 milestone Nov 11, 2022

Tyriar self-assigned this Nov 11, 2022

chrisdias mentioned this issue Nov 14, 2022

Iteration Plan for November 2022 microsoft/vscode#165643

Closed

This was referenced Nov 14, 2022

Remove lodash flatten usage #12010

Merged

Remove _.isObject usage #12011

Merged

Tyriar added a commit that referenced this issue Nov 14, 2022

Remove _.isNil usage

0ba8107

Part of #11996

Tyriar mentioned this issue Nov 14, 2022

Remove _.isNil usage #12013

Merged

Tyriar added a commit that referenced this issue Nov 14, 2022

Manually get png image size

c91d772

Part of #11996

Tyriar mentioned this issue Nov 14, 2022

Manually get png image size #12017

Merged

Tyriar added a commit that referenced this issue Nov 14, 2022

Remove _.isNil usage (#12013)

1bde50f

Part of #11996

Tyriar mentioned this issue Nov 14, 2022

Avoid using innerHTML in error-renderer #12018

Closed

rebornix pushed a commit that referenced this issue Nov 14, 2022

Manually get png image size (#12017)

890dd7a

Part of #11996

Tyriar added a commit that referenced this issue Nov 15, 2022

Remove untildify

36d4cda

All the module does is replace ~ with os.homedir(). This change won't reduce bundle size particularly but it simplifies dependencies. Part of #11996

Tyriar mentioned this issue Nov 15, 2022

Remove untildify #12033

Merged

This was referenced Nov 15, 2022

Remove named-js-group #12035

Merged

Avoid require when initializing logging #12034

Merged

Tyriar mentioned this issue Nov 15, 2022

Remove direct usages of the url package #12036

Merged

Tyriar mentioned this issue Nov 16, 2022

Remove msrCrypto in favor of native APIs #12050

Merged

Tyriar mentioned this issue Nov 18, 2022

Set all tsconfigs to target es2020 #12090

Merged

Tyriar added a commit that referenced this issue Nov 18, 2022

Remove reflect-metadata dependency

abd508d

This module was a polyfill which is now well supported so it should be safe to remove it. Part of #11996

Tyriar added a commit that referenced this issue Nov 18, 2022

Set all tsconfigs to target es2020 (#12090)

b59bb6d

This upgrades the node/web packages to use es2020 and downgrades the webviews bundle from esnext because that target changes over time. Part of #11996

Tyriar added a commit that referenced this issue Nov 18, 2022

Remove reflect-metadata dependency

29d7f45

It would be great if we could remove this dependency as it modifies global state. Part of #11996

This was referenced Nov 18, 2022

Remove reflect-metadata dependency #12103

Closed

Clean up tsconfigs #12107

Merged

Tyriar mentioned this issue Nov 21, 2022

Support dynamic imports in the node bundle #12129

Merged

Tyriar modified the milestones: November 2022, December 2022 Nov 28, 2022

roblourens added the deferred label Jan 27, 2023

roblourens modified the milestones: January 2023, February 2023 Jan 27, 2023

DonJayamanne modified the milestones: February 2023, March 2023 Feb 22, 2023

Tyriar assigned DonJayamanne and unassigned Tyriar Mar 23, 2023

Tyriar modified the milestones: March 2023, April 2023 Mar 23, 2023

DonJayamanne modified the milestones: April 2023, May 2023 Apr 26, 2023

DonJayamanne closed this as completed Oct 30, 2023

DonJayamanne reopened this Oct 30, 2023

DonJayamanne closed this as completed Oct 31, 2023

github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Investigate reducing bundle size #11996

Investigate reducing bundle size #11996

Tyriar commented Nov 11, 2022

Tyriar commented Nov 15, 2022 •

edited

Loading

Tyriar commented Nov 15, 2022 •

edited

Loading

DonJayamanne commented Nov 15, 2022

karrtikr commented Nov 16, 2022 •

edited

Loading

Tyriar commented Nov 16, 2022

karrtikr commented Nov 16, 2022

DonJayamanne commented Nov 16, 2022 •

edited

Loading

DonJayamanne commented Oct 30, 2023 •

edited

Loading

DonJayamanne commented Oct 31, 2023

Investigate reducing bundle size #11996

Investigate reducing bundle size #11996

Comments

Tyriar commented Nov 11, 2022

Tyriar commented Nov 15, 2022 • edited Loading

Tyriar commented Nov 15, 2022 • edited Loading

DonJayamanne commented Nov 15, 2022

karrtikr commented Nov 16, 2022 • edited Loading

Tyriar commented Nov 16, 2022

karrtikr commented Nov 16, 2022

DonJayamanne commented Nov 16, 2022 • edited Loading

DonJayamanne commented Oct 30, 2023 • edited Loading

DonJayamanne commented Oct 31, 2023

Tyriar commented Nov 15, 2022 •

edited

Loading

Tyriar commented Nov 15, 2022 •

edited

Loading

karrtikr commented Nov 16, 2022 •

edited

Loading

DonJayamanne commented Nov 16, 2022 •

edited

Loading

DonJayamanne commented Oct 30, 2023 •

edited

Loading