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

Don't await plugin deployment in backend process #13134

Merged
merged 1 commit into from
Dec 21, 2023

Conversation

tortmayr
Copy link
Contributor

@tortmayr tortmayr commented Dec 1, 2023

What it does

  • Don't await initialize method in PluginDeployerContribution sync to continue with backend loading while plugins are deployed
  • Fix wrong performance measurements of resolvePlugins & deployPlugins & in PluginDeployerImpl
  • Improve performance logging in HostedPluginSupport by only logging relevant measurements of sync/load and start plugins (i.e. if the plugin count is 0 just stop the measurement but don`t log)
  • Update I18nPreloadContribution to ensure that we only load the localizations from the back end if the current locale is not equal to the default locale. This fixes an issue where we unnecessarily queried the localization server in the preload step if the locale is explicitly set to "en". (This happens for instance if you change the display language and then switch back to english).

Contributed on behalf of ST Microelectronics

How to test

Everything should work as before. In the example apps the deploy plugin step does not take very long because most of the
default plugins are rather minimal and can deployed very fast.
To see a the real benefit you could add a timeout to the PluginDeployerImpl to simulate a longer plugin deploy process:

    protected async doStart(): Promise<void> {
            await new Promise(resolve => setTimeout(resolve, 3500));
            ...
    }

On the current master the backend process will block and not start the frontend loading until the pluginDeployer is finished.
With this change the fronted is started and loaded right away. Once the plugins are deployed the frontend will be notified and
starts the plugins.

Follow-ups

Review checklist

Reminder for reviewers

- Make `initialize` method in `PluginDeployerContribution` sync to  continue with backend loading while plugins are deployed
- Fix wrong performance measurements of `resolvePlugins` & `deployPlugins` &  in `PluginDeployerImpl`
- Improve performance logging in `HostedPluginSupport` by only logging relevent measurements of `sync/`load` and `start` plugins 
  (i.e. if the plugin count is 0 just stop the measurement but don`t log)
- Update `I18nPreloadContribution` to ensure that we only load the localizations from the backend if the current locale is not equal to the default locale
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

@tortmayr tortmayr merged commit f55710d into eclipse-theia:master Dec 21, 2023
13 of 14 checks passed
@vince-fugnitto vince-fugnitto added this to the 1.45.0 milestone Dec 21, 2023
@JonasHelming JonasHelming mentioned this pull request Dec 26, 2023
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants