Either `QFileInfo` caching does not work, or just its methods are unreasonable (very) slow

Christian Ehrlicher

Ok, I think the caching works, it's the QDateTime creation. You can prove that when you add

  for (QFileInfo &fileInfo : fileInfoList) {
    fileInfo.setCaching(false);
  }

After you gathered the file information.

/edit:
Looks like it's the QDateTime conversion from UTC to localTime here. This is done after the cache and therefore executed every time. Removing toLocalTime() gives very impressive results:

with .toLocalTime()
[timer][sort(QList<MyFileInfo>)]: 35 ms
[timer][sort(QList<MyFileInfo>)]: 30 ms
[timer][sort(QList<QFileInfo>)]: 176 ms
[timer][sort(QList<QFileInfo>)]: 169 ms

without .toLocalTime()
[timer][sort(QList<MyFileInfo>)]: 3 ms
[timer][sort(QList<MyFileInfo>)]: 2 ms
[timer][sort(QList<QFileInfo>)]: 5 ms
[timer][sort(QList<QFileInfo>)]: 3 ms

Source code: https://code.woboq.org/qt5/qtbase/src/corelib/io/qfileinfo.cpp.html#_ZNK9QFileInfo8fileTimeEN11QFileDevice8FileTimeE

QDateTime::toLocalTime() calls some OS tz - Functions and one of this is calling getenv() which is the bottleneck here on Linux. Think it's a similar thing on Windows.

RTEDFD

Most likely it is.

Since the caching code is pretty simple*: https://github.com/qt/qtbase/blob/f29566c5a41c127eacaf13f3dbfe4624e55bc83f/src/corelib/io/qfileinfo.cpp#L191-L218

It checks does there a value in the array, if it does, returns it, so the performance should be the same as when I read it from MyFileInfo object.

*Also it caches ONLY the requested time, so if I need all 4 time values (M, B, C, A) I need to ask OS 4 times, while most likely the OS call returns all 4 times once. It's not optimal in terms of performance.

Using of that stats method is overkill, since it is slower that calls of 4 methods (lastModified, birthTime, metadataChangeTime, lastRead). Currently with that 30000 files directory stat takes 1100 ms, only lastModified — 95 ms, all 4 times — 360 ms.

JonB

@RTEDFD
I looked at the code that @Christian-Ehrlicher has been looking at. Unfortunately you cannot influence/replace the conversion to localtime each time you call one of the filetime functions, and it uses internal Qt calls you cannot access so it's not possible to write a drop in replacement.

So unless you want to wait for a possible fix --- which I think would mean them changing quite some code they may not be prepared to do --- if this speed issue breaks you you are left with are you prepared to use your own structure like you show so that you can hold the converted times for the files?

Christian Ehrlicher

@RTEDFD said in Either `QFileInfo` caching does not work, or just its methods are unreasonable (very) slow:

*Also it caches ONLY the requested time, so if I need all 4 time values (M, B, C, A) I need to ask OS 4 times, while most likely the OS call returns all 4 times once. It's not optimal in terms of performance.

You should really look and understand the code before ranting about something. You're wrong: https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/io/qfilesystemengine_unix.cpp#n377

Most likely it is.

Not only most likely - see my numbers.

Here again numbers for 30k files. With QDateTime::toLocalTime() 1.3s, without 0.07s.

Christian Ehrlicher

Here the bug report: https://bugreports.qt.io/browse/QTBUG-100349

RTEDFD

It looks you run the code from the bug report in the debug mode.

When I run your code in the debug mode:

getFileInfos: 62
#files: 30000
myFileInfoList 93
bencMyFileInfo 19
bencQFileInfo 2925

The difference is 150 times.

But when I run it in the release mode:

getFileInfos: 64
#files: 30000
myFileInfoList 93
bencMyFileInfo 2
bencQFileInfo 2879

MyFileInfo sorting is 1439 times faster.

Christian Ehrlicher

@RTEDFD said in Either `QFileInfo` caching does not work, or just its methods are unreasonable (very) slow:

It looks you run the code from the bug report in the debug mode.

I don't see what it should matter - the problem was identified and it the testcase properly shows it.

Christian Ehrlicher

Quite old but in qt6.6 QDateTime::lastModified() gained a new parameter 'QTimeZone' where you can pass QTimeZone::UTC to avoid conversion to the local time.
See also https://codereview.qt-project.org/c/qt/qtbase/+/437009

RTEDFD

Only for QDateTime::lastModified()?

The problem exists with the other times too.

For example, birthTime() and metadataChangeTime() are affected with this issue too.

JonB

@RTEDFD
https://bugreports.qt.io/browse/QTBUG-100349 states:

Should be fixed in Qt 6.6 https://codereview.qt-project.org/c/qt/qtbase/+/437009

You can now pass a QTimeZone to any of the QFileInfo file times related methods to specify which time zone returned times should be in

So should apply to e.g. birthTime() too, but awaits Qt6.6..