tibirna / qgit

Official git repository for QGit.
Other
171 stars 66 forks source link

Speedup browsing huge patches - show changes for single files. #125

Open agutikov opened 2 years ago

agutikov commented 2 years ago

With large patches next functions of PatchContent became so slow that app became unresponsive:

And if you select revision with huge patch in RevsView app eats 100% cpu core and freeze for seconds to minutes. Then if you select file from FileList - then it hangs almost forever.

This patch makes PatchContent show changes only for selected file if size of patch text exceeds configured limit.

Added configuration option "Max patch size MiB". If size of patch text exceeds that limit - PatchContent splits it into per-file change and shows changes for each selected file separately. Otherwise - no changes, will be shown entire patch content.

tibirna commented 2 years ago

Thank you for the useful functionality.

A few issues:

Please, give an example of a repository containing very big diffs, on which I could inspect the observed slowness issue.

Thanks again Cristian

WickedSmoke commented 2 years ago

I run into this bug occasionally so I've applied a simplified version of this patch (without settings configuration) to my repository with a hardcoded 400KiB limit. At that size the pause is a couple seconds on my system.

This repository has a variety of large commits that can be used for testing: https://github.com/huxingyi/dust3d

commit 8e5d622db78c00f5c9c1b7b1ad9d13985c7cc531 
Date:   Thu Nov 18 22:58:01 2021 +0800
    Restructure the code base
10390 files changed

commit 5b7c9dbc448079b3e0d21a9b046d8def4bdfc9e8
Date:   Tue Oct 13 22:14:25 2020 +0930
    Update CGAL to 5.1
5222 files changed

commit 8c6a9fef29684884dca8d9d2f11adefd907b1188
Date:   Sat Jan 4 22:19:03 2020 +0930
    Mark remeshed component as Uncombined
7002 files changed

commit 3b4d4ddc52e5cdfecedac7523b185073e95d403e
Date:   Tue Dec 31 21:16:33 2019 +0930
    Update libigl and eigen
2012 files changed

commit dd628b6af7f1b425c128fd0ef59a4bada17fa234
Date:   Sat Dec 28 14:51:59 2019 +0930
    Integrate QuadriFlow
283 files changed

commit 17cb1cb0599f2514fba5cc3147ff0dade0dfbe14
Date:   Sun Sep 22 00:00:27 2019 +0930
    Introduce bullet3 to support ragdoll procedural animation
702 files changed

commit 208d2a0166f34de8073cb57eba5be9fea09c1f5e
Date:   Thu Oct 25 08:19:38 2018 +0800
    Cleanup
135 files changed

commit a4a734a67b6bad10fc7005534976e6259e019ad4
Date:   Mon Oct 15 23:02:31 2018 +0800
    Repleace the uv unwrapping library with simpleuv
1533 files changed

commit 09d21e893527454b56d7fcfb0a832c9a2dd2073a
Date:   Mon Mar 26 20:41:46 2018 +0800
    Add skeleton snapshot support (Prepare for undo/redo feature)
1172 files changed
tibirna commented 2 years ago

Hello Karl

Thanks for the test repo example and for the report on the behaviour of your patch.

Alexei,

Do you have comments on my previous notes from the reply of 2022-02-25? I don't see any additional changes in your forked repo (although you mention a simplified version above). Am I looking in the wrong place?

WickedSmoke commented 2 years ago

The change was to my local repo, but I've now pushed it to https://github.com/WickedSmoke/qgit/tree/large_patch. Note that patchcontent.h is unchanged from Aleksei's code, so the comments there are not accurate.

Ideally this would be fixed in a way that prevents the slowdown from ever occurring and without any work-around settings. It may need to be considered along with Log/Diff tab changes of #44, #85, & #105. Other Git browsers handle this by keeping all diffs in one view, but collapsing the diff of individual files which are too large. This would work with my preferred change of eliminating the tabs.

Note that a short term solution of using my hardcoded largePatchBytes along with your recommend message in the "Diff" display would make this initial display unreachable after clicking on a file. The user would have to select another commit and re-select the desired one to see the list of changes again.

WickedSmoke commented 2 years ago

I updated my large_patch branch with a "[Large Patch]" notice. Even with the caveat about the unreachable initial display, I think pushing something similar to master is preferable to the current behavior which makes the program unusable.