Code quality: Move Git operations to a different thread #13608

ferrariofilippo · 2023-10-30T11:33:45Z

Resolved / Related Issues

Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
Related Bug: Git columns causing CPU to increase #12737

Changes

Branches are enumerated and filtered on a different thread
Head branch name is now "cached"
Fetch, Pull, Push, Sync, Checkout are now executed on a different thread.

Note
You may need to log-in github from Files Preview or Stable to use some functionalities

Validation
How did you test these changes?

Did you build the app and test your changes?
Did you check for accessibility? You can use Accessibility Insights for this.
Did you remove any strings from the en-us resource file?
- Did you search the solution to see if the string is still being used?
Did you implement any design changes to an existing feature?
- Was this change approved?
Are there any other steps that were used to validate these changes?
1. Check that Files doesn't freeze when opening large git repos
2. Check that git actions (fetch, pull, push, checkout, sync) still work

yaira2 · 2023-10-30T13:53:26Z

Fyi @hishitetsu @hez2010

src/Files.App/Utils/Git/GitHelpers.cs

hishitetsu

This is still not completely thread-safe theoretically, but I think it's actually okay.

src/Files.App/Utils/Git/GitHelpers.cs

Co-authored-by: hishitetsu <66369541+hishitetsu@users.noreply.github.com>

src/Files.App/Utils/Git/GitHelpers.cs

hishitetsu

LGTM

ferrariofilippo · 2023-11-06T08:36:20Z

I'm not quite sure why this is okay, but it certainly solved the issue.

Making BaseLayout.OnNavigatedTo() async caused this line
https://github.com/files-community/Files/blob/main/src/Files.App/Views/LayoutModes/BaseLayout.cs#L480C11-L480C11
to be executed after this one
https://github.com/files-community/Files/blob/main/src/Files.App/Views/LayoutModes/DetailsLayoutBrowser.xaml.cs#L126
causing EnabledGitProperties to reset to None

soleon · 2023-11-08T22:50:14Z

When browsing any folder in a large git repo, the UI was still unresponsive, froze everywhere, the app was unusable. This happened with or without VS or debugger attached.

ferrariofilippo · 2023-11-09T10:25:23Z

I did some tests, here are the conclusions:

Changing the priority of GitOperations-Thread does not consistently reduce CPU spikes (I tried with both BelowNormal and Lowest).
The source of the freeze seems to be GitHelpers.GetGitInformationForItem, which used the CPU for 10-15% of the total time. That's a lot considering I opened many non-git folders.
LibGit2Sharp.DiffCompare<T> run for about 4-8% of total CPU time

I'll see if I can find a fix

…cals

yaira2 · 2023-11-23T21:47:40Z

@soleon can you try with the latest commit?

yaira2

LGTM

