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

[perf] Load localizations lazily #12932

Merged
merged 1 commit into from
Oct 26, 2023
Merged

[perf] Load localizations lazily #12932

merged 1 commit into from
Oct 26, 2023

Conversation

msujew
Copy link
Member

@msujew msujew commented Sep 22, 2023

What it does

Related to #12924

Makes the localization entries used in the LocalizationProvider lazy. This is mainly relevant for localizations contributed by language packs, as they are usually split into dozens of files, which takes quite a while to load and parse. Each language pack takes roughly ~50-100ms to load. Having 10+ language packs installed can slow down startup time considerably.

How to test

  1. Load a language pack and assert that it still works as expected.
  2. Install all language packs (see here) and assert that the startup time isn't influenced (at least not noticably).

Follow-ups

Review checklist

Reminder for reviewers

@msujew msujew added performance issues related to performance localization issues related to localization/internalization/nls labels Sep 22, 2023
@msujew msujew mentioned this pull request Sep 22, 2023
11 tasks
@msujew
Copy link
Member Author

msujew commented Sep 22, 2023

cc @kittaakos This should help improve the startup performance in the Arduino IDE. I'd appreciate some feedback :)

@kittaakos
Copy link
Contributor

I checked out the branch and made a quick comparison.

My changeset:

package.json:

diff --git a/package.json b/package.json
index 929fb05554..32b8de6e19 100644
--- a/package.json
+++ b/package.json
@@ -100,11 +100,23 @@
   ],
   "theiaPluginsDir": "plugins",
   "theiaPlugins": {
-    "eclipse-theia.builtin-extension-pack": "https://open-vsx.org/api/eclipse-theia/builtin-extension-pack/1.79.0/file/eclipse-theia.builtin-extension-pack-1.79.0.vsix",
-    "EditorConfig.EditorConfig": "https://open-vsx.org/api/EditorConfig/EditorConfig/0.16.6/file/EditorConfig.EditorConfig-0.16.6.vsix",
-    "dbaeumer.vscode-eslint": "https://open-vsx.org/api/dbaeumer/vscode-eslint/2.4.2/file/dbaeumer.vscode-eslint-2.4.2.vsix",
-    "ms-vscode.js-debug": "https://open-vsx.org/api/ms-vscode/js-debug/1.78.0/file/ms-vscode.js-debug-1.78.0.vsix",
-    "ms-vscode.js-debug-companion": "https://open-vsx.org/api/ms-vscode/js-debug-companion/1.1.2/file/ms-vscode.js-debug-companion-1.1.2.vsix"
+    "vscode-language-pack-bg": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-bg/1.48.3/file/MS-CEINTL.vscode-language-pack-bg-1.48.3.vsix",
+    "vscode-language-pack-cs": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-cs/1.78.0/file/MS-CEINTL.vscode-language-pack-cs-1.78.0.vsix",
+    "vscode-language-pack-de": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-de/1.78.0/file/MS-CEINTL.vscode-language-pack-de-1.78.0.vsix",
+    "vscode-language-pack-es": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-es/1.78.0/file/MS-CEINTL.vscode-language-pack-es-1.78.0.vsix",
+    "vscode-language-pack-fr": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-fr/1.78.0/file/MS-CEINTL.vscode-language-pack-fr-1.78.0.vsix",
+    "vscode-language-pack-hu": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-hu/1.48.3/file/MS-CEINTL.vscode-language-pack-hu-1.48.3.vsix",
+    "vscode-language-pack-it": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-it/1.78.0/file/MS-CEINTL.vscode-language-pack-it-1.78.0.vsix",
+    "vscode-language-pack-ja": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-ja/1.78.0/file/MS-CEINTL.vscode-language-pack-ja-1.78.0.vsix",
+    "vscode-language-pack-ko": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-ko/1.78.0/file/MS-CEINTL.vscode-language-pack-ko-1.78.0.vsix",
+    "vscode-language-pack-nl": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-nl/1.48.3/file/MS-CEINTL.vscode-language-pack-nl-1.48.3.vsix",
+    "vscode-language-pack-pl": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-pl/1.78.0/file/MS-CEINTL.vscode-language-pack-pl-1.78.0.vsix",
+    "vscode-language-pack-pt-BR": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-pt-BR/1.78.0/file/MS-CEINTL.vscode-language-pack-pt-BR-1.78.0.vsix",
+    "vscode-language-pack-ru": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-ru/1.78.0/file/MS-CEINTL.vscode-language-pack-ru-1.78.0.vsix",
+    "vscode-language-pack-tr": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-tr/1.78.0/file/MS-CEINTL.vscode-language-pack-tr-1.78.0.vsix",
+    "vscode-language-pack-uk": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-uk/1.48.3/file/MS-CEINTL.vscode-language-pack-uk-1.48.3.vsix",
+    "vscode-language-pack-zh-hans": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-zh-hans/1.78.0/file/MS-CEINTL.vscode-language-pack-zh-hans-1.78.0.vsix",
+    "vscode-language-pack-zh-hant": "https://open-vsx.org/api/MS-CEINTL/vscode-language-pack-zh-hant/1.78.0/file/MS-CEINTL.vscode-language-pack-zh-hant-1.78.0.vsix"
   },
   "theiaPluginsExcludeIds": [
     "ms-vscode.js-debug-companion",

.vscode/launch.json:

diff --git a/.vscode/launch.json b/.vscode/launch.json
index 00e3862ace..37276a42ef 100644
--- a/.vscode/launch.json
+++ b/.vscode/launch.json
@@ -30,7 +30,6 @@
         ".",
         "--log-level=debug",
         "--hostname=localhost",
-        "--no-cluster",
         "--app-project-path=${workspaceFolder}/examples/electron",
         "--remote-debugging-port=9222",
         "--no-app-auto-install",

HEAD (f038673) of the main branch:

theia_ide2_languages_cluster_mode_HEAD.mp4

From this PR:

theia_ide2_languages_cluster_mode_lazy.mp4

I did the profiling with the built-in profiler in VS Code on macOS 13.5.2, 2,6 GHz 6-Core Intel Core i7, 16 GB, starting the electron example app from the sources in cluster mode. Both profiles show the first second in the backend process.

From this PR, I can select and change the language. The app reloads as expected. Excellent work, Mark

@JonasHelming
Copy link
Contributor

@planger FYI

@msujew msujew force-pushed the msujew/lazy-localization branch from 5f86cfc to 0c3f387 Compare September 26, 2023 12:52
@JonasHelming
Copy link
Contributor

@msujew Do you want to trigger a re-review from @kittaakos ?

@msujew
Copy link
Member Author

msujew commented Oct 26, 2023

@JonasHelming I don't need a second review from @kittaakos, nothing has really changed. I just need an approval to merge this. Let me quickly rebase the PR :)

@msujew msujew force-pushed the msujew/lazy-localization branch from 0c3f387 to 9ed9428 Compare October 26, 2023 11:33
@msujew msujew merged commit c745963 into master Oct 26, 2023
11 of 12 checks passed
@msujew msujew deleted the msujew/lazy-localization branch October 26, 2023 12:10
@github-actions github-actions bot added this to the 1.43.0 milestone Oct 26, 2023
@vince-fugnitto vince-fugnitto modified the milestone: 1.43.0 Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
localization issues related to localization/internalization/nls performance issues related to performance
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants