Homerun sources and Plasma runners

[Aaron J. Seigo]

On Wednesday, November 21, 2012 13:57:10 Aurélien Gâteau wrote:
> Le mardi 20 novembre 2012 20:16:17 Aaron J. Seigo a écrit :
> > the question is which of the following makes sense:
> > 
> > * AbstractRunner should be a model (BIC)
> > * AbstractRunner should export a model (SC & BC)
> > * RunnertContext should provide a model for RunnerManager to expose (SC &
> > BC) * A Qt Model should sit on top of RunnerManager (SC & BC)
> > 
> > assuming we don't need the first option, and i keep noting that we don't
> > (really, we don't), i really don't understand what you mean by your first
> > paragraph.
> 
> I am not advocating for the first option. I would like something between 2nd
> and 3nd option.

3rd is my current preferred; 2 is possible .. though probably ugly in its own 
ways (especially when considering what to do about synchronizing search 
results for different query terms across runners)

> Homerun AbstractSource class is not a model. It inherits from QObject and
> provide virtual methods to create models. A source must implement at least:
> "QAbstractItemModel *createModelFromConfigGroup(const KConfigGroup &group)"
> and optionally:
> "QAbstractItemModel *createModelFromArguments(const QVariantMap &args)"

yes, AbstractSource is essentially a factory. the API is very questionable 
imho, however.

what are the args? the config group? when are they written? restored? what 
standards are there for this? it's all very add-hoc.

who owns the model pointers? can i have more than one model? can i share the 
models creatd for the same args?

it's not the sort of API i'd like to see re-used due to this.


of course, not everything is roses in AbstractRunner, either. one thing i 
really don't like is how small objects (matches) get made so often. i fear it 
leads to more memory fragmentation than would be good, though this has not yet 
been measured. a caching mechanism would be very helpful there .. at which 
point it would approach the efficiency of data-backed-models.

i'd love to even have the ability to easily associate sets of matches with 
classes of matches internally in the runner. this would allow runners with 
stock sets of matches to create them once, sort them into sets (with matches 
belonging potentially to multiple sets) and then return the sets on match. 
this would likely speed things up and reduce memory fragmentation, though at 
the cost of some (small amount) of memory usage.

this would be completely ignored by runners which can only be 100% dynamic, 
such as the nepomuk runner.

however, the API that is there works and is maintainable. what it needs is 
some additions, such as the ability to update matches and perhaps some match 
caching.

> Trying to map the way sources work to runners, instead of having a runner
> calling RunnerContext::addMatches(), a runner would have a createModel()
> method which would be called by RunnerContext or RunnerManager to reach the
> runner data.

how would this be thread safe? or change on query updates?

> This gives runners the ability to use whatever model class is more
> appropriate: a model inheriting QAbstractListModel, or a simple
> QStandardItemModel, or maybe a proxy model on top of an existing model
> provided by a library, etc.

the time i can see this being of benefit is when proxying another model. 
otherwise, there's no benefit to be had as the results need to be uniform 
*anyways*.

given how few runners proxy models, this seems to be a non-issue.

> "Also, all matches need to be reported once this method returns.
> Asynchronous runners therefore need to make use of a local event loop to
> wait for all matches."
> 
> But OK, I am not going to pretend all my docs are always up to date.

well, this documentation is accurate, though it does not note that you can in 
theory add actions after match() is done .. and for good reason with the 
current API:

once match() returns, the thread may be culled due to subsequent searches and 
search actions (e.g. "run the first match" as used in krunner when typing text 
and hitting Enter before anything is shown) may be taken as soon as it 
returns. so any matches that the runner wants in the results set need to be 
created in match() .. but it is possible to add more later. the guarantees 
around what happens with them changes or goes away.

in any case, it's impractical to actually take advantage of this right now due 
to the lack of a way to update matches in the RunnerContext API. so if/when 
that gets added, this apidox will indeed need updating.

> > > Considering prior art, Kickoff and Folder View are not built on top of
> > > runners either (true, Kickoff uses runners to implement searching, but
> > > not
> > > for browsing), so I am not sure why runners would be appropriate for
> > > Homerun but not for them?
> > 
> > you picked two of the oldest plasmoids which also happen to have the most
> > complex internal implementations.
> 
> Well, I picked the ones which have similar features as Homerun, but fair
> enough, history often gets in the way of the simplest solution.

the ones most similar to homerun are Search and Launch and Plasma Active's 
Contour. both of which use runners.

> > homerun appears to present, predominantly, a set of query results. these
> > could easily be done with runners.
> > 
> > the exception is when it goes to list full directory contents. and as i've
> > stated elsewhere, imo this should be done with a KDirModel directly
> > without
> > any wrapper around it.
> > 
> > with some necessary additions to the runner functinality in libplasma, i
> > don't see anything in homerun, other than directory listings, that could
> > not be done as well with runners as it is now.
> 
> That would mean different APIs to provide content... I am not sure I want to
> go in that direction.

if you want one set of roles from the model in homerun no matter what model is 
being used for the view, then one could:

* bridge KDirModel + runners as an internal implementation detail to homerun; 
runners remain as they are.

* put KDirModel behind an API and use that API also for all other listings.

obviously, homerun is doing the latter right now and it means that we end up 
with sources and runners. sources are not useful for something like krunner, 
so we live with a split.

alternatively, homerun could simply handle directory listing models and runner 
models differently; we do similar things elsewhere with QML and it makes it 
very easy to do so, unlike in C++

Search and Launch and Active both show that using runners works for listing 
based UIs. while i can certainly imagine a more efficient / direct method for 
listing based UIs, the larger design goal of "having universal components for 
finding these things that works in both search and listing environments" trumps 
that.

it all depends on what you're optimizing for.

it appears you are optimizing an API for using kdirmodel with other things 
conforming to that type of model. i'm interested in an API that provides 
heterogeneous listings treated as a specialized case of search, where full 
directory listing is treated as a separate use case to be handled separately.

what makes this less clear is that some things clearly straddle the 
boundaries: bookmarks, for instance.

other things already have models, so it is tempting to simply proxy them.

conversely, many things which have models on top are no better off than done 
with the "specialization of search" mechanism, as can be clearly seen by the 
various application menu hierarchy models that exist.

> > this makes very little sense. sources are models which are not written in
> > a
> > threadsafe manner to be used from runners which are built on a threaded
> > model. behind the scenes they all access similar / the same convenience
> > API.
> 
> I actually don't like the idea of trusting every runner code to be thread-
> safe. This is all the more dangerous when Homerun is running as a
> containment because in this case the Homerun process is the Plasma Desktop
> process.

when was the last time we saw kickoff going down because of this? it's the same 
issue.

what would be even worse is having to wait for results in the GUI thread. 
right now, all runners get threading "for free"; they just have to ensure that 
their match() method is thread safe. 

some runners/listers really need to do their work in another thread to prevent 
the UI from stopping for long periods of time. having just finished off fixing up 
some things in the new metadatamodel code, it is clear how non-trivial this is 
to get right.

which means most people won't even do so, or will do so poorly. and the 
results will be even *worse* -> non-threaded blocking code with horribly 
jittery UI, bugs in thread management code, etc.

> I want to experiment with writing a proxy model which would run
> its source model in a separate process. Don't know how well it is going to
> perform, but I am going to give it a try. If it is good enough it would be
> IMO a safer alternative to having heavily multi-threaded code.

so in the krunner case where we match during typing we'd spawn a set of 
processes for each keystroke?

and all the IPC would also be smooth? our experience is that such IPC really 
hinders the fluidity of the UI.

> > search on sources is implemented apparently by filter models, which means
> > having to load entire datasets into the model and then filter based on the
> > data .. rather than querying first to pull just the data needed.
> 
> That is not correct. A basic source can indeed provide a model which expose
> all its data, Homerun will then provide a generic filter on top of it. But a
> source can also provide a model with a "query" property, in which case the
> model is responsible for implementing searching/filtering itself.

so models provide properties that differentiate their capability? making some 
searchable, some not, some done way, some another?

-- 
Aaron J. Seigo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20121122/7295a11c/attachment.sig>