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

Improve launch performance by delay loading NavigationView #1213

Merged
merged 14 commits into from
May 19, 2020

Conversation

joseartrivera
Copy link
Contributor

@joseartrivera joseartrivera commented May 7, 2020

Partially addresses #209

Description of the changes:

  • This PR delay loads NavigationView until after the rest of the app is loaded and the UI is responsive.
  • Removed Header and AoT button from NavView footer to allow it to be loaded independently of NavView
    - Delay loads NavigationView by placing a placeholder Hamburger button that loads the full NavigationView UI when invoked.
  • Added logic to load NavView when hot keys are used to allow existing hot keys to still work (Alt + 1, 2, 3, 4 and Ctrl +E).
  • Delay loads GraphEngine DLL until the code needs it

How changes were validated:

I used the MeasureAppStartupTime tool created by @mcooley to launch the app repeatedly and measure startup performance.

There is roughly a ~16% decrease in start up time with these changes when measuring locally on my dev machine. The scenario tested is launching the Calculator app into standard mode at the default launch size (smallest possible).

Launch perf with the above circumstances before this PR:
Average: 455.8604766666667 ms

Launch perf with the above circumstances after this PR:
Average: 380.44501666666656 ms

@joseartrivera joseartrivera requested a review from rudyhuyn May 7, 2020 20:48
@rudyhuyn
Copy link
Contributor

rudyhuyn commented May 7, 2020

Good improvement, but I'm curious if Delay loads NavigationView by placing a placeholder Hamburger button that loads the full NavigationView UI when invoked. is necessary and if we can only delay-load a sub-part of it.

Can you try without delay loading the navigationView but removing the check (read a registry key) related to Graphing Calculator mode and compare the perf? (I suppose it's the main factor)

@joseartrivera
Copy link
Contributor Author

Good improvement, but I'm curious if Delay loads NavigationView by placing a placeholder Hamburger button that loads the full NavigationView UI when invoked. is necessary and if we can only delay-load a sub-part of it.

Can you try without delay loading the navigationView but removing the check (read a registry key) related to Graphing Calculator mode and compare the perf? (I suppose it's the main factor)

It's a good idea, I gave it a shot but didn't see too much of a difference, although I could see how it may add delay in certain machines. I think the main issue with perf related to NavView is the amount of UI elements it introduces to the visual tree.

Here are the measurements, re-ran all 3 scenarios, seems like my PC was slightly faster overall than when I first ran them in the PR, but the difference between the changes in this PR and master is still roughly ~75ms.

Master:
Average: 400.1869333333334 ms

Remove graph reg key check (Made IsGraphingModeEnabled and IsGraphingModeAvailable always return true);
Average: 396.1287066666669 ms

This PR:
Average: 326.93176666666665 ms

Can you elaborate on only delay loading a part of NavView? I wanted to see how these numbers look in the wild and perhaps propose an option in NavView itself to support this as an option if the numbers looked good. But maybe there is a way we can do it without modifying WinUI?

Copy link
Contributor

@rudyhuyn rudyhuyn left a comment

Choose a reason for hiding this comment

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

Can you elaborate on only delay loading a part of NavView? I wanted to see how these numbers look in the wild and perhaps propose an option in NavView itself to support this as an option if the numbers looked good. But maybe there is a way we can do it without modifying WinUI?

I was thinking about solutions to keep the burger menu but still improve the launch performance. For example: not set MenuItemsSource at launch, but only when you open the menu (before: 409.69ms, after: 372.79ms, this PR: 355ms)

@mcooley
Copy link
Member

mcooley commented May 8, 2020

@joseartrivera Can we check in these improvements as separate commits, and measure the impact of each one separately? That way if any of them end up causing problems later on, we'll know whether reverting is a big deal or not.

@joseartrivera joseartrivera changed the title Improve launch performance by delay loading NavigationView and GraphEngine DLL Improve launch performance by delay loading NavigationView May 13, 2020
@joseartrivera
Copy link
Contributor Author

@joseartrivera Can we check in these improvements as separate commits, and measure the impact of each one separately? That way if any of them end up causing problems later on, we'll know whether reverting is a big deal or not.

Done!

Copy link
Contributor

@rudyhuyn rudyhuyn left a comment

Choose a reason for hiding this comment

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

Thank you Pepe!!! Excellent job as usual!

@joseartrivera joseartrivera merged commit f2e4233 into microsoft:master May 19, 2020
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

Successfully merging this pull request may close these issues.

3 participants