Bug 415698 - VCS plugin function calls redundant
Summary: VCS plugin function calls redundant
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR minor
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-12-30 08:04 UTC by Nikolai Krasheninnikov
Modified: 2020-02-18 21:55 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 20.04
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolai Krasheninnikov 2019-12-30 08:04:42 UTC
SUMMARY
This applies to any VCS dolphin plugin. KVersionControlPlugin base class virtual functions get called more than actually needed.
1. KVersionControlPlugin::fileName() calls each time entering directory or updating existing one. Sometimes it get called 3 times.
2. KVersionControlPlugin::beginRetrieval()/endRetrieval() pair get called 2 times on each directory update.
3. KVersionControlPlugin::itemVersion() for each directory entry called twice.
4. When KVersionControlPlugin::actions() got implied KVersionControlPlugin::itemVersion() get called on current item.

STEPS TO REPRODUCE
1. Add debug output to specified functions.
2. Enter a directory under VCS.
3. Call popup VCS plugin menu by right click.

OBSERVED RESULT
Virtual functions got called several times.

EXPECTED RESULT
1. KVersionControlPlugin::fileName() should be called only once on Dolphin execution.
2. KVersionControlPlugin::beginRetrieval()/endRetrieval() should be called once on directory update.
3. KVersionControlPlugin::itemVersion() should be called once on directory update.
4. Implying KVersionControlPlugin::actions() should not result in itemVersion() call since item version updates only in beginRetrieval()/endRetrieval().

SOFTWARE/OS VERSIONS
Dolphin: 20.03.70

ADDITIONAL INFORMATION
Compiled from sources of current master
Comment 1 Elvis Angelaccio 2019-12-30 16:50:31 UTC
Patches welcome ;)
Comment 2 Nikolai Krasheninnikov 2020-01-17 07:14:58 UTC
Addition on #4: as it turns out KVersionControlPlugin::itemVersion() got called by VCS plugin itself on the context menu creation so its ok.
Comment 3 Elvis Angelaccio 2020-01-26 16:10:03 UTC
Git commit 305085b58143c431d9abb7bd8dcb2f0954df725a by Elvis Angelaccio, on behalf of Nikolai Krasheninnikov.
Committed on 26/01/2020 at 16:09.
Pushed by elvisangelaccio into branch 'master'.

Fixes multiple VCS plugin calls on single directory update.

Summary:
Fixes multiple VCS plugin beginRetrival()/endRetrival()/itemVersion() calls on single directory update.
When VCS pluging finished gathering directory information VersionControlObserver::slotThreadFinished() calls KFileItemModel::setData() on each entry with appropriate item VCS information.
This in turn emits KFileItemModel::itemsChanged() which is connected with VersionControlObserver::delayedDirectoryVerification() which is starting to gather VCS directory information again.
This commits breaks the vicious circle.
FIXED-IN: 20.04.0

Reviewers: #dolphin, meven, ngraham, elvisangelaccio

Subscribers: kfm-devel

Tags: #dolphin

Differential Revision: https://phabricator.kde.org/D26721

M  +14   -2    src/views/versioncontrol/versioncontrolobserver.cpp
M  +7    -0    src/views/versioncontrol/versioncontrolobserver.h

https://commits.kde.org/dolphin/305085b58143c431d9abb7bd8dcb2f0954df725a
Comment 4 Nate Graham 2020-02-18 21:55:34 UTC
Git commit d0cbcf9718bb5192738ea9f5a5a3d6c4f9f7dcef by Nate Graham, on behalf of Nikolai Krasheninnikov.
Committed on 18/02/2020 at 21:55.
Pushed by ngraham into branch 'master'.

Fixes multiple KVersionControlPlugin::fileName() calls on entering or updating directory.

Summary:
FIXED-IN: 20.04

On each VCS plugin creation corresponding file name is saved (cached) so when we search which VCS plugin is appropriate for current directory we don't need to call KVersionControlPlugin::fileName() again.

Reviewers: #dolphin, meven, elvisangelaccio, ngraham

Reviewed By: #dolphin, meven, ngraham

Subscribers: kfm-devel

Tags: #dolphin

Differential Revision: https://phabricator.kde.org/D26962

M  +6    -6    src/views/versioncontrol/versioncontrolobserver.cpp
M  +2    -1    src/views/versioncontrol/versioncontrolobserver.h

https://commits.kde.org/dolphin/d0cbcf9718bb5192738ea9f5a5a3d6c4f9f7dcef