Differential D10627

Reduce plasmashell frozen time to almost nothing
ClosedPublic
Actions

Authored by jtamate on Feb 18 2018, 9:41 AM.

Details

Reviewers
davidedmundson
Group Reviewers
Frameworks
Plasma
Commits
R120:e1fe26f1fc78: Reduce plasmashell frozen time to almost nothing
Summary

CCBUG: 358231
CCBUG: 342056
Even the icon with the number of tasks pending moves from time to time.

To reduce the frozen time, a similar patch must be applied also to
frameworks/kwindowsystem src/platforms/xcb/kxmessages.cpp
frameworks/plasma-framework src/plasma/private/effectwatcher.cpp

According to the documentation (and a look to the source code)
http://doc.qt.io/qt-5/qabstractnativeeventfilter.html

The type of event eventType is specific to the platform plugin chosen
at run-time, and can be used to cast message to the right type.

On X11, eventType is set to "xcb_generic_event_t", and the message can
be casted to a xcb_generic_event_t pointer.

The other eventType are "windows_generic_MSG" and "mac_generic_NSEvent".
No other eventType starts with an 'x'.

Test Plan

Cut & paste 2000 small files.
Before, a freeze (plasmashell not responding) of minutes
After, a freeze of seconds

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jtamate created this revision.Feb 18 2018, 9:41 AM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 18 2018, 9:41 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
jtamate requested review of this revision.Feb 18 2018, 9:41 AM
davidedmundson accepted this revision.Feb 18 2018, 9:43 AM
davidedmundson added a subscriber: davidedmundson.

This code already has a platform check before adding the native event filter.

IMHO we could just make this an assert.

But I like your approach too.

This revision is now accepted and ready to land.Feb 18 2018, 9:43 AM
jtamate added a comment.Feb 18 2018, 10:10 AM

May I do the other 2 commits without another revision?

broulik added a subscriber: broulik.Feb 18 2018, 4:47 PM
This comment was removed by broulik.
jtamate retitled this revision from Reduce plasma lock up time to almost nothing to Reduce plasmashell frozen time to almost nothing.Feb 19 2018, 7:31 AM
jtamate edited the summary of this revision. (Show Details)
jtamate edited the test plan for this revision. (Show Details)
davidedmundson added a comment.Feb 19 2018, 9:36 AM

I'd do reviews on the other two, because there's a mistake in your commit message:

The other eventType are "windows_generic_MSG" and "mac_generic_NSEvent".

QAbstractEventDispatcher::filterNativeEvent is public. Client code could be using that for anything, clients are weird.
We know what Plasma is doing, but that's not true for the Frameworks case.

Closed by commit R120:e1fe26f1fc78: Reduce plasmashell frozen time to almost nothing (authored by jtamate). · Explain WhyFeb 19 2018, 8:09 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mshell/screenpool.cpp (2 lines)

Diff 27577

View Options

shell/screenpool.cpp

Loading...