Don't stat(/etc/localtime) between read() and write() copying files
AbandonedPublic

Authored by jtamate on Jan 12 2018, 4:08 PM.

Details

Summary

CCBUG: 384561

Unfortunately, QDateTime::currentDateTime() checks /etc/localtime
each time it is called.
Chaning to QElapsedTime, no check of /etc/localtime.
Reproducing bug 384561, the strace of file.so was something like:
read(), stat(/etc/localtime), stat(/etc/localtime),
stat(/etc/localtime), stat(/etc/localtime), stat(/etc/localtime),
write(), read() ......
Now it is: read(), write()
It also reduces the cpu in io/wait around 10% in a debug build.

Diff Detail

Repository
R241 KIO
Lint
Lint Skipped
Unit
Unit Tests Skipped
jtamate created this revision.Jan 12 2018, 4:08 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 12 2018, 4:08 PM
jtamate requested review of this revision.Jan 12 2018, 4:08 PM
ngraham edited the summary of this revision. (Show Details)Jan 12 2018, 4:57 PM
ngraham added a subscriber: Dolphin.
ngraham edited the summary of this revision. (Show Details)Jan 12 2018, 5:00 PM
ngraham added a subscriber: ngraham.
dfaure accepted this revision.Jan 14 2018, 12:19 PM

Well spotted!

This revision is now accepted and ready to land.Jan 14 2018, 12:19 PM
broulik added inline comments.
src/core/slavebase.cpp
117

Init with 0 in the constructor?

This revision was automatically updated to reflect the committed changes.
jtamate added inline comments.Jan 14 2018, 12:44 PM
src/core/slavebase.cpp
117

Could it be done without a review? I was busy doing arc land (with some problems) and I didn't notice.

broulik added inline comments.Jan 14 2018, 12:48 PM
src/core/slavebase.cpp
117

Sure

jtamate marked an inline comment as done.Jan 14 2018, 12:54 PM
fvogt reopened this revision.Jan 15 2018, 11:33 AM
fvogt added a subscriber: fvogt.

Reverted in master as this broke various important ioslaves, like desktop and trash.

I believe the issue is that the timeout fires immediately after construction, without being configured by the slave.

This revision is now accepted and ready to land.Jan 15 2018, 11:33 AM
jtamate updated this revision to Diff 25391.Jan 15 2018, 12:37 PM

I'm sorry for the inconvenience.
I'm going to divide the patch in two, the first part only changes lastTimeout and should be safe.
nextTimeout part needs more thinking (if can be done).

jtamate abandoned this revision.Mar 2 2018, 1:05 PM