Compress calls to configuration slots upon new connections
Needs ReviewPublic

Authored by jacopods on May 28 2018, 5:54 PM.

Details

Reviewers
hein
broulik
drosca
Group Reviewers
Plasma (Plasma 5.13)
Summary

Some input devices register several copies of themselves upon connection (e.g. a mouse registers as a keyboard as well, or a keyboard registers twice as a keyboard). Hence the keyboard/mouse configuration is called multiple times for no reason.

This commit compresses the configuration calls using a singleshot timer.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
jacopods created this revision.May 28 2018, 5:54 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 28 2018, 5:54 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
jacopods requested review of this revision.May 28 2018, 5:54 PM
jacopods updated this revision to Diff 35045.May 28 2018, 6:00 PM

Fix compile

drosca added a comment.EditedMay 28 2018, 6:04 PM

Is there any reason for this, except save some cycles?

jacopods added a comment.EditedMay 28 2018, 8:42 PM

Short story: It just feels more right to me to compress several configuration calls in one, although it does not give a perceivable advantage in terms of speed or anything (such calls happen quite rarely as a matter of fact)

Long story:

In fact, I have been experimenting a bit with the daemon. One of the things that I experimented with was to let it execute a script to fix some corner cases (xkbcomp +xcape users). In case the configuration bit is run several times simultaneously we can get into some race conditions, so I figured that one needs to compress the calls.
Since I don't see why one wouldn't want to compress the calls even without the scripting (what is the point of running xmodmap 5 times in a row with the same parameters?) I have proposed this patch. I don't think it adds too much complexity and it feels cleaner.

mart added a subscriber: mart.May 30 2018, 8:38 AM

are configureKeyboard configureMouse etc doing an immediate write to disk? (like KConfig::sync()
where using timers to compress really saves a lot of time, is to compress the actual disk writes. just writing to kconfiggroup is fast, because it doesn't actually immediately write to disk, but only on application (clean) exit or at a sync() call

In D13178#270671, @mart wrote:

are configureKeyboard configureMouse etc doing an immediate write to disk? (like KConfig::sync()

Hey notmart, long time no see!

The configureKeyboard() method is calling xmodmap (should the user need special configuration), so who knows what happens in there.

mart added a comment.EditedJun 7 2018, 10:35 AM
In D13178#270671, @mart wrote:

are configureKeyboard configureMouse etc doing an immediate write to disk? (like KConfig::sync()

Hey notmart, long time no see!

The configureKeyboard() method is calling xmodmap (should the user need special configuration), so who knows what happens in there.

(long time no see indeeed :)

yup, then a compresstimer is probably a good idea there

broulik added inline comments.Jun 7 2018, 10:37 AM
kcms/keyboard/keyboard_daemon.cpp
163

Add this as context (third argument) to have it auto-disconnect when this is destroyed, else you'll crash:

connect(xEventNotifier, &XInputEventNotifier::newPointerDevice, this, [this]() { configureMouseTimer.start(); });

Or you could perhaps even connect directly (assuming arguments match)

connect(xEventNotifier, &XInputEventNotifier::newPointerDevice, &configureMouseTimer, &QTimer::start);
jacopods updated this revision to Diff 36031.Jun 12 2018, 4:05 AM

I used the direct connection as suggested.

Today I learned about QOverload...

mart added a comment.Nov 12 2018, 10:53 AM

still needs this parameter then is good to go