On Sun, 2005-07-17 at 02:06 +1000, James Livingston wrote: > 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. Cool, thanks for this work. > 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. This is generally a good idea as long as the allocation is actually in a kind of memory hot-spot; e.g. if it's happening for each song or something. But I don't think e.g. making these kinds of changes for option processing or something is going to be a win if it makes the code messier. I haven't looked at what you're changing, just making the general point. > 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). There was some interesting discussion re memchunks lately among the GLib developers; some felt that it was actually worse than a good malloc implementation. It's something we need to investigate: http://bugzilla.gnome.org/show_bug.cgi?id=118439#c19 Your point about fragmentation is good though, not sure it was really addressed in that discussion. > d) actually call g_blow_chunks() so that empty memchunks are released. > You've gotta love that function name :) When do you call this? > 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. I don't understand this one; why shouldn't we be doing the stats in parallel with loading? Why would the rhythmdb.xml be on a slow media? It can't really be on a media slower than the rest of the desktop. > However, > this stops *massive* heap fragmentation, which is caused when > allocations of stat actions (transient) are alternated with db entries > (permanent). Do you know what kind of performance impact fragmentation has? I understand the concept but I'm not very familiar with what kind of impact it typically has. > 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. This is the kind of change I don't think we should be making...we should identify the hotspots and fix those, not change all of the code. > 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. This sounds cool. > 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, Yeah, this sounds fixable. > 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. What if we limited the size of the action queue? Should be easy to do. Actually I'm working on this code now, I might do that while I'm at it. The potential cost here is that it could block the UI if the action thread isn't scheduled, which to the user looks pretty bad. > 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. That doesn't sound bad really, we're only talking about like 2MB of space? > 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. Yeah...the queries are expensive, no doubt. GHashTable is pretty fast but we make very extensive use of it and have data indexed in N different ways. I don't have any great ideas at the moment on reducing the impact here. > 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. Yeah, definitely. > 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. Makes sense with the caveat that to the user it could be kind of annoying if they switch back to their music and see their song list visibly refilling. What would be really nice is if the system somehow told us that it was low on memory (D-BUS signal from HAL?) and we could drop cached data. > Hopefully this will give people some food-for-though, and maybe someone > can come up with some ideas. Thanks a lot for this work and your thoughts!
Attachment:
signature.asc
Description: This is a digitally signed message part