Bug 150508 - posix_fadvise to speed up batch reading of tags in amarok
Summary: posix_fadvise to speed up batch reading of tags in amarok
Status: RESOLVED INTENTIONAL
Alias: None
Product: taglib
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Ubuntu Linux
: NOR wishlist
Target Milestone: ---
Assignee: Scott Wheeler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-10-05 17:41 UTC by Manuel Amador (Rudd-O)
Modified: 2008-01-28 10:55 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Manuel Amador (Rudd-O) 2007-10-05 17:41:14 UTC
Version:            (using KDE KDE 3.5.7)
Installed from:    Ubuntu Packages
OS:                Linux

I have a patch that implements posix_fadvise POSIX_FADV_RANDOM so as to not have readahead on files that are going to be read/tagged.  The patch is at:

http://rudd-o.com/archives/2007/10/05/speeding-amarok-up/

This should provide a significant performance gain on large collections and prevent lots of readahead IO currently being triggered by the default readahead heuristics in 99% of distros out there.

It's ifdeffd so if there is no Linux/POSIX available, there is no effect.
Comment 1 Manuel Amador (Rudd-O) 2007-10-08 15:02:55 UTC
The ifdefd stuff is borken, sorry.  Need an expert in autocrap (or cmake?) to make it work.

You should also know that I expect this to help with initial Amarok startup when the user has a very long playlist.  I understand that Amarok stats() all the files in the playlist and reads their tags from disk when it starts up, so this should provide a brutal speedup.  And let me tell you, even with the monster machine I have (70 MB/s linear reads per-disk, 1.4 GB from cache reads, in a RAID-1 configuration), starting up with 200 tracks on my playlist makes Amarok freeze for several seconds while the machine weathers a severe disk storm.
Comment 2 Lukáš Lalinský 2007-12-28 13:46:25 UTC
I personally think this one is WONTFIX. I don't see any benchmarks showing that it actually helps anything, and it adds unnecessary dependency on a linux-specific function. Scott?
Comment 3 Manuel Amador (Rudd-O) 2007-12-29 01:54:53 UTC
I specifically asked for someone to test the patch.  I don't have a development machien where I could install taglib and not break my running system.  Collaboration means we all get to do a little piece of the puzzle.

Moreover, posix_fadvise is part of the POSIX standard, it's not linux-specific.  And if your target platform doesn't have it, you can always #ifdef it or use autoconf/cmake magic to detect its presence.

Why you would want to WONTFIX a patch that can dramatically improve system responsiveness while scanning for media files in media players is truly befuddling to me and BEYOND my imagination.
Comment 4 Lukáš Lalinský 2008-01-14 09:51:56 UTC
Because not all players have the same workflow as Amarok and not all applications that use TagLib are players. If you read the tags and then start playing the file right away, this patch would slow things down. But as I said earlier, I don't see evidence of improving system responsiveness either.
Comment 5 Manuel Amador (Rudd-O) 2008-01-14 19:28:11 UTC
No, your conjecture about this patch slowing down playback of MP3 files does not follow, because if your player thread has the file open (which would be the case 100% of the time), the file won't be evicted from cache until it's closed.  Read the posix_fadvise page.

Testing system responsiveness is hard because it's usually non-deterministic.  But you should see a difference in disk cache size before and after scanning your collection using a TagLib-enabled application.  Now, if system responsiveness is no different on your system, I can understand that, but don't deny the rest of the world a chance to experience better responsiveness.  It's not like I'm introducing a bug with this patch here!

And, of course, the increase in system responsiveness should be evident only during collection scans, because the effect is overwhelmingly insignificant if all you do is play one file.  Do you have a 20.000 track collection to test on?
Comment 6 Scott Wheeler 2008-01-26 21:07:41 UTC
I just finally got around to testing this.  Not only does it not make things faster, it makes no difference on a cold cache and makes the reads take several times longer with a warm cache.
Comment 7 Manuel Amador (Rudd-O) 2008-01-28 10:55:49 UTC
Ooookay.  The patch wasn't to make collection scans faster anyway, it was to AVOID pagecache thrashing during collection scans.  Care to release your testing data?  I'd like to take a look at it.