Amarok 3 architecture brainstorm (was: GSoC considerations)

[Matěj Laitl]

On 12. 4. 2013 Edward Toroshchin wrote:
>     My personal view on this point being: the engine, media database and UI
>     (at least) must be separate processes, so that:
> 
>      + at least the first two can be made rock-solid, valgrinded,
>        unit-tested, etc.,
>      + the data structures and flows are strictly defined,
>      + no bullshit a la "we can't use class XXX from YYY thread if ZZZ",

That won't go away, unless you disable threading entirely (which you probably 
don't want, moving all our async code to separate processes could be a lot of 
work). But see below.

>      + theoretically, any component should be completely replaceable.
> 
>     I'll try to describe the benefits (as I see them) of these changes
>     below (if you bear with me).
> 
>  * which parts of the existing code can be spared and reused, and which
>    would rather be killed with fire;
>  * and so on.

I think I share 100% of the problem statement with you. In my words, our 
"components" are way too much interdependent and therefore fragile to 
seemingly unrelated changes to the extent that it makes development tricky.

I also share that our components (and I mean components like "playback", 
"playback queue model", "collections model", "QWidget gui") must be made more 
independent, replaceable and behave according to more well-defined semantics. 
Our heavy usage of the singleton pattern makes it only worse.

I also agree that we should have a technical measure to actually ensure the 
independence, not just encourage it and hope it happens.

OTOH I don't share the idea that multi-threading is inherently bad. For me, 
multithreading *is* the fun part and classes like ThreadWeaver::Job make it 
easy. The main source of problems we may currently have with multithreading is 
IMO that many parts of Amarok design and implementation didn't anticipate it 
at all, and were perhaps written by developers that didn't know much about it. 
However I think our code-base got a lot better in this regard compared to 
let's say 2 years ago.

