Re: [Rhythmbox-devel] State Of The Rhythmbox (3)

[Joshua Haberman <joshua haberman com>]

I'm getting a little bit lost by not knowing the details, so let me try
to understand what you're proposing.  Tell me if any of this is wrong or
inaccurate.

RhythmDB is an abstraction that provides these services to Rhythmbox:
* storing a set of songs and associated attributes
* adding individual songs to this database
* querying this set of songs by a number of different criteria

Each song in RhythmDB is represented as a RhythmDBEntry, and are added
to the DB as such, but results from queries are returned as
GtkTreeModels.  The UI uses these GtkTreeModels directly as the source
data for the GtkTreeViews (but how?  the main_model GtkTreeModel only
has one column: a pointer to a RhythmDBTreeEntry.  How can a GtkTreeView
display this directly?  Do you subclass GtkTreeView to know about
RhythmDB?)

RhythmDB will emit signals when the library changes, but apparently it
needs the GDK lock to do this!  Why?  And who is intended to catch these
signals?

RhythmDB is only an API: it needs an implementation to actually do the
work.  You have implemented one such subclass: RhythmDBTree, which
stores the DB information in an XML file.

RhythmDB is multithreaded because it needs to be able to provide
services to all of these uses:
* Songs added to the DB by a background thread when the user uses the
"Add to Library" command
* Song attributes updated or songs deleted by a background thread when
Rhythmbox refreshes the DB on startup
* Queries performed by the main thread
* Song attributes updated by the main thread when the user modifies song
properties

When a RhythmDBEntry is added, changed or is deleted, it is neccesary to
emit an appropriate signal so that the GtkTreeViews can redraw
accordingly.  Emitting these signals will cause a synchronous redraw, so
it is necessary to hold the GDK lock to do so.

I'm still fuzzy, but if this much is semi-true, then I have somewhere to
start.

On Sun, 2003-09-07 at 10:35, Colin Walters wrote:
> On Sun, 2003-09-07 at 04:56, Joshua Haberman wrote:
> 
> > Does it really need to be this complicated?  
> 
> Well, it's not a simple situation.  Essentially we have two databases. 
> One is being displayed by Rhythmbox (the GtkTreeView/GtkTreeModel
> backing).  Another (the multithreaded RhythmDB) is being manipulated by
> various threads.

How many threads are currently in the design and what do they all do? 
How do they communicate and synchronize (roughly.  I know it's a big
question)?


> > Why introduce other threads?  So that each thread can block on a
> > different resource so no resource lays idle.  The blocking part is all
> > the sub-threads really *need* to do.  They can then communicate their
> > results to the main thread through an asynchronous queue that the main
> > thread can poll in its event loop. 
> 
> I have thought about it more, and I think - in essence - you are right. 
> The main thread should have a polling model, because that means that if
> a bunch of updates happen, the main thread can just take care of them as
> fast as it can, rather than being pinged on each one.
> 
> However, I can't think of a sane way to implement this without modifying
> GtkTreeView.  The part of the main thread we care about for this
> discussion is the GtkTreeView, which is using a GtkTreeModel returned by
> RhythmDB as a model.
> 
> How can I have the main thread poll for changes in this circumstance? 
> Essentially what I want is for it to call a function to process a queued
> change  (or a group of them say), before actually calling any of the
> gtk_tree_model_* functions like gtk_tree_model_get_value.

I don't quite understand the problem.

1. Why would you need to call the gtk_tree_model_* functions before
processing queued changes by emitting the appropriate signals?
2. Why is it harmful to call the gtk_tree_model_* functions before
processing queued changes?

>   I don't think
> I can have the model itself emit signals like gtk_tree_model_row_changed
> inside an implementation of gtk_tree_model_get.  
> 
> Given that lack - maybe what we need for now is another thread dedicated
> to processing the queue of db changes and updating the UI.

I'm skeptical that another thread is necessary here.

>   Basically,
> its code would look like this:
> 
> GDK_THREADS_ENTER ();
> 
> RhythmDBUpdate *data;

What data does this RhythmDBUpdate structure carry?

> GTimeVal start;
> g_get_current_time (&start);
> g_time_val_add (&start, G_USEC_PER_SEC * 0.75);
> if ((data = g_async_queue_try_pop (queue)) != NULL) {
>   GTimeVal now;
>   process_update (data);

This emits all the row_{changed,inserted,deleted} signals?

>   g_get_current_time (&now);
>   if (compare_times (now, start) >= 0)
>     break;
> }
> 
> GDK_THREADS_LEAVE ();
> 
> So the basic idea is that process_update will look at a RhythmDBUpdate
> structure, which would say which RhythmDBEntries have been
> added/changed/deleted, and emit
> gtk_tree_model_row_inserted/row_changed/row_deleted as necessary.
> Note also we try to do as many updates as possible within 0.75 seconds.

What does this thread (as you have written it) do that the main thread
cannot?

> One other thought occurs to me - if you were trying to implement
> RhythmDB with an SQL database, I'm not sure how you could provide the
> kind of fine-grained notification like "This particular entry changed"
> (maybe with some sort of database-specific trigger or something?  I
> don't know enough about SQL databases to say.)

If I'm understanding your question correctly, I think the easiest way
would be to add information to the database row indicating whether the
GUI needed to have this row refreshed.  Either a boolean column called
"needs_redraw" or a timestamp column that you could compare with the
last time you processed GUI updates.

> > The sub-thread does only what it has to, and therefore the application
> > is simpler.  In particular, I think it is good design to have only one
> > thread perform GUI operations (probably the main thread).
> 
> I think that unfortunately with GTK+ we have no choice really.

What do you mean?  Are you saying GTK+ only allows you to perform GUI
operations in the main thread?  I thought that this was the purpose of
the GDK lock, to allow GUI operations from other threads.

> > For reference, my experience comes from rewriting Audacity's audio i/o
> > subsystem to be multithreaded.  When recording or playing, Audacity uses
> > 3 threads:
> > 
> > 1. the main thread, which is the only thread that performs GUI
> > operations
> > 2. the disk thread, which blocks on disk i/o of audio data
> > 3. the audio thread, which blocks on i/o to the soundcard
> 
> Well this is a different problem really, but it would be a good idea to
> do this for Rhythmbox too.

I didn't mean to imply it was the same problem, just illustrate a
situation where the GUI is polling for backend changes instead of the
backend updating the GUI.  And as far as implementing this disk/audio
model for Rhythmbox, I was under the impression that all disk and audio
i/o was handled by Gstreamer !

If nothing else, I hope this discussion is helping to solidify your
design ideas for RhythmDB by making you explain it to a GTK+ n00b like
me :)

Josh