Merge lp:~mhr3/unity-lens-music/memory-optimizations into lp:unity-lens-music

[Michal Hruby]

Optimize memory allocation

There were reports from multiple users with huge music collections that the new music lens is using huge amount of memory. After profiling this turned out to be mostly used by the xml parser which was keeping the entire parsed file in memory and since we constructed the song model right after the parsing, this caused fragmentation of memory, so even though the xml parser freed everything after the model was built, the mem usage of the process remained very high.

Replace the xml parser from libxml-2.0 with simple GMarkupParser that is able to process the xml by chunks and therefore doesn't have these kind of problems.

[Ted Gould (ted)]

Using this branch lowered my memory usage from OVER ONE HUNDRED MEGABYTES to LESS THAN THIRTY. I am also now more attractive, smarter and I have more friends.

[Gord Allott (gordallott)]

clever ;) +1

review: Approve

[Unity Merger (unity-merger)]

No commit message specified.

=== modified file 'configure.ac'
--- configure.ac	2012-03-23 12:09:52 +0000
+++ configure.ac	2012-03-28 07:32:18 +0000
@@ -63,7 +63,6 @@
                   dee-1.0 >= 1.0.7
                   sqlite3 >= 3.7.7
                   gee-1.0
-                  libxml-2.0
 		  json-glib-1.0
                   unity >= 4.99.0)
 

=== modified file 'src/Makefile.am'
--- src/Makefile.am	2012-02-01 10:07:59 +0000
+++ src/Makefile.am	2012-03-28 07:32:18 +0000
@@ -28,7 +28,6 @@
   --pkg gio-2.0 \
   --pkg gio-unix-2.0 \
   --pkg glib-2.0 \
-  --pkg libxml-2.0 \
   $(MAINTAINER_VALAFLAGS)
 
 unity_music_daemon_LDADD = \

=== modified file 'src/rhythmbox-collection.vala'
--- src/rhythmbox-collection.vala	2012-03-22 09:13:52 +0000
+++ src/rhythmbox-collection.vala	2012-03-28 07:32:18 +0000
@@ -33,36 +33,179 @@
         ARTWORK,
         MIMETYPE,
         GENRE,
+        ALBUM_ARTIST,
         TRACK_NUMBER,
-        ALBUM_ARTIST,
         YEAR,
