Bug 492010 - Rename is slow as though it's trying to resave the entire file each time
Summary: Rename is slow as though it's trying to resave the entire file each time
Status: REPORTED
Alias: None
Product: digikam
Classification: Applications
Component: AdvancedRename-engine (show other bugs)
Version: 8.4.0
Platform: Microsoft Windows Microsoft Windows
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords: efficiency-and-performance
Depends on:
Blocks:
 
Reported: 2024-08-21 22:26 UTC by Matt Baker
Modified: 2024-08-28 10:31 UTC (History)
2 users (show)

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


Attachments
Debug log (794.99 KB, text/plain)
2024-08-21 22:26 UTC, Matt Baker
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Baker 2024-08-21 22:26:39 UTC
Created attachment 172834 [details]
Debug log

Renaming a batch of images based on their metadata is extremely slow. It's as though it's rewriting the whole file with a new name instead of just moving it. Very noticeable on a network share (2 seconds per files in my case).


STEPS TO REPRODUCE
1. Select multiple files from the Album view
2. Press F2 to rename
3. Apply a new naming format and press Ok to begin renaming

OBSERVED RESULT
Each rename takes much longer than expected compared to a file move operation in the same location.

EXPECTED RESULT
Fast rename by just using a move operation with a new name.

SOFTWARE/OS VERSIONS
Windows: 10

Logs attached
Rename starts at line 3376
Comment 1 caulier.gilles 2024-08-22 05:55:42 UTC
This is how Exiv2 patch files with metadata. A copy of original is done in temporary very, changes are applied, and at end temporary file is renamed.

For few type of file as JPEG or TIFF, the file is not reencoded, for other the operation is more complex. This is how Exiv2 works in background.

The only solution to prevent time latency in GUI is to fork all metadat operations in a separated thread.

About metadata and database cache, not all information are store in database. This depend of options passed in the advanced rename settings.

Gilles Caulier
Comment 2 Maik Qualmann 2024-08-22 06:07:09 UTC
Unfortunately, the renaming function has to do a little more, e.g. search for sidecar files and rename them if they exist. This obviously leads to a significant slowdown on a network drive. The file also has to be renamed in the database and thumbnail database.

Maik
Comment 3 Matt Baker 2024-08-22 07:07:04 UTC
Thanks for the information on how this works - useful to know.

Based on this, is it most likely the sidecar file search that is taking the most time?
I can't see why Exiv2 would have to get involved for the actual rename operation - would it not only be the generation of the new name that requires Exiv2. After the name is generated, couldn't it just use standard OS move calls?
Comment 4 Maik Qualmann 2024-08-22 08:08:08 UTC
You should activate the WAL mode for the SQLite database, in the digiKam settings under database. This increases the performance of the SQLite database.

No Exiv2 is used when it comes to the execution of the renaming, only when preparing.

Maik
Comment 5 Maik Qualmann 2024-08-22 19:53:21 UTC
Git commit 57fc5bd6244991e0df7bcc4e27b488d109d10644 by Maik Qualmann.
Committed on 22/08/2024 at 19:52.
Pushed by mqualmann into branch 'master'.

do not read the entire directory to see if the file already exists

M  +1    -7    core/libs/iojobs/iojob.cpp

https://invent.kde.org/graphics/digikam/-/commit/57fc5bd6244991e0df7bcc4e27b488d109d10644
Comment 6 caulier.gilles 2024-08-27 05:47:05 UTC
Maik,

I identified also this code which take age with large items list to rename :

https://invent.kde.org/graphics/digikam/-/blob/master/core/utilities/advancedrename/advancedrenamedialog.cpp?ref_type=heads#L91

Gilles
Comment 7 caulier.gilles 2024-08-27 05:49:59 UTC
The item setImageUrl() is called in constructor for each item on the list in this loop :

https://invent.kde.org/graphics/digikam/-/blob/master/core/utilities/advancedrename/advancedrenamedialog.cpp?ref_type=heads#L317

Gilles
Comment 8 Maik Qualmann 2024-08-27 06:34:23 UTC
Really, for is faster than Q_FOREACH? That's interesting.

Maik
Comment 9 caulier.gilles 2024-08-27 08:02:51 UTC
Not faster but better. Q_FOREACH is also deprecated to use since we uses C++17. There is a switch not yet enabled to break the compilation when using Q_FOREACH.

Gilles
Comment 10 caulier.gilles 2024-08-27 09:02:59 UTC
Maik,

look the details at end of this page :

https://doc.qt.io/qt-6/foreach-keyword.html

Q_FOREACH copy the data, for C+11 use a detached one. So typically, it must be a little bit faster i think.

Gilles
Comment 11 caulier.gilles 2024-08-27 09:08:38 UTC
Maik,

KDAB has a good blog entry about the Q_FOREACH to for C++11 migration :

https://www.kdab.com/goodbye-q_foreach/

Gilles
Comment 12 Maik Qualmann 2024-08-27 09:41:59 UTC
Gilles, have you noticed this?

https://www.kdab.com/blog-qasconst-and-stdas_const/

Maik
Comment 13 caulier.gilles 2024-08-27 10:40:04 UTC
yes, i seen. Clazy static analyzer will report this kind of change to do in the code where it's can do it. So i will fix the code in a second pass.

Gilles
Comment 14 Maik Qualmann 2024-08-27 12:45:11 UTC
Do we want to use qAsConst()? It's shorter and we already use it. ((:-))

Maik
Comment 15 caulier.gilles 2024-08-27 12:49:06 UTC
yes, done...

Gilles
Comment 16 caulier.gilles 2024-08-27 13:22:44 UTC
Maik,

Note that it still more than 1000 occurrences of Q_FOREACH in whole digiKam to patch...

Gilles
Comment 17 caulier.gilles 2024-08-28 09:14:35 UTC
Maik,

With Qt 6.7.2 and clang  : warning: 'qAsConst<...>' is deprecated: Use std::as_const() instead. [-Wdeprecated-declarations]

Gilles
Comment 18 Maik Qualmann 2024-08-28 10:31:50 UTC
Ok, then we can't use qAsConst()...

Maik