[Development] [fix]: Assistant (and others) burning CPU because of #f27d1ccbb24ec2fd4098f2976503478831006cc8 ?

[Volker Krause]

On Thursday, 31 August 2017 00:13:43 CEST Thiago Macieira wrote:
> On Wednesday, 30 August 2017 14:17:05 PDT René J. V. Bertin wrote:
> > Thiago Macieira wrote:
> > 
> > Oops, missed this one.
> > 
> > > Unfortunately, we can't change anymore.
> > 
> > Can someone explain what could possibly be broken by changing this? Can
> > the
> > current base-level QObject::timerEvent() be used for anything constructive
> > other than heating the office or preventing a CPU core from going to
> > sleep?
> > I only made my suggestion because it seemed rather evident that anyone
> > using the QObject timer would override the timerEvent() method.
> 
> It makes QTimer possible.
> 
> void QTimer::start()
> {
>     if (id != INV_TIMER)                        // stop running timer
>         stop();
>     nulltimer = (!inter && single);
>     id = QObject::startTimer(inter, Qt::TimerType(type));
> }
> 
> That sounds quite important to me :-)
> 
> QBasicTimer also used it until f821d6352e384a737c82a0db903104ec4f8611bd,
> when Brad wrote "QBasicTimer becomes even more low-level. It cannot use
> QObject::startTimer() anymore, since we do not have a way to call
> QObject::killTimer() from QBasicTimer::stop()."
> 
> On a serious note, there's a lot of code that uses startTimer() to get
> timers more efficiently than QTimer or even QBasicTimer. We can't remove
> this function, because it would break the existing code. And we can't make
> it a single shot because it would be even worse: it would silently would
> equally break existing code that expects the timer to repeat itself. This
> code is probably buried deep into codebases several years old and not that
> trivial to find.
> 
> And I don't want to add a flag to say it's a single shot to the event
> dispatcher, because it would add overhead for all existing timers. Not to
> mention it's not doable until Qt 6 when we can add or modify virtual methods
> to QAbstractEventDispatcher.
> 
> > That said, it does seem a bit overkill to give each and every instance
> > inheriting QObject timer features. Most will presumably never use that in
> > a
> > typical application, would they?
> 
> Most won't. How do you choose which ones should get the feature? If you're
> arguing for a white list, please compile such al ist, including all uses in
> KDE code, all applications, etc.
> 
> And how would we even achieve this? The only way I can think of is to make
> those functions private (a Qt 6 change) and add the whitelisted classes as
> friends.
> 
> > Found the bug/fix? The usual way, reverting all changes and restoring them
> > in order of least likely culprit. Then by generating trace output,
> > evidently I thought last of putting it *before* the timer ID check, and
> > then I had to consult the docs to confirm that one indeed needs to kill
> > the
> > timer when done. I'd never have guessed that a noop event handler callback
> > could have pump this many resources.
> 
> Ok, so you found by git bisect (or similar) action. I was hoping you had
> some interesting way of debugging that the problem is timers constantly
> firing. I woder if GammaRay can give you a list of timers active in a
> thread and which objects it's connected to. Volker?

There's the timer inspector in GammaRay, which indeed should help in this 
scenario. It's not the most polished tool, but it can list all QTimer and QML 
Timer instances, as well as all QObjects receiving timer events, and the 
corresponding timer frequencies.

For timer objects it allows you to switch to the corresponding view in the 
object browser (via context menu), where you see the outbound connections 
(this seems broken for timer event receivers atm). We don't have a filter per 
thread yet though.

Regards,
Volker
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4664 bytes
Desc: not available
URL: <http://lists.qt-project.org/pipermail/development/attachments/20170831/902ad27d/attachment.bin>