[Panel-devel] Multithreaded krunner

[Aaron J. Seigo]

On Thursday 22 November 2007, Ryan Bitanga wrote:
> > > Changes I snuck in:
> > > - change canBeConfigured() to isConfigurable() for the sake of API
> > > consistency
> >
> > looks good. this part should be committed regardless.
>
> Committed the whitelist and the cosmetic change to canBeConfigured.

yes, i noticed on the commits list. thanks =)

> > > - KRunner may not be responsive if the query is changed while the
> > > match method of a slow runner is being executed by threadweaver.
> > >   This is because ThreadWeaver doesn't abort jobs that are currently
> > > being run and aborting jobs requires the implementers of runners to
> > > have knowledge about ThreadWeaver jobs.
> >
> > hm... how much and what kind of knowledge? because this probably isn't
> > worth doing unless we can make the UI completely fluid.
>
> 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.

> Another solution would be for Runners to cache the query by storing it
> in AbstractRunner::Private::term then pass the cached query to
> addPossibleMatch(), addExactMatch(), and addInformationalMatch(). If
> the query is not the same as the current query, the request can be
> safely ignored. The only problem would be deleting the match.

deleting the match wouldn't be a problem if they are also cached in 
AbstractRunner, though. e.g. if there was AbstractRunner::add*Match methods 
they could be cached locally first, then applied in one batch.

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).

then AbstractRunner can have a non-virtual method (performMatch or some such) 
that KRunner's Interface calls that looks something like:

AbstractRunner::performMatch(SearchContext &context)
{
    SearchContext localContext(context);
    match(localContext);

    if (!d->exactMatches.isEmpty()) {
        context.addExactMatches(localContext.searchTerm(), d->exactMatches);
    }

    .. etc for other lists ..
    clearMatches();
}

SearchContext would then obviously check the term first ... we could even pass 
in the whole localContext and let SearchContext deal with what it means to 
be "the same".

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?

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Trolltech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20071122/3b7d54ad/attachment.pgp