-        PLAY_COUNT
+        PLAY_COUNT,
+
+        N_COLUMNS
     }
 
     class RhythmboxCollection : Object
     {
 
-        GenericArray<Track> tracks = new GenericArray<Track> ();
         SequenceModel all_tracks;
         FilterModel tracks_by_play_count;
 
+        HashTable<unowned string, Variant> variant_store;
+        HashTable<int, Variant> int_variant_store;
+        Variant row_buffer[11];
+
         Analyzer analyzer;
         Index index;
         ICUTermFilter ascii_filter;
 
         string media_art_dir;
 
-        // contains genre maps
-        Genre genre = new Genre ();
+        class XmlParser: Object
+        {
+          const MarkupParser parser =
+          {
+            start_tag,
+            end_tag,
+            process_text,
+            null,
+            null
+          };
+
+          // contains genre maps
+          Genre genre = new Genre ();
+
+          MarkupParseContext context;
+          bool is_rhythmdb_xml = false;
+
+          construct
+          {
+            context = new MarkupParseContext (parser, 0, this, null);
+          }
+
+          public bool parse (string content, size_t len) throws MarkupError
+          {
+            return context.parse (content, (ssize_t) len);
+          }
+
+          bool processing_song;
+          Track current_track;
+          int current_data = -1;
+
+          private void start_tag (MarkupParseContext context, string name,
+            [CCode (array_length = false, array_null_terminated = true)] string[] attr_names, [CCode (array_length = false, array_null_terminated = true)] string[] attr_values)
+            throws MarkupError
+          {
+            if (!processing_song)
+            {
+              switch (name)
+              {
+                case "rhythmdb": is_rhythmdb_xml = true; break;
+                case "entry":
+                  bool is_song = false;
+                  for (int i = 0; attr_names[i] != null; i++)
+                  {
+                    if (attr_names[i] == "type" && attr_values[i] == "song")
+                      is_song = true;
+                  }
+                  if (!is_song) return;
+                  processing_song = true;
+                  current_track = new Track ();
+                  break;
+              }
+            }
+            else
+            {
+              switch (name)
+              {
+                case "location": current_data = Columns.URI; break;
+                case "title": current_data = Columns.TITLE; break;
+                case "artist": current_data = Columns.ARTIST; break;
+                case "album": current_data = Columns.ALBUM; break;
+                case "genre": current_data = Columns.GENRE; break;
+                case "track-number": current_data = Columns.TRACK_NUMBER; break;
+                case "play-count": current_data = Columns.PLAY_COUNT; break;
+                case "date": current_data = Columns.YEAR; break;
+                case "media-type": current_data = Columns.MIMETYPE; break;
+                case "album-artist": current_data = Columns.ALBUM_ARTIST; break;
+                default: current_data = -1; break;
+              }
+            }
+          }
+
+          public signal void track_info_ready (Track track);
+
+          private void end_tag (MarkupParseContext content, string name)
+            throws MarkupError
+          {
+            switch (name)
+            {
+              case "location":
+              case "title":
+              case "artist":
+              case "album":
+              case "genre":
+              case "track-number":
+              case "play-count":
+              case "date":
+              case "media-type":
+              case "album-artist":
+                if (current_data >= 0) current_data = -1;
+                break;
+              case "entry":
+                if (processing_song && current_track != null)
+                {
+                  track_info_ready (current_track);
+                }
+                processing_song = false;
+                break;
+            }
+          }
+
+          private void process_text (MarkupParseContext context,
+                                     string text, size_t text_len)
+            throws MarkupError
+          {
+            if (!processing_song || current_data < 0) return;
+            switch (current_data)
+            {
+              case Columns.URI: current_track.uri = text; break;
+              case Columns.TITLE: current_track.title = text; break;
+              case Columns.ARTIST: current_track.artist = text; break;
+              case Columns.ALBUM: current_track.album = text; break;
+              case Columns.ALBUM_ARTIST: 
+                current_track.album_artist = text;
+                break;
+              case Columns.GENRE:
+                current_track.genre = genre.get_id_for_genre (text.down ());
+                break;
+              case Columns.MIMETYPE:
+                current_track.mime_type = text;
+                break;
+              case Columns.YEAR:
+                current_track.year = int.parse (text) / 365;
+                break;
+              case Columns.PLAY_COUNT:
+                current_track.play_count = int.parse (text);
+                break;
+              case Columns.TRACK_NUMBER:
+                current_track.track_number = int.parse (text);
+                break;
+            }
+          }
+        }
 
         construct
         {
+          static_assert (11 == Columns.N_COLUMNS); // sync with row_buffer size
           media_art_dir = Path.build_filename (
               Environment.get_user_cache_dir (), "media-art");
 
+          variant_store = new HashTable<unowned string, Variant> (str_hash,
+                                                                  str_equal);
+          int_variant_store = new HashTable<int, Variant> (direct_hash,
+                                                           direct_equal);
           all_tracks = new SequenceModel ();
           // the columns correspond to the Columns enum
-          all_tracks.set_schema ("s", "s", "s", "s", "s", "s", "s", "s", "s", "i", "i");
+          all_tracks.set_schema ("s", "s", "s", "s", "s", "s", "s", "s", "i", "i", "i");
+          assert (all_tracks.get_schema ().length == Columns.N_COLUMNS);
+          
 
           var filter = Dee.Filter.new_sort ((row1, row2) =>
           {
@@ -137,116 +280,90 @@
             return null;
         }
 
+        private Variant cached_variant_for_string (string? input)
+        {
+          unowned string text = input != null ? input : "";
+          Variant? v = variant_store[text];
+          if (v != null) return v;
+
+          v = new Variant.string (text);
+          // key is owned by value... awesome right?
+          variant_store[v.get_string ()] = v;
+          return v;
+        }
+
+        private Variant cached_variant_for_int (int input)
+        {
+          Variant? v = int_variant_store[input];
+          if (v != null) return v;
+
+          v = new Variant.int32 (input);
+          // let's not cache every random integer
+          if (input < 128)
+            int_variant_store[input] = v;
+          return v;
+        }
+
+        private void prepare_row_buffer (Track track)
+        {
+          Variant uri = new Variant.string (track.uri);
+          Variant title = new Variant.string (track.title);
+          Variant artist = cached_variant_for_string (track.artist);
+          Variant album_artist = cached_variant_for_string (track.album_artist);
+          Variant album = cached_variant_for_string (track.album);
+          Variant mime_type = cached_variant_for_string (track.mime_type);
+          Variant artwork = cached_variant_for_string (track.artwork_path);
+          Variant genre = cached_variant_for_string (track.genre);
+          Variant track_number = cached_variant_for_int (track.track_number);
+          Variant year = cached_variant_for_int (track.year);
+          Variant play_count = cached_variant_for_int (track.play_count);
+
+          row_buffer[0] = uri;
+          row_buffer[1] = title;
+          row_buffer[2] = artist;
+          row_buffer[3] = album;
+          row_buffer[4] = artwork;
+          row_buffer[5] = mime_type;
+          row_buffer[6] = genre;
+          row_buffer[7] = album_artist;
+          row_buffer[8] = track_number;
+          row_buffer[9] = year;
+          row_buffer[10] = play_count;
+        }
+
         public void parse_file (string path)
         {
-            Xml.Parser.init ();
-
-            Xml.Doc* doc = Xml.Parser.parse_file (path);
-            if (doc == null) {
-                return;
-            }
-
-            Xml.Node* root = doc->get_root_element ();
-            if (root == null) {
-                delete doc;
-                return;
-            }
-            parse_node (root);
-            delete doc;
-
-            Xml.Parser.cleanup ();
-        }
-
-        private void parse_node (Xml.Node* node)
-        {
-            for (Xml.Node* iter = node->children; iter != null; iter = iter->next) {
-                if (iter->type != Xml.ElementType.ELEMENT_NODE) {
-                    continue;
-                }
-                for (Xml.Attr* prop = iter->properties; prop != null; prop = prop->next) {
-                    string attr_content = prop->children->content;
-                    if (attr_content == "song") {
-                        Track track = new Track ();
-              
-                        for (Xml.Node* iter_track = iter->children; iter_track != null; 
-                            iter_track = iter_track->next) {
-                            if (iter_track->type != Xml.ElementType.ELEMENT_NODE) {
-                                continue;
-                            }
-                            switch (iter_track->name) {
-                            case "title":
-                                string node_content = iter_track->get_content ();
-                                track.title = node_content;
-                                break;
-                            case "location":
-                                string node_content = iter_track->get_content ();
-                                track.uri = node_content;
-                                break;
-                            case "artist":
-                                string node_content = iter_track->get_content ();
-                                track.artist = node_content;
-                                break;
-                            case "media-type":
-                                string node_content = iter_track->get_content ();
-                                track.mime_type = node_content;
-                                break;
-                            case "album":
-                                string node_content = iter_track->get_content ();
-                                track.album = node_content;
-                                break;
-                            case "genre":
-                                string node_content = iter_track->get_content ();
-                                track.genre = node_content;
-                                break;
-                            case "track-number":
-                                string node_content = iter_track->get_content ();
-                                track.track_number = node_content;
-                                break;
-                            case "album-artist":
-                                string node_content = iter_track->get_content ();
-                                track.album_artist = node_content;
-                                break;
-                            case "date":
-                                string node_content = iter_track->get_content ();
-                                track.year = int.parse (node_content);
-                                break;
-                            case "play-count":
-                                string node_content = iter_track->get_content ();
-                                track.play_count = int.parse (node_content);
-                                break;
-                            }
-                        }
-                        // append to tracks array
-                        tracks.add (track);
-
-                        // Get cover art
-                        string albumart = get_albumart (track);
-                        if (albumart != null)
-                            track.artwork_path = albumart;
-                        else
-                            track.artwork_path = "audio-x-generic";
-                        
-                        // Get genre filter id
-                        track.genre = genre.get_id_for_genre(track.genre.down ()); 
-                        
-                        if (track.year > 0) {
-                            track.year /= 365;
-                        }
-
-                        all_tracks.append (track.uri, track.title,
-                                           track.artist, track.album,
-                                           track.artwork_path,
-                                           track.mime_type,
-                                           track.genre,
-                                           track.track_number,
-                                           track.album_artist,
-                                           track.year,
-                                           track.play_count);
-                    }
-                    // Next track
-                    parse_node (iter);
-                }
-            }
+          var parser = new XmlParser ();
+          parser.track_info_ready.connect ((track) =>
+          {
+            // Get cover art
+            string albumart = get_albumart (track);
+            if (albumart != null)
+              track.artwork_path = albumart;
+            else
+              track.artwork_path = "audio-x-generic";
+
+            prepare_row_buffer (track);
+            all_tracks.append_row (row_buffer);
+          });
+
+          var file = File.new_for_path (path);
+
+          try
+          {
+            var stream = file.read (null);
+            uint8 buffer[65536];
+
+            size_t bytes_read;
+            while ((bytes_read = stream.read (buffer, null)) > 0)
+            {
+              parser.parse ((string) buffer, bytes_read);
+            }
+          }
+          catch (Error err)
+          {
+            warning ("Error while parsing rhythmbox DB: %s", err.message);
+          }
         }
 
         public void search (LensSearch search, 

=== modified file 'src/track.vala'
--- src/track.vala	2012-03-19 18:52:51 +0000
+++ src/track.vala	2012-03-28 07:32:18 +0000
@@ -29,7 +29,7 @@
     public string album { get; set; }
     public string album_artist { get; set; }
     public string genre { get; set; }
-    public string track_number { get; set; }
+    public int track_number { get; set; }
     public int year { get; set; }
     public int play_count { get; set; }
   }