Group completion requests and only handle the last one

Authored by mwolff on Apr 17 2018, 10:22 PM.



I noticed that we sometimes wait quite long on code completion
requests, especially when editing header files. The edit operation
in a header invalidates the preamble, so reparsing will always take
quite some time. Nothing we can do about that, really. But at least
we can make sure we don't do work for naught:

The old code just emitted signals whenever code completion is invoked.
This can happen multiple times while we are editing the header. Since
the computation takes time, the subsequent completion requests pile
up and will all be handled eventually. If you are then impatient and
go to a different file, we will still go through all requests even
though they are not interesting anymore.

This patch groups the completion requests together with a simple
timer to ensure we only handle the last request once the current
completion job has finished.

Diff Detail

R32 KDevelop
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
mwolff created this revision.Apr 17 2018, 10:22 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptApr 17 2018, 10:22 PM
mwolff requested review of this revision.Apr 17 2018, 10:22 PM

I'd appreciate if others could give this a spin and report back whether it is improving things or not.

One big source of latency here is waiting for the header parse job to finish and thus yield the ParseSession lock... This isn't touched by this patch at all, but I still hope it improves things and ensures we don't show random outdated results.

brauch added a subscriber: brauch.Apr 18 2018, 7:16 AM

I think the change makes sense, I applied the patch here, let's see how it goes.


Can you not use QMetaObject::invokeMethod(this, &ClangCodeCompletionWorker::run, Qt::QueuedConnection) instead of this timer? (The function pointer syntax is Qt 5.10+ though I think)

ok let's see


yes, but as you say it's 5.10 - this code here is fine and a bit better than using QTimer::singleShot since that would recreate the timer continously

brauch added inline comments.Apr 18 2018, 9:31 PM

Eh, never mind me, wouldn't work -- you need the timer object for the merging, calling invokeMethod several times would still invoke the slot more than once. Sorry.

@brauch did you notice any issues? if not, can you accept this then I'll push it.

brauch accepted this revision.Apr 23 2018, 4:40 PM

Didn't do super much C++ since but didn't notice anything. Thank you!

This revision is now accepted and ready to land.Apr 23 2018, 4:40 PM
This revision was automatically updated to reflect the committed changes.