Bug 486949

Summary: Kdevelop freezes after git checkout feature-branch
Product: [Applications] kdevelop Reporter: Dominik Kummer <admin>
Component: UI: generalAssignee: kdevelop-bugs-null
Status: CONFIRMED ---    
Severity: normal CC: igorkuo, jonathan.verner, xaver.hugl
Priority: NOR    
Version First Reported In: 5.13.240202   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: first backtrace during hang
second backtrace during hang

Description Dominik Kummer 2024-05-13 11:09:52 UTC
SUMMARY

KDevelop freezes after git checkout to another branch of the git project.

Operating System: Archlinux 
KDE Plasma Version: 6.0.4
KDE Frameworks Version: 6.1.0
Qt Version: 6.7.0
Kernel Version: 6.8.9-arch1-2 (64-bit)
Graphics Platform: X11
Processors: 12 × Intel® Xeon® W-2135 CPU @ 3.70GHz
Memory: 31,1 GiB of RAM
Graphics Processor: Quadro P4000/PCIe/SSE2
Manufacturer: HP
Product Name: HP Z4 G4 Workstation
Comment 1 Igor Kushnir 2024-05-21 18:00:15 UTC
When this happens, run `gdb -batch -ex "bt" -ex "quit" -p 12345 > ~/path/to/frozen.bt`, where 12345 is the ID of the frozen KDevelop process. Wait a few seconds/minutes and run the command again (with a different output filename) to check if KDevelop is still stuck in the same place in code. Then attach the generated backtrace(s) to your bug report. Make sure to have KDevelop debug symbols installed (either build KDevelop manually with debug info and don't strip it or use debuginfod - https://wiki.archlinux.org/title/Debuginfod).
Comment 2 Zamundaaa 2024-10-07 13:54:09 UTC
Can confirm, on lots of git operations KDevelop hangs temporarily (for a few seconds).
Comment 3 Zamundaaa 2024-10-07 13:54:38 UTC
Created attachment 174503 [details]
first backtrace during hang
Comment 4 Zamundaaa 2024-10-07 13:55:09 UTC
Created attachment 174504 [details]
second backtrace during hang
Comment 5 Igor Kushnir 2024-10-07 17:40:25 UTC
The backtrace seems to be a recursion, but is not, because getLsFiles() does not connect the parseGitStatusOutput() slot to any signal. So it looks like a lot of `git status` jobs finish one after another, each job's result is handled in the previous job's nested event loop and thus blocks the previous job's progress.

I suspect the jobs are triggered by many almost simultaneous ProjectModel::rowsInserted signals, to which the ProjectChangesModel::itemsAdded() slot is connected, which in turn calls ProjectChangesModel::changes(), and IBasicVersionControl::status() is called from there.

This is not the first time that the slowness of many non-parallel git commands causes a UI freeze: https://invent.kde.org/kdevelop/kdevelop/-/merge_requests/299 . Replacing job->exec() in GitPlugin::getLsFiles() with running the process asynchronously could help, but the implementation of DVcsJob::jobIsReady() does not allow such an optimization, because it emits result right after emitting readyForParsing(). So such a fix would require some DVcsJob redesign and adjustments to all DVCS plugins. Maybe a simpler fix exists, but I don't see it yet, and I'm not even sure my guess about the source of many `git status` jobs is correct.
Comment 6 Igor Kushnir 2024-10-08 14:01:39 UTC
(In reply to Igor Kushnir from comment #5)
> Maybe a simpler fix exists,
> but I don't see it yet, and I'm not even sure my guess about the source of
> many `git status` jobs is correct.
The essence of the convoluted function GitPlugin::parseGitStatusOutput() is 14 years old. Perhaps this function can be simplified and optimized so as to avoid calling getLsFiles() from it. Even GitPlugin::parseGitStatusOutput_old() seems more efficient (though it is likely less correct because practically untested for a long while). Merge requests are welcome.
Comment 7 Igor Kushnir 2024-12-15 15:38:49 UTC
I started noticing this slowdown lately. Very annoying.
Comment 8 Zamundaaa 2025-01-21 17:51:28 UTC
Interestingly, I'm currently doing stuff in Mesa (which uses Meson), and while it still hangs for a second, it's responsive again way sooner than in KWin, despite being a larger project with more files.
Comment 9 Igor Kushnir 2025-02-03 16:31:18 UTC
(In reply to Zamundaaa from comment #8)
> Interestingly, I'm currently doing stuff in Mesa (which uses Meson), and
> while it still hangs for a second, it's responsive again way sooner than in
> KWin, despite being a larger project with more files.
I think the extent of the slowdown depends on how long the git commands `git status` and `git ls-files` take. The slowdown for the KDevelop project was very significant on my system until recently. There had been a repository corruption caused by a system freeze, which produced an error message whenever I run `gitk --reflog`. But not anymore. I suspect that git garbage collection has run, got rid of corrupted objects and mitigated the slowdown tracked in this bug.

I just found that connections to the VCS plugin's branch change are not unique. So if N git projects are open, the number of `git status` and `git ls-files` calls is multiplied by N. I'll fix this.

Another thing I noticed is a slowdown when git-rebase applies many commits one after another. This is likely caused by the line `QTimer::singleShot(1000, this, &GitPlugin::delayedBranchChanged);` in GitPlugin::fileChanged(), which accumulates timers and emits repositoryBranchChanged() many times. I intend to fix this by reusing a single QTimer per git repository and restarting it in GitPlugin::fileChanged().

A more complicated issue is that RepoStatusModel and ProjectChangesModel both run `git status` with the same parameters when something changes. Ideally, their code should be deduplicated and refactored so that `git status` is run only once. But that's much more difficult, especially because the Project Changes tool view seems utterly broken and would require a lot of work to fix.
Comment 10 Igor Kushnir 2025-02-03 21:31:30 UTC
(In reply to Igor Kushnir from comment #9)
> A more complicated issue is that RepoStatusModel and ProjectChangesModel
> both run `git status` with the same parameters when something changes.
> Ideally, their code should be deduplicated and refactored so that `git
> status` is run only once. But that's much more difficult, especially because
> the Project Changes tool view seems utterly broken and would require a lot
> of work to fix.
The relevant duplicated code in RepoStatusModel and ProjectChangesModel is broken in so many ways that I am tempted to mostly remove ProjectChangesModel.

Since many users don't use the Git Commit tool view, they should not pay the performance price for its poor performance in the background. So I would like to make RepoStatusModel, along with the Git Commit tool view that uses it, optional and off by default.
Comment 11 Igor Kushnir 2025-02-03 21:40:11 UTC
(In reply to Igor Kushnir from comment #9)
> A more complicated issue is that RepoStatusModel and ProjectChangesModel
> both run `git status` with the same parameters when something changes.
> Ideally, their code should be deduplicated and refactored so that `git
> status` is run only once. But that's much more difficult, especially because
> the Project Changes tool view seems utterly broken and would require a lot
> of work to fix.
The relevant duplicated code in RepoStatusModel and ProjectChangesModel is broken in many ways. ProjectChangesModel is almost completely nonfunctional (otherwise, KDevelop would freeze unbearably!), so I am tempted to mostly remove or at least disable it.

Since many KDevelop users don't use the Git Commit tool view, they should not pay the price for its poor performance in the background. So I would like to make RepoStatusModel, along with the Git Commit tool view that uses it, optional and off by default. Perhaps moving m_repoStatusModel from GitPlugin to CommitToolView would work? This way, the slow RepoStatusModel would exist only as long as the Git Commit tool view is present, and then we could display some kind of poor-background-performance warning for the Git Commit tool view.
Comment 12 Bug Janitor Service 2025-03-06 10:22:18 UTC
A possibly relevant merge request was started @ https://invent.kde.org/kdevelop/kdevelop/-/merge_requests/735
Comment 13 Igor Kushnir 2025-03-06 17:09:03 UTC
Git commit 737fd44d385acd2d1e174601a226968ba57fd4e0 by Igor Kushnir.
Committed on 06/03/2025 at 10:17.
Pushed by igorkushnir into branch 'master'.

Make connections to the signal repositoryBranchChanged() unique

The slots ProjectChangesModel::addProject(),
RepoStatusModel::addProject() and VcsOverlayProxyModel::addProject() are
invoked for each opened project. If an opened project's version control
plugin is the GitPlugin, each of these 3 slots connects the
corresponding class's repositoryBranchChanged() slot to the namesake
signal of GitPlugin. As the connections are not unique, the number of
invocations of each class's repositoryBranchChanged() slot is multiplied
by the number of opened git projects.

Use the function pointer-based connection syntax in
RepoStatusModel::addProject(). The string-based connection and the
accompanying comment have been thoughtlessly copied from a place where
`plugin` is an IPlugin* rather than a GitPlugin*.

M  +2    -1    kdevplatform/project/projectchangesmodel.cpp
M  +2    -2    plugins/git/repostatusmodel.cpp
M  +2    -1    plugins/projectmanagerview/vcsoverlayproxymodel.cpp

https://invent.kde.org/kdevelop/kdevelop/-/commit/737fd44d385acd2d1e174601a226968ba57fd4e0
Comment 14 Bug Janitor Service 2025-03-11 12:02:55 UTC
A possibly relevant merge request was started @ https://invent.kde.org/kdevelop/kdevelop/-/merge_requests/737
Comment 15 Igor Kushnir 2025-03-12 17:10:52 UTC
Git commit c68593429384cccd05c893ce7ddef7c7e0cde4e2 by Igor Kushnir.
Committed on 12/03/2025 at 15:49.
Pushed by igorkushnir into branch 'master'.

Unregister repositories for current branch changes

When a project with an IBranchingVersionControl version control plugin
(Git or Mercurial) is opened, its path is registered for current branch
changes. GitPlugin::registerRepositoryForCurrentBranchChanges() adds the
HEAD file of each registered git repository to a KDirWatch. However,
when a project is closed, its path is not unregistered and remains
watched by the KDirWatch.

Unregister a closed project's path to properly clean up and optimize.
Also unregister the paths of all still-open projects when a registrant
(a listener to the signal repositoryBranchChanged()) is destroyed.
Multiple listeners register the same opened project path:
ProjectChangesModel, RepoStatusModel (this one failed to register before
now) and VcsOverlayProxyModel. Therefore, robust unregistering requires
adding a listener parameter to the functions
(un)registerRepositoryForCurrentBranchChanges() and storing lists of
listeners in GitPlugin. Otherwise, if a listener unregisters a path,
which it has not registered before, another listener may miss current
branch updates.

At this time, the elaborate (un)registering is not very useful because
ProjectController::initialize() unconditionally creates the
ProjectChangesModel instance, which unconditionally registers all
IBranchingVersionControl project repositories and remains alive until
KDevelop exit (incidentally, the late destruction prevents
~ProjectChangesModel() from unregistering all still-open projects).
Similarly, RepoStatusModel exists as long as its parent GitPlugin is
alive.

Due to KDevelop UI freezes caused by background work of
ProjectChangesModel and RepoStatusModel (manifested in Bug 486949),
subsequent commits will make sure that the instance of each of the two
classes exists only as long as its tool view is present in the UI. And
then the diligent unregistering will prevent useless work when all
listeners to the signal repositoryBranchChanged() are destroyed.

RepoStatusModel fails to register project repositories for current
branch changes. Correct this omission to prevent bugs when a subsequent
commit makes the ProjectChangesModel instance optional and prevents it
from reliably registering all repositories.

GitPlugin::fileChanged() emits the signal repositoryBranchChanged() with
a 1-second delay because the HEAD file is modified while the git
repository is still being updated and is not yet ready for use. When a
git rebase process quickly applies commits one after another, multiple
changes to the HEAD file occur within this 1-second interval. The slow
handling by the listeners of the multiple accumulated and emitted
repositoryBranchChanged() signals can freeze the KDevelop UI from the 1
second after the beginning of the rebase operation until several seconds
after the operation's completion. Each of these emitted signals reports
a current branch change of the same git repository, so the listeners
perform essentially the same work over and over again, and overwrite
previous results in response to each emission.

Store a timer per watched HEAD file and restart it instead of starting a
new one when the file is changed before the 1-second timeout. This
effectively compresses multiple close-by signals into one. Now the UI
freeze (if noticeable at all) starts a second after the completion of
the rebase operation and usually lasts less than a second.

M  +11   -2    kdevplatform/project/projectchangesmodel.cpp
M  +10   -5    kdevplatform/vcs/interfaces/ibranchingversioncontrol.h
M  +109  -10   plugins/git/gitplugin.cpp
M  +71   -2    plugins/git/gitplugin.h
M  +16   -2    plugins/git/repostatusmodel.cpp
M  +16   -1    plugins/projectmanagerview/vcsoverlayproxymodel.cpp
M  +1    -0    plugins/projectmanagerview/vcsoverlayproxymodel.h

https://invent.kde.org/kdevelop/kdevelop/-/commit/c68593429384cccd05c893ce7ddef7c7e0cde4e2
Comment 16 Igor Kushnir 2025-03-31 10:06:09 UTC
https://invent.kde.org/kdevelop/kdevelop/-/merge_requests/743 allows to completely work around these UI freezes by removing the Project Changes and the Git Commit tool views from all used sublime areas (Code, Debug, Review). These tool views are absent from all areas by default, but some users may have added them at some point without actually using them much.
Comment 17 Igor Kushnir 2025-06-21 10:04:42 UTC
Git commit 91b9dc05c7f304ae515c794064e26f82c7c6e7cb by Igor Kushnir.
Committed on 21/06/2025 at 09:36.
Pushed by igorkushnir into branch 'master'.

Tie the lifetime of ProjectChangesModel to VcsChangesView

Not long after the ProjectChangesModel class was added to KDevPlatform,
8c8b4bbf4b6807f7c379cd2366b587a28bdd177f started creating an instance of
the model only when a VcsChangesView instance, which requires it, was
created (i.e. only when the Project Changes tool view widget was
created). Then 8e9c863e76d8229bdccc6a7b56af2f1cbed3d98a reused the
ProjectChangesModel in VcsOverlayProxyModel::data(). A
VcsOverlayProxyModel instance is created whenever a widget for the much
more widely used Projects tool view, present by default in the Code and
the Review sublime areas, is created. The construction of the
ProjectChangesModel instance in VcsOverlayProxyModel::data() caused a
crash due to a nested event loop, so
6d4591fb1733f4490d77d9e6a1e619b31767d49b switched from creating the
instance of ProjectChangesModel on demand to unconditionally creating it
in ProjectController::initialize().

ProjectChangesModel tracks the VCS statuses of all project files and
does so very inefficiently. The inefficiency causes UI freezes discussed
in the linked bug report.

Store a std::weak_ptr<ProjectChangesModel> in ProjectControllerPrivate
and a std::shared_ptr<ProjectChangesModel> in VcsChangesView in order to
keep the slow ProjectChangesModel instance alive only as long as a
VcsChangesView instance exists. Now the user can work around the UI
freezes by removing the Project Changes tool view. The author of all
relevant code, Aleix expressed hope that this optimization will be
restored eventually in a comment under https://phabricator.kde.org/D8852

Do not display the VCS status of non-project items in the Projects tool
view if the ProjectChangesModel instance does not exist. This VCS status
info in the Projects tool view is not essential. No one has complained
about its absence for many years between the regression introduced in
5f3e04f5c6c870b3b4bafe57b372df74957185ff and a recent commit that fixed
index lookup by URL in ProjectChangesModel to make it work again.

Unregister project repositories for current branch changes in
~ProjectChangesModel(). This is now possible because the
ProjectChangesModel instance is destroyed earlier - when the
VCS Integration plugin is unloaded.

Make use of the shared pointer to ProjectChangesModel now stored in
VcsChangesView:
* get rid of the cast qobject_cast<ProjectChangesModel*>(model());
* call the ProjectChangesModel::reload() slots directly instead of
  connecting them to VcsChangesView's reload() signals and emitting.

M  +12   -5    kdevplatform/interfaces/iprojectcontroller.h
M  +4    -1    kdevplatform/project/projectchangesmodel.cpp
M  +1    -1    kdevplatform/project/projectchangesmodel.h
M  +15   -5    kdevplatform/shell/projectcontroller.cpp
M  +2    -1    kdevplatform/shell/projectcontroller.h
M  +14   -0    kdevplatform/shell/tests/test_projectcontroller.cpp
M  +2    -0    kdevplatform/shell/tests/test_projectcontroller.h
M  +8    -4    plugins/projectmanagerview/vcsoverlayproxymodel.cpp
M  +8    -12   plugins/vcschangesview/vcschangesview.cpp
M  +10   -10   plugins/vcschangesview/vcschangesview.h
M  +6    -19   plugins/vcschangesview/vcschangesviewplugin.cpp
M  +0    -7    plugins/vcschangesview/vcschangesviewplugin.h

https://invent.kde.org/kdevelop/kdevelop/-/commit/91b9dc05c7f304ae515c794064e26f82c7c6e7cb
Comment 18 Igor Kushnir 2025-06-21 10:23:31 UTC
Git commit 0a81f709204f6bf0c844381d673244b20188381e by Igor Kushnir.
Committed on 21/06/2025 at 09:36.
Pushed by igorkushnir into branch 'master'.

Tie the lifetime of RepoStatusModel to CommitToolView

GitPlugin() creates the instance of RepoStatusModel on KDevelop start.
This model instance is destroyed only when the GitPlugin itself is
destroyed, i.e. on KDevelop exit.

Similarly to ProjectChangesModel, RepoStatusModel tracks the VCS
statuses of all (git-version-controlled) project files and does so very
inefficiently. The inefficiency causes UI freezes discussed in the
linked bug report.

CommitToolView is the only user of the RepoStatusModel instance. Store a
std::weak_ptr<RepoStatusModel> in CommitToolViewFactory and a
std::shared_ptr<RepoStatusModel> in CommitToolView in order to keep the
slow RepoStatusModel instance alive only as long as a CommitToolView
instance exists. Now the user can work around the UI freezes by removing
the Git Commit tool view.

The parent commit made an equivalent lifetime change to a similarly slow
ProjectChangesModel and the Project Changes tool view. As the two model
classes perform largely the same slow work, it has to be deduplicated
(do the work once if instances of both models exist at the same time)
and optimized before the linked bug can be resolved.

M  +14   -12   plugins/git/committoolview.cpp
M  +7    -7    plugins/git/committoolview.h
M  +1    -3    plugins/git/gitplugin.cpp
M  +0    -4    plugins/git/gitplugin.h
M  +1    -1    plugins/git/repostatusmodel.h

https://invent.kde.org/kdevelop/kdevelop/-/commit/0a81f709204f6bf0c844381d673244b20188381e