Skip to content

Feature: Lazily loading themes to improve startup time #10329

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
Oct 30, 2022
Merged

Feature: Lazily loading themes to improve startup time #10329

merged 15 commits into from
Oct 30, 2022

Conversation

@hez2010
Copy link
Member

@hez2010 hez2010 commented Oct 30, 2022 •
edited
Loading

  • Enable dynamic PGO
  • Postpone loading other themes to the time user open the theme settings
  • Use x:Bind instead of Binding where possible

Validation
How did you test these changes?

  • Built and ran the app
  • Tested the changes for accessibility

Screenshots (optional)
Add screenshots here.

@hez2010 hez2010 requested a review from yaira2 October 30, 2022 05:51
@yaira2
Copy link
Member

yaira2 commented Oct 30, 2022

Enable dynamic PGO

Are there any benefits from this change?

yaira2
yaira2 reviewed Oct 30, 2022
View reviewed changes
@hez2010
Copy link
Member Author

hez2010 commented Oct 30, 2022

Enable dynamic PGO

Are there any benefits from this change?

It can instrument methods and optimize them using profile data at the runtime.

@yaira2
Copy link
Member

yaira2 commented Oct 30, 2022 •
edited by hez2010
Loading

@hez2010 can you see if changing the custom theme adds a duplicate theme? image

@yaira2 yaira2 added the ready for review Pull requests that are ready for review label Oct 30, 2022
@hez2010
Copy link
Member Author

hez2010 commented Oct 30, 2022 •
edited
Loading

@hez2010 can you see if changing the custom theme adds a duplicate theme? image

Weird. I think I have guarded it here:
https://github.com/files-community/Files/pull/10329/files#diff-d43965edb43d80794b663802fa2b2a4feb4f039c29d8107c2ed2ef42ca5caa4dR97

Need debug to investigate.

yaira2
yaira2 reviewed Oct 30, 2022
View reviewed changes
yaira2
yaira2 reviewed Oct 30, 2022
View reviewed changes
@hez2010
Copy link
Member Author

hez2010 commented Oct 30, 2022

Need debug to investigate.

Resolved.

@hez2010 hez2010 requested a review from yaira2 October 30, 2022 11:20
yaira2
yaira2 reviewed Oct 30, 2022
View reviewed changes
yaira2
yaira2 approved these changes Oct 30, 2022
View reviewed changes
Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed ready for review Pull requests that are ready for review labels Oct 30, 2022
@yaira2 yaira2 merged commit 2d02fc6 into main Oct 30, 2022
@yaira2 yaira2 deleted the lazy-theme branch October 30, 2022 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yaira2 yaira2 yaira2 approved these changes

Assignees

No one assigned

Labels

ready to merge Pull requests that are approved and ready to merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hez2010 @yaira2