5
\$\begingroup\$

I'm trying to make an audio file tag editor, but I ran into some serious performance issues. Here's my method for loading files:

private void LoadFiles(params string[] fileNames) {
    foreach (string fileName in fileNames) {
        string path = fileName;
        if (loadedSongs.ContainsKey(path))
            continue;

        new Thread(new ThreadStart(() => {
            using (TagFile file = TagFile.Create(path)) {
                Song song = new Song() {
                    Album = file.Tag.Album,
                    AlbumArtists = file.Tag.AlbumArtists,
                    Artists = file.Tag.Performers,
                    BeatsPerMinute = (file.Tag.BeatsPerMinute != 0 ? 
                        (uint?)file.Tag.BeatsPerMinute : null),

                    // ...snip...
                };
                lock (this.loadedSongs) {
                    this.loadedSongs.Add(path, song);
                }
                this.Invoke((MethodInvoker)delegate {

                    int rowId = songDataGrid.Rows.Add();
                    DataGridViewRow row = songDataGrid.Rows[rowId];

                    UpdateRow(row, song);
                });
            }
        })).Start();
    }
}

So I have a Song class defined which is really just a container for the various tags that TagLib.File provides, so I don't need to keep a handle on the file.

I have a few problems with this function though:

  • It is incredibly slow. Even with threading, it takes about 12 seconds before the DataGridView even updates and adds the first rows if I load 2GB of files (on my machine).
    That may not seem long, but imagine if a user adds 10 or 20GB of music - they could be waiting several minutes before they can use the application.

  • The songDataGrid.Rows.Add() call doesn't seem to execute immediately - it seems to do it in "batches." I don't know if the problem is the TagLib Sharp library or with the DataGridView control, or if I'm just imagining things. It's probably the fact that the TagLib.File.Create() function finishes on all the Threads at the same time, then the rest of the code is fast.

  • This takes up a LOT of memory - once this runs, my app has over 1GB of memory in use until the garbage collector is run (at which point it dwindles down to ~80k, which is alright). I believe this is because TagLib loads the entire file into memory. I am disposing the file properly, as far as I know.

I don't know if my issues are because of limitations of TabLib or it's something in my code.

Note that I don't really care that much if it takes several seconds/minutes to load several gigabytes worth of files. What I really want is to curb memory usage as well as immediate addition into the DataGridView so that the user can at least edit the first few rows of the grid while the rest of the application is loading.

What can I change?


Update I know that in my personal research of the subject, I noticed a lot of people complaining that TagLib is kind of slow. It's mostly likely because of your code (as it was in my situation). I've alleviated some of the problems, so in case someone else stumbles on this post with a similar issue, I'll give a quick update on my predicament.

Thanks to Dan Lyons, I've refactored my code to use a different threading and update model, which makes the method much snappier.

It was slow for two reasons: because I was locking access to a Dictonary plus invoking a UI update, and the fact that I was parsing the album art images in the method itself.

  • To fix the locking/UI issue, I used a BackgroundWorker and did the updates on the RunWorkerCompleted event, which is called on the UI thread, so no invocation is necessary. Doing this was orders of magnitudes more efficient than blindly running my own Thread.

  • If you're using TagLib for dealing with album art, it can spike your memory through the roof if handled poorly. If you're showing the art to the user at all, I would advise against storing the full ByteVector worth of data in memory, and instead convert it to a resized Bitmap and store that in memory. If the user wishes to modify or export the artwork, reload the file. I was able to get my app down to ~60k memory maximum with 2GB of songs + album art.

\$\endgroup\$
0
4
\$\begingroup\$

There are a couple suggestions:

  • First, you do not need to use your path variable - it is simply fileName re-packaged. This is probably of negligible impact, but there's no reason to keep the extra string around, either.
  • Next, a filesystem is not going to give you much in the way of performance gains by multi-threading access. In fact, if you're doing this on a spindle drive, you are probably making things worse. Instead of generating threads for every file, toss your entire loop into a separate async method and have the loop run on a single thread. You still get the UI responsiveness of it being a background task this way.
  • You may want to avoid hand-building threads. It is generally preferable to use one of the other mechanisms in the language instead, such as ThreadPool, Task<T>, or even BeginAsync or BackgroundWorker (since you seem to be in a UI).
  • Consider having your async loop build its own collection and doing a batch add to loadedSongs later. This removes the need to synchronize access to loadedSongs until the very end and the corresponding locking overhead.
  • Finally, I would suspend layout on your grid until the update is complete. DataGridView is pretty poor at updating itself quickly. You almost always win by suspending, making all your updates in a batch, and then resuming layout.

However, my suspicion is that the biggest gain you will get is implementing your own tag reader library rather than using TagLib, assuming you are correct that it reads the entire file. I/O is one of the most expensive things you can do on a computer. ID3v1 and ID3V2 tags should generally appear in the first X bytes of the file, so you only have to read until the end of the tag, rather than the entire file.

As with any performance issue, though, you need to run this through a profiler. The results may very well point you in a completely different direction. For example, it may reveal that TagLib isn't actually reading the entire file. At the very least, it provides baselines to use in determining if you are making meaningful gains.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the suggestions. This is actually very helpful, particularly the idea of processing first, updating last. Only one caveat - the reason I am duplicating the path variable is because of the outer variable trap that occurs because of the fact that I am using a lambda expression. \$\endgroup\$
    – Corey
    Mar 9 '12 at 19:02
  • \$\begingroup\$ Wow, just from doing two of those things (point 3 & 4, using BackgroundWorker and removing the need for syncronization), this method is magnitudes faster, and it took very little code to do so. \$\endgroup\$
    – Corey
    Mar 9 '12 at 20:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.