ScreenEdge: Do not use localtime for measuring duration
ClosedPublic

Authored by ahiemstra on Feb 3 2020, 12:19 AM.

Details

Summary

QDateTime::fromMSecSinceEpoch uses Qt::LocalTime by default. This involves an
expensive localtime conversion. So instead force things to use UTC, as there
is no need for timezone information when tracking durations.

This is especially noticeable on Bedrock Linux, which uses a Fuse mounted
/etc, which is slower than a plain /etc and causes quite some slowdown there.
See https://github.com/bedrocklinux/bedrocklinux-userland/issues/140 for
details.

Test Plan

The screenedge unit test still passes.

Diff Detail

Repository
R108 KWin
Branch
dont-use-localtime
Lint
Lint ErrorsExcuse: unrelated change
SeverityLocationCodeMessage
Errorutils.h:123CppcheckunknownMacro
Unit
No Unit Test Coverage
Build Status
Buildable 21982
Build 22000: arc lint + arc unit
ahiemstra created this revision.Feb 3 2020, 12:19 AM
Restricted Application added a project: KWin. · View Herald TranscriptFeb 3 2020, 12:19 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
ahiemstra requested review of this revision.Feb 3 2020, 12:19 AM

Arguably it might even be better to completely remove the QDateTime usage here and switch to something like std::chrono::time_point or so, but that is a much more invasive change.

X11Time isn't even a datetime, it's ms since the server started, so this data time isn't the best thing for that reason too!
We need to fix Qt to have a cache, it's getting ridiculous how many places we've started doing this trick.


Only one comment:

abstract_client.cpp:1333 hasn't been updated

Arguably it might even be better to completely remove the QDateTime usage here and switch to something like std::chrono::time_point or so, but that is a much more invasive change.

+1

ahiemstra updated this revision to Diff 74910.Feb 3 2020, 11:17 AM
  • Add some missing fromMSecSinceEpoch
  • Use currentDateTimeUtc in tests
davidedmundson accepted this revision.Feb 3 2020, 11:18 AM
This revision is now accepted and ready to land.Feb 3 2020, 11:18 AM

Only one comment:

abstract_client.cpp:1333 hasn't been updated

Good catch, my initial search seems to have missed that one. Added now.

Also, should this maybe go into 5.18?

zzag added a subscriber: zzag.Feb 3 2020, 11:20 AM

Also, should this maybe go into 5.18?

yes

This revision was automatically updated to reflect the committed changes.