commit 432b47c Author: Yair <39923744+yaira2@users.noreply.github.com> Date: Thu Nov 23 14:04:12 2023 -0500 Update FUNDING.yml commit f42f2ea Author: Yair <39923744+yaira2@users.noreply.github.com> Date: Thu Nov 23 10:01:39 2023 -0500 Preview: 3.0.11 commit dfc7c2f Author: Yair <39923744+yaira2@users.noreply.github.com> Date: Thu Nov 23 10:00:16 2023 -0500 Fix: Fixed check for admin (files-community#13971) commit 79d31e3 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu Nov 23 09:52:34 2023 -0500 Build(deps): Bump SQLitePCLRaw.bundle_green from 2.1.6 to 2.1.7 (files-community#14029) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit c0bb1b8 Author: Tony Barrera <antonioseet1@gmail.com> Date: Wed Nov 22 18:53:53 2023 -0800 Fix: Fixed an issue with drag and dropping items onto .py files (files-community#13986) commit 4b58c2d Author: Yair <39923744+yaira2@users.noreply.github.com> Date: Wed Nov 22 17:12:09 2023 -0500 Fix: Disable media preview for archive items (files-community#14021) commit 00543d8 Author: Yair <39923744+yaira2@users.noreply.github.com> Date: Wed Nov 22 16:34:25 2023 -0500 Feature: Indicate "Always keep on device" status (files-community#14012) commit 92c5231 Author: Yair <39923744+yaira2@users.noreply.github.com> Date: Wed Nov 22 14:01:23 2023 -0500 New Crowdin updates (files-community#14022) commit b3c4d83 Author: 0x5bfa <62196528+0x5bfa@users.noreply.github.com> Date: Thu Nov 23 00:18:41 2023 +0900 Code Quality: Renamed layout page classes to improve readability (files-community#13958) commit 1e1bbe7 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed Nov 22 09:50:51 2023 -0500 Build(deps): Bump Microsoft.CodeAnalysis.CSharp from 4.5.0 to 4.8.0 (files-community#14017) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 5adae99 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed Nov 22 09:50:36 2023 -0500 Build(deps): Bump Microsoft.Graphics.Win2D from 1.1.0 to 1.1.1 (files-community#14016) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 8e974c7 Author: Yair <39923744+yaira2@users.noreply.github.com> Date: Tue Nov 21 21:39:12 2023 -0500 Feature: Display error when failing to calculate the hash of open files (files-community#14014) commit bf7b5da Author: Yair <39923744+yaira2@users.noreply.github.com> Date: Tue Nov 21 21:18:46 2023 -0500 Fix: Fixed NullReferenceException in GetCompressDestination (files-community#14007) commit 79855ba Author: Yair <39923744+yaira2@users.noreply.github.com> Date: Tue Nov 21 17:36:37 2023 -0500 Fix: Fixed xamlRoot in the ConsentDialog (files-community#14013) commit 494153c Author: William-Hiatt <98793239+William-Hiatt@users.noreply.github.com> Date: Tue Nov 21 13:17:49 2023 -0800 Fix: Fixed "Drive Unplugged" error when compressing items in Column layout (files-community#13695)

soleon · 2023-11-24T23:12:12Z

@soleon can you try with the latest commit?

Sorry for the late response @yaira2, I can confirm that this problem is fixed in the latest 'main' branch 🎉.

yaira2 · 2023-11-25T23:42:12Z

Thank you for testing

ferrariofilippo added 5 commits October 27, 2023 16:32

Use a task to fetch branches

57ac822

Delay repository head sorting

ad6f72a

Move some git operations to a different thread

94d50b6

Move more git operations to a different thread

6ce6cba

Merge branch 'main' into Git_Tests

c58b6ca

yaira2 added the needs - code review label Oct 30, 2023

yaira2 added 2 commits October 30, 2023 12:38

GitHub: Preview deploy action (files-community#13615)

5ad0970

Merge branch 'main' into Git_Tests

013ea59

yaira2 force-pushed the main branch 3 times, most recently from 50ee7a2 to 7df32e0 Compare October 30, 2023 21:55

GitHub: Preview deploy action

eea0fb2

yaira2 force-pushed the main branch from 3ebb408 to eea0fb2 Compare October 31, 2023 01:16

hishitetsu reviewed Oct 31, 2023

View reviewed changes

src/Files.App/Utils/Git/GitHelpers.cs Outdated Show resolved Hide resolved

ferrariofilippo added 2 commits October 31, 2023 09:04

Dispose the thread when not in use

6330b43

Merge branch 'main' into Git_Tests

3963d0c

hishitetsu reviewed Oct 31, 2023

View reviewed changes

src/Files.App/Utils/Git/GitHelpers.cs Outdated Show resolved Hide resolved

Use Interlocked

2119e12

hishitetsu reviewed Oct 31, 2023

View reviewed changes

src/Files.App/Utils/Git/GitHelpers.cs Outdated Show resolved Hide resolved

Update GitHelpers.cs

bfc2cb2

hishitetsu reviewed Oct 31, 2023

View reviewed changes

ferrariofilippo and others added 7 commits October 31, 2023 13:22

Update src/Files.App/Utils/Git/GitHelpers.cs

eac354c

Co-authored-by: hishitetsu <66369541+hishitetsu@users.noreply.github.com>

Update src/Files.App/Utils/Git/GitHelpers.cs

0d890bc

Co-authored-by: hishitetsu <66369541+hishitetsu@users.noreply.github.com>

Update src/Files.App/Utils/Git/GitHelpers.cs

6190177

Co-authored-by: hishitetsu <66369541+hishitetsu@users.noreply.github.com>

Update src/Files.App/Utils/Git/GitHelpers.cs

d95cd7a

Co-authored-by: hishitetsu <66369541+hishitetsu@users.noreply.github.com>

Update src/Files.App/Utils/Git/GitHelpers.cs

4cef968

Co-authored-by: hishitetsu <66369541+hishitetsu@users.noreply.github.com>

Update src/Files.App/Utils/Git/GitHelpers.cs

1494fc9

Co-authored-by: hishitetsu <66369541+hishitetsu@users.noreply.github.com>

Update src/Files.App/Utils/Git/GitHelpers.cs

f89d8fe

Co-authored-by: hishitetsu <66369541+hishitetsu@users.noreply.github.com>

hishitetsu reviewed Oct 31, 2023

View reviewed changes

src/Files.App/Utils/Git/GitHelpers.cs Outdated Show resolved Hide resolved

hishitetsu approved these changes Nov 6, 2023

View reviewed changes

yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Nov 6, 2023

yaira2 mentioned this pull request Nov 7, 2023

Bug: Git columns causing CPU to increase #12737

Closed

Catch git exceptions

8694b2d

yaira2 added needs - code review and removed ready to merge Pull requests that are approved and ready to merge labels Nov 10, 2023

yaira2 force-pushed the main branch 3 times, most recently from cecddda to 7f8c927 Compare November 17, 2023 16:57

ferrariofilippo added 4 commits November 21, 2023 22:08

Merge branch 'main' into Git_Tests

a4affb6

Fix a bug where branches flyout might automatically switch back to lo…

1bd2cde

…cals

Delay git info fetching

575efb8

Delay branch load

66f37c4

yaira2 previously approved these changes Nov 23, 2023

View reviewed changes

yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Nov 23, 2023

yaira2 dismissed their stale review via 5043b3c November 23, 2023 21:53

ferrariofilippo added 2 commits November 23, 2023 22:57

Merge branch 'main' into Git_Tests

4105051

Hide Git status column by default

fbc9380

yaira2 merged commit 95163cc into files-community:main Nov 23, 2023

ferrariofilippo deleted the Git_Tests branch November 23, 2023 22:22

ferrariofilippo mentioned this pull request Nov 24, 2023

Feature: Allow deleting of local Git branches #14059

Merged

8 tasks

Uh oh!

Code quality: Move Git operations to a different thread #13608

Code quality: Move Git operations to a different thread #13608

Uh oh!

Conversation

ferrariofilippo commented Oct 30, 2023

Uh oh!

yaira2 commented Oct 30, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hishitetsu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hishitetsu left a comment

Choose a reason for hiding this comment

Uh oh!

ferrariofilippo commented Nov 6, 2023

Uh oh!

soleon commented Nov 8, 2023

Uh oh!

ferrariofilippo commented Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yaira2 commented Nov 23, 2023

Uh oh!

yaira2 left a comment

Choose a reason for hiding this comment

Uh oh!

soleon commented Nov 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yaira2 commented Nov 25, 2023

Uh oh!

Uh oh!

ferrariofilippo commented Nov 9, 2023 •

edited

Loading

soleon commented Nov 24, 2023 •

edited

Loading