[Panel-devel] Multithreaded krunner

[Ryan Bitanga]

> > Should I continue looking for solutions to this? Would it be
> > acceptable to add a requestAbort() method and an isAborting() method
> > to AbstractRunner, a bool m_abort to AbstractRunner::Private, then
> > require placing if( isAborting() ) statements in critical parts of
> > match() methods?
>
> i like this, even if it does muddy the API a wee bit, as it allows us to
> prevent as much processing as possible (e.g. no "do the entire match, then
> discard the results if we've moved on already"). the question, though, is
> where the isAborting() checks would go. are the runners able to chunk the
> processing meaningfully?
>
> there are a lot of for/while type loops in the runners matchings, but those
> are often just processing results passed back by ksycoca or search indexes;
> which means that the interuptable bits may not be overly interesting.
>
> would take some experimenting i suppose.
>
What I don't like about this solution is authors of runners have to
know where to place the checks. The code won't look as nice and I'd be
more comfortable with them only focusing on what the runner needs to
be done. But this should perform better than the other solution
because CPU time won't be wasted for unneeded matches.
>
> this would imply adding a set of "void add*Matches(QList<SearchAction*>)" (or
> SearchMatch*, if we make that change as well which it looks like we are)
> methods to SearchContext. we could even remove the add*Match() methods from
> SearchContext entirely (having moved them to AbstractRunner instead).
>
> the nice thing about this approach is that it lowers contention over
> SearchContext dramatically, from NumberOfMatches to NumberOfRunners * 3. we
> could even make the API less pretty and have:
>
> SearchContext::addMatches(const QString& term, QList<SearchMatch> exact,
> QList<SearchMatch> possible, QList<SearchMatch> informational)
>
> and bring it down to NumberOfRunners. this would also allow us to get rid of
> the locks in most of the SearchContext methods, only having to make the copy
> constructor thread safe. that should help with the locking, no?
>
Yes, this greatly reduce the penalties incurred by using locks. Only
the call(s) to add(*)Matches would need to be thread-safe. I'm not
sure if the constructor would need to be made thread-safe. Either a
local mutex in performMatch or a copy constructor would be sufficient.
The problem is SearchContext is a QObject, and QObjects can't be
children of objects in a different thread. We could create another
constructor, say SearchContext( QObject* parent, const SearchContext&
context) to get around this. The new constructor will have to be
thread-safe.

Then  AbstractRunner::performMatch(SearchContext &context)
 {
     SearchContext localContext(context);
     match(localContext);
...
}
would become
AbstractRunner::performMatch(SearchContext &context)
{
    SearchContext localContext = new SearchContext( 0, context);
    match(localContext);
...
   delete localContext;
}

The only drawback I see with this is that if it takes the runner a
long time between finding matches, krunner will seem to be less
responsive. But I can't really think of an example where a runner take
a noticeable amount of time between finding matches so it's probably
worth the trade-off.

I'll see if I can write patches for both solutions then post them here
for comments.

Cheers,
Ryan