> Now, as promised, my view on the process-based architecture.
> 
> Although switching to IPC may seem over-complicated, I think it would be
> actually much easier to live with than our current thread zoo.
> Currently, before any tiny bit of code can be changed, a lot of things
> have to be considered, which raises the entry difficulty for developers
> a lot (and we're not in the best of situations there ATM).
> 
> So here are the benefits, as I see them (in a loose order of decreasing
> importance):
> 
>  * it would be easier to debug and develop upon individual components
>    (hence, improving the quality, userbase, and developer community),

I think this is tied to more general idea of better independence of the 
components rather than to IPC suggestion specifically. When it comes to 
debugging, I think this is very subjective and varies case-by-case, for 
example debugging out-of-process kio slaves in gdb sense is all but pleasant 
[1].

>  * database would be easy to replace (mysql, nepomuk, whatever), it
>    might even become plausible once more to maintain several backends
>    (although a sql and nepomuk would probably be the favorites),

Again not specific to IPC, but of course a goal we want to achieve and we 
hopefully are already marching towards that direction. The direction is that 
SqlCollection will be a plugin equal to all others (you would be able to 
disable it, etc), esp. to NepomukCollection in future.

>  * the playback engine would do only playback, do it well, and not fuck
>    up anything else (I believe currently the first question that's being
>    asked in a bug report is 'have you tried changing the playback
>    backend'),

Yup, EngineController should be made a proper component with and Abstract Base 
Class declaration and defined semantics. And I think a lot of its current logic 
is reversed (i.e. EngineController asks Playback Queue model to feed it with 
the next track. Hello? Why does it even know that Playback Queue exists? It 
should just signal "I'm about to finish the current track")

The comment about bug is true, however that question is sometimes asked 
needlessly.

>  * UI might be replaceable. We might want to have a variant of an UI for
>    low-performance systems. Web UI (someone mentioned recently)? Easy!

Exactly. I'd love if our UI was just a component (although a big one) perhaps 
called "QWidget MainWindow". One day we might have "QML MainWindow" or 
whatever.

>  * the environment and resource usage could be contained within each of
>    the processes. Think MySQL (all the exit()'s, all the *argv, all the
>    pain and misery and 500-megabyte caches for no reason).

Hmm, right, but what would be an advantage of that? I think that summary 
memory usage of the IPC solution would be higher - a little duplication of 
otherwise shared data is IMO inevitable.

> Of course, this may not require splitting the components into processes
> per se.

Yes, this is exactly my point.

> But I still think the split is important, because it requires a
> good fundamental design, and confines the development to it. Which is a
> good thing.

I agree 99%, I just suggest that "some split" may not necessary be IPC design. 
See also [2] for some background info on IPC design and why it may not be the 
best thing under sun, article by our lovely Aaron Seigo.

I consider rude just criticising without offering and alternative, so here's my 
suggestion, something I've been toying with for 6 months. Please spend a few 
moments thinking about it.

Everything Generic is a Component
===================
 * code would be (re)split into "core" and everything else, where everything
   else would be an implementation of a component (or a part of it); core
   would contain just:
    * well documented declarations for components (abstract base classes for
      e.g. Logger (done; subject to rename to Notifier), CollectionManager,
      EngineController, PlayBackQueueModel, mainwindow ui, config ui, tray
      icon, OSD, ScriptManager)
    * declarations for common abstract classes like: Meta::Track & friends,
      Collection, CollectionLocation, Playlist, Observer ...
    * a sort of "application controller", let's call it Amarok::Components
      that would manage the components
    * each component would have 2 accessors: strong and weak. strong accessor
      would create the component if it is not already alive. It would be the
      only way to instantiate a component.
 * A component is not a singleton. It is memory-managed (using exclusively
   Q{Shared,Weak}Pointers), created on demand and destroyed when not in use.
   Components can depend on other components in 2 ways:
    * strong dependency: it calls the strong accessor in own constructor and
      holds a QSharedPointer. For example PlaybackQueue would depend strongly
      on EngineController. Used with caution; circular strong dependency is a
      fatal error.
    * weak dependency: the master component may call the weak accessor,
      getting may-be-null QWeakPointer, may store the weak pointer. It
      temporarily promotes the weak pointer to the strong one when it needs to
      call slave component methods. The master component can by notified about
      slave component creation/destruction via Amarok::Components.
 * In fact, nothing is a singleton. Amarok::Components is created in main.cpp
   and is parented to QApplication. Only utility singletons internal to
   a single component are allowed.
 * things like SqlCollection are not components. This one is considered a part
   of the "CollectionManager" component, though implemented in an extra shared
   library. I.e. there should be only one component of a given *type* at a
   time, but perhaps multiple implementations of a type, and you don't know
   which one you've got.
 * in other words, "core" would be our API (with no stability guarantee) and
   everything else will use/implement it. How this could be enforced? By
   linking
    * currently, everything links at least to amarokcore and amaroklib and
      amaroklib contains virtually everything including the GUI. This is very
      prone to adding interdependencies.
   * in this proposal, amaroklib will cease to exist. Much of it will be moved
      to "QWidget MainWindow", some declarations to core and some
      implementations to separate modules.
   * amarokcore will be the only Amarok thing to link to, therefore ensuring
      that only well-defined public APIs are used.

Of course, this is a very long road, but at the end, we should get the 
following for free:
 * when you minimize Amarok to tray, the whole mainwindow component could be 
deleted; starting Amarok to tray should be blazing-fast as the bulk of gui 
won't be loaded until you open it.
 * when you stop playing, the enginecontroller and perhaps all phonon thingies 
could be destroyed. (questionable)
 * porting Amarok to let's say smartphones should be as easy as providing 
alternative implementation for "GUI" and perhaps "EngineController" 
components.

The single most important advantage if this approach is that it could be done 
*gradually*. Other advantages of modularization mentioned by Edward should 
still hold.

What do you think? This is of course a rough draft and I have omitted some 
details I've thought about.

Regards,
		Matěj

[1] http://techbase.kde.org/Development/Tutorials/Debugging/Debugging_IOSlaves
[2] http://aseigo.blogspot.cz/2013/02/process-separation.html