-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Improve frontend startup performance #12936
Improve frontend startup performance #12936
Conversation
@kittaakos : Maybe you are interested in reviewing this one? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good to me. I cannot identify any regressions with the approach and the startup time of frontend contributions is indeed reduced.
Though not as much as advertised, but the initial startup time wasn't as long anyway. Reduced the FrontendApplicationContribution#onStart
time from ~500ms to ~300ms for me.
I measured the times only on my own machine ;) |
471a050
to
88d9495
Compare
I rebased the branch on latest master to solve conflicts in the Changelog. Nothing else changed. |
@sdirix I wasn't fast enough to press the merge button. Please rebase again :) |
The frontend awaits 'initialize', 'configure' and 'onStart' for all frontend contributions. It's therefore very important that no expensive code is run there. By not awaiting expensive operations in - DebugFrontendApplicationContribution.onStart (~300ms) - EditorNavigationContribution.onStart (~36-400ms) - TerminalFrontendContribution.onStart (~100-300ms) the reported startup time without using plugins is reduced by ~400-1000ms which is an improvement of ~20-40%. Contributed on behalf of STMicroelectronics
88d9495
to
4400ff0
Compare
The frontend awaits 'initialize', 'configure' and 'onStart' for all frontend contributions. It's therefore very important that no expensive code is run there.
By not awaiting expensive operations in
the reported startup time without using plugins is reduced by ~400-1000ms which is an improvement of ~20-40%.
Contributed on behalf of STMicroelectronics
What it does
It removes some
await
ed instructions in the critical execution paths which are not really necessary to be awaited. The remainingawait
instruction in thecommon-frontend-contribution
which takes about ~200ms of time to be executed was not removed as this might have some side effects in theory, although in practice I was not able to observe any.Note: I also tried to run all
initialize
,configure
andonStart
phases each in "parallel", usingPromise.all
. However when combined with the remaining changes this actually lead to a slowdown, probably because the Javascript thread is already fully utilized.How to test
Verify that Theia behaves as before. Please have a look at the code paths to see whether something was missed.
You can start Theia with
logLevel=debug
for more extensive logging and verify the reported startup times. For proper logging inproduction
mode, you need to disable class name substitution. This can be achieved by adding the following to thewebpack.config.js
:Review checklist
Reminder for reviewers