[Rhythmbox-devel] Rhythmbox memory patch, and a (long) discussion



G'day everyone,

I've been having a go at finding leaks, and bad memory usage, in RB, and
seeing where it is using all it's memory. I've atatched a patch which is
the collection of a reasonable number of small improvements I've been
doing over the past few days (it's described below), but it probably
needs some testing to make sure it doesn't break anything (hopefully it
doesn't).



Also below is a discussion of what I've found out about where RB's
memory goes. It's fairly long, but hopefully someone might be able to
use this to think of some novel ways of reducing RB's memory usage.


The Patch
---------

The attached patch quite a couple of things; if anyone thinks any these
are a bad idea, or could be done a better way, let me know.

a) fix a couple of memory leaks

b) remove some pointless re-allocation of memory; this is mostly where
we duplicate a string, call a function that will copy it, and then
release our duplicate.

c) use memchunks in more places: for sequence nodes, rhythmdb actions
and rhythmdb events. This helps lower heap fragmentation (and reduces RB
total heap size, because of it).

d) actually call g_blow_chunks() so that empty memchunks are released.
You've gotta love that function name :)

e) not commit the db until it is completely loaded, rather than after
each entry. This means no entries will be stat'd until after it is
loaded, which could be bad if the xml files is on a slow media. However,
this stops *massive* heap fragmentation, which is caused when
allocations of stat actions (transient) are alternated with db entries
(permanent).

f) use RbRefStrings in a couple of places, rather than getting the
contained string and then duplicating it. This changes the return type
of the get_artist and get_album methods of RbSource, but these are never
actually called anywhere, and library-source is the only thing that
implements them.

h) has some memory analysis stuff in there that we wouldn't actually
commit (I left it in, in case someone wanted to play with it): if you
run with -d, you will get reports on the memory usage by various
memchunks and rb-refstrings.



Rhythmbox memory analysis
-------------------------

First a disclaimer that all the numbers here are for my library and
machine, so should be taken with a grain of salt, but are still somewhat
useful. For what it's worth, my library is around 3500 songs, half ogg
vorbis, half mp3 and a couple of aac/mp4 tracks. I currently have 4
automatic playlists, which combine to 3000 entries (songs in two
playlists counted twice).

All of this is done with the current cvs version, which means that
several (big) leaks I fixed last week are not included.

Tools used: valgrind (especially massif and also memcheck), the heap
fragmentation analyser posted to the evolution list a couple of months
ago and gnome-system-monitor


When RB running, playing a song and generally being used, RB has ~80Mb
of address space. ~15Mb of this appears when you start playing a song,
and is GStreamer buffers, elements, decoders, etc. This will be ignored
for the rest of the discussion. A few people have been posting on blogs
about how much memory RB takes up, quoting this number - it isn't that
bad, my clock applet has ~60Mb of address space.

Of the 65Mb address space that is present when RB is not playing, most
of it will be in memory anyway - because it belongs to shared libraries,
common locale data, etc. Assuming that the user is running the Gnome DE
this can basically be ignored.

Which leaves around 8Mb of address space for the heap, which is the
interesting bit and what you all want to know about. The peak usage if
6.5Mb, which drops to 3Mb after db loading is done, and the rises to a
steady-state level of around 5Mb.


RB's memory usage is interesting, because it allocates a *lot* of very
small memory blocks - simply loading RB and then quitting immediately,
involves around 3 million calls to malloc (or equivalent). Most of these
are transient, and get quickly free'd, but this is expensive: massif
(one of the valgrind tools) estimates that around 6-8% of the heap is
used to manage the rest of the heap.


When RB is loading the db from disk, it allocates a "stat action" for
each entry and puts it into the action queue. When processing of these
stat actions occurs the results are put into a "stat event" and put on
the event queue.

Action processing is in a thread and event processing is in a
low-priority idle function; for me (on a single processor system) this
means that all the action will be processed, before any of the events
are.  A stat event is fairly big (around 1k each, all up) so right
before the events get processed 6.5Mb of the heap is in use; after they
get processed (and freed) heap use is only 3Mb. With a SMT/SMP system
they might be able to be run in parallel so this backlog doesn't occur,
but I don't have one to check on.

Without (c) and (e) from the patch this leaves a highly fragment mess in
the middle of our heap; with it the middle is mostly clear, but it can't
be shrunk due to other things being allocated further on in the heap. I
think one of the people said they had around 10k songs they had in their
RB library, which means they would have had around 10Mb of transient
data in the queue.


Another issues is the overhead our data structures have. After a short
amount of usage, I have 270kb of sequence nodes, 420kb of hash-table
nodes, 100kb of G(S)List nodes and 210kb of GtkTreeView red-black tree
nodes. All of those value don't include the data stored, that is just
the structure overhead.

At the same time I have 480kb of allocated RhythmDbEntries, and 329 Kb
of RbRefStrings (81kb normal, 61k folded, 187k sorting key).


I don't think there is much we can do about the GtkTreeView stuff, and
the list allocations are spread around everywhere, but we might be able
to do something about the sequences and hash-tables.



First off we have queries: they are the cause of almost all (>98%) of
the sequence allocations, a quarter of the hashtable allocations. There
is one sequence insertion and one hash-table insertion per db entry that
satisfies the criteria per query. Our two options are basically
redesigning queries to not use hashtables or sequences (which may be
hard/impossible to do well) or reducing the number of queries.


If we are going towards "music as a background service", and having the
window hidden most of the time, then we can probably make a some savings
by purging data we don't need when minimised.

One idea that I had earlier today was that is RB was not active
(minimised to the tray) we don't really need to keep all the queries
active, we can destroy most of them. The only one we need is the one we
are playing from, and we could even get rid of that if playback is
stopped.

I haven't though in too much detail about this, but we could add two new
methods to sources, purge queries, and build queries; which would get
called on non-playing sources when RB become inactive/active, and ask
the source to destroy/recreate the queries.


Most of the other hashtable usage would be more difficult to lower, they
are the uri->RhythmDbEntry mapping table, the RbRefString mapping table
and the genre->artist->album->track tree hashtables.



Hopefully this will give people some food-for-though, and maybe someone
can come up with some ideas.


Cheers,

James "Doc" Livingston 
-- 
We all enter the world in the same way: naked... screaming... soaked in
blood. But if you live your life right, that kind of thing doesn't have
to stop there.

Attachment: rb-mem.patch
Description: application/bzip

Attachment: signature.asc
Description: This is a digitally signed message part



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]