Merge lp:~mhr3/unity-lens-music/memory-optimizations into lp:unity-lens-music
- memory-optimizations
- Merge into trunk
Proposed by
Michal Hruby
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Michal Hruby | ||||
Approved revision: | 78 | ||||
Merged at revision: | 77 | ||||
Proposed branch: | lp:~mhr3/unity-lens-music/memory-optimizations | ||||
Merge into: | lp:unity-lens-music | ||||
Diff against target: |
425 lines (+232/-117) 4 files modified
configure.ac (+0/-1) src/Makefile.am (+0/-1) src/rhythmbox-collection.vala (+231/-114) src/track.vala (+1/-1) |
||||
To merge this branch: | bzr merge lp:~mhr3/unity-lens-music/memory-optimizations | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gord Allott (community) | Approve | ||
Review via email: mp+99682@code.launchpad.net |
Commit message
Optimize memory allocation
Description of the change
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.
To post a comment you must log in.
Revision history for this message
Ted Gould (ted) wrote : | # |
Revision history for this message
Unity Merger (unity-merger) wrote : | # |
No commit message specified.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'configure.ac' | |||
2 | --- configure.ac 2012-03-23 12:09:52 +0000 | |||
3 | +++ configure.ac 2012-03-28 07:32:18 +0000 | |||
4 | @@ -63,7 +63,6 @@ | |||
5 | 63 | dee-1.0 >= 1.0.7 | 63 | dee-1.0 >= 1.0.7 |
6 | 64 | sqlite3 >= 3.7.7 | 64 | sqlite3 >= 3.7.7 |
7 | 65 | gee-1.0 | 65 | gee-1.0 |
8 | 66 | libxml-2.0 | ||
9 | 67 | json-glib-1.0 | 66 | json-glib-1.0 |
10 | 68 | unity >= 4.99.0) | 67 | unity >= 4.99.0) |
11 | 69 | 68 | ||
12 | 70 | 69 | ||
13 | === modified file 'src/Makefile.am' | |||
14 | --- src/Makefile.am 2012-02-01 10:07:59 +0000 | |||
15 | +++ src/Makefile.am 2012-03-28 07:32:18 +0000 | |||
16 | @@ -28,7 +28,6 @@ | |||
17 | 28 | --pkg gio-2.0 \ | 28 | --pkg gio-2.0 \ |
18 | 29 | --pkg gio-unix-2.0 \ | 29 | --pkg gio-unix-2.0 \ |
19 | 30 | --pkg glib-2.0 \ | 30 | --pkg glib-2.0 \ |
20 | 31 | --pkg libxml-2.0 \ | ||
21 | 32 | $(MAINTAINER_VALAFLAGS) | 31 | $(MAINTAINER_VALAFLAGS) |
22 | 33 | 32 | ||
23 | 34 | unity_music_daemon_LDADD = \ | 33 | unity_music_daemon_LDADD = \ |
24 | 35 | 34 | ||
25 | === modified file 'src/rhythmbox-collection.vala' | |||
26 | --- src/rhythmbox-collection.vala 2012-03-22 09:13:52 +0000 | |||
27 | +++ src/rhythmbox-collection.vala 2012-03-28 07:32:18 +0000 | |||
28 | @@ -33,36 +33,179 @@ | |||
29 | 33 | ARTWORK, | 33 | ARTWORK, |
30 | 34 | MIMETYPE, | 34 | MIMETYPE, |
31 | 35 | GENRE, | 35 | GENRE, |
32 | 36 | ALBUM_ARTIST, | ||
33 | 36 | TRACK_NUMBER, | 37 | TRACK_NUMBER, |
34 | 37 | ALBUM_ARTIST, | ||
35 | 38 | YEAR, | 38 | YEAR, |
37 | 39 | PLAY_COUNT | 39 | PLAY_COUNT, |
38 | 40 | |||
39 | 41 | N_COLUMNS | ||
40 | 40 | } | 42 | } |
41 | 41 | 43 | ||
42 | 42 | class RhythmboxCollection : Object | 44 | class RhythmboxCollection : Object |
43 | 43 | { | 45 | { |
44 | 44 | 46 | ||
45 | 45 | GenericArray<Track> tracks = new GenericArray<Track> (); | ||
46 | 46 | SequenceModel all_tracks; | 47 | SequenceModel all_tracks; |
47 | 47 | FilterModel tracks_by_play_count; | 48 | FilterModel tracks_by_play_count; |
48 | 48 | 49 | ||
49 | 50 | HashTable<unowned string, Variant> variant_store; | ||
50 | 51 | HashTable<int, Variant> int_variant_store; | ||
51 | 52 | Variant row_buffer[11]; | ||
52 | 53 | |||
53 | 49 | Analyzer analyzer; | 54 | Analyzer analyzer; |
54 | 50 | Index index; | 55 | Index index; |
55 | 51 | ICUTermFilter ascii_filter; | 56 | ICUTermFilter ascii_filter; |
56 | 52 | 57 | ||
57 | 53 | string media_art_dir; | 58 | string media_art_dir; |
58 | 54 | 59 | ||
61 | 55 | // contains genre maps | 60 | class XmlParser: Object |
62 | 56 | Genre genre = new Genre (); | 61 | { |
63 | 62 | const MarkupParser parser = | ||
64 | 63 | { | ||
65 | 64 | start_tag, | ||
66 | 65 | end_tag, | ||
67 | 66 | process_text, | ||
68 | 67 | null, | ||
69 | 68 | null | ||
70 | 69 | }; | ||
71 | 70 | |||
72 | 71 | // contains genre maps | ||
73 | 72 | Genre genre = new Genre (); | ||
74 | 73 | |||
75 | 74 | MarkupParseContext context; | ||
76 | 75 | bool is_rhythmdb_xml = false; | ||
77 | 76 | |||
78 | 77 | construct | ||
79 | 78 | { | ||
80 | 79 | context = new MarkupParseContext (parser, 0, this, null); | ||
81 | 80 | } | ||
82 | 81 | |||
83 | 82 | public bool parse (string content, size_t len) throws MarkupError | ||
84 | 83 | { | ||
85 | 84 | return context.parse (content, (ssize_t) len); | ||
86 | 85 | } | ||
87 | 86 | |||
88 | 87 | bool processing_song; | ||
89 | 88 | Track current_track; | ||
90 | 89 | int current_data = -1; | ||
91 | 90 | |||
92 | 91 | private void start_tag (MarkupParseContext context, string name, | ||
93 | 92 | [CCode (array_length = false, array_null_terminated = true)] string[] attr_names, [CCode (array_length = false, array_null_terminated = true)] string[] attr_values) | ||
94 | 93 | throws MarkupError | ||
95 | 94 | { | ||
96 | 95 | if (!processing_song) | ||
97 | 96 | { | ||
98 | 97 | switch (name) | ||
99 | 98 | { | ||
100 | 99 | case "rhythmdb": is_rhythmdb_xml = true; break; | ||
101 | 100 | case "entry": | ||
102 | 101 | bool is_song = false; | ||
103 | 102 | for (int i = 0; attr_names[i] != null; i++) | ||
104 | 103 | { | ||
105 | 104 | if (attr_names[i] == "type" && attr_values[i] == "song") | ||
106 | 105 | is_song = true; | ||
107 | 106 | } | ||
108 | 107 | if (!is_song) return; | ||
109 | 108 | processing_song = true; | ||
110 | 109 | current_track = new Track (); | ||
111 | 110 | break; | ||
112 | 111 | } | ||
113 | 112 | } | ||
114 | 113 | else | ||
115 | 114 | { | ||
116 | 115 | switch (name) | ||
117 | 116 | { | ||
118 | 117 | case "location": current_data = Columns.URI; break; | ||
119 | 118 | case "title": current_data = Columns.TITLE; break; | ||
120 | 119 | case "artist": current_data = Columns.ARTIST; break; | ||
121 | 120 | case "album": current_data = Columns.ALBUM; break; | ||
122 | 121 | case "genre": current_data = Columns.GENRE; break; | ||
123 | 122 | case "track-number": current_data = Columns.TRACK_NUMBER; break; | ||
124 | 123 | case "play-count": current_data = Columns.PLAY_COUNT; break; | ||
125 | 124 | case "date": current_data = Columns.YEAR; break; | ||
126 | 125 | case "media-type": current_data = Columns.MIMETYPE; break; | ||
127 | 126 | case "album-artist": current_data = Columns.ALBUM_ARTIST; break; | ||
128 | 127 | default: current_data = -1; break; | ||
129 | 128 | } | ||
130 | 129 | } | ||
131 | 130 | } | ||
132 | 131 | |||
133 | 132 | public signal void track_info_ready (Track track); | ||
134 | 133 | |||
135 | 134 | private void end_tag (MarkupParseContext content, string name) | ||
136 | 135 | throws MarkupError | ||
137 | 136 | { | ||
138 | 137 | switch (name) | ||
139 | 138 | { | ||
140 | 139 | case "location": | ||
141 | 140 | case "title": | ||
142 | 141 | case "artist": | ||
143 | 142 | case "album": | ||
144 | 143 | case "genre": | ||
145 | 144 | case "track-number": | ||
146 | 145 | case "play-count": | ||
147 | 146 | case "date": | ||
148 | 147 | case "media-type": | ||
149 | 148 | case "album-artist": | ||
150 | 149 | if (current_data >= 0) current_data = -1; | ||
151 | 150 | break; | ||
152 | 151 | case "entry": | ||
153 | 152 | if (processing_song && current_track != null) | ||
154 | 153 | { | ||
155 | 154 | track_info_ready (current_track); | ||
156 | 155 | } | ||
157 | 156 | processing_song = false; | ||
158 | 157 | break; | ||
159 | 158 | } | ||
160 | 159 | } | ||
161 | 160 | |||
162 | 161 | private void process_text (MarkupParseContext context, | ||
163 | 162 | string text, size_t text_len) | ||
164 | 163 | throws MarkupError | ||
165 | 164 | { | ||
166 | 165 | if (!processing_song || current_data < 0) return; | ||
167 | 166 | switch (current_data) | ||
168 | 167 | { | ||
169 | 168 | case Columns.URI: current_track.uri = text; break; | ||
170 | 169 | case Columns.TITLE: current_track.title = text; break; | ||
171 | 170 | case Columns.ARTIST: current_track.artist = text; break; | ||
172 | 171 | case Columns.ALBUM: current_track.album = text; break; | ||
173 | 172 | case Columns.ALBUM_ARTIST: | ||
174 | 173 | current_track.album_artist = text; | ||
175 | 174 | break; | ||
176 | 175 | case Columns.GENRE: | ||
177 | 176 | current_track.genre = genre.get_id_for_genre (text.down ()); | ||
178 | 177 | break; | ||
179 | 178 | case Columns.MIMETYPE: | ||
180 | 179 | current_track.mime_type = text; | ||
181 | 180 | break; | ||
182 | 181 | case Columns.YEAR: | ||
183 | 182 | current_track.year = int.parse (text) / 365; | ||
184 | 183 | break; | ||
185 | 184 | case Columns.PLAY_COUNT: | ||
186 | 185 | current_track.play_count = int.parse (text); | ||
187 | 186 | break; | ||
188 | 187 | case Columns.TRACK_NUMBER: | ||
189 | 188 | current_track.track_number = int.parse (text); | ||
190 | 189 | break; | ||
191 | 190 | } | ||
192 | 191 | } | ||
193 | 192 | } | ||
194 | 57 | 193 | ||
195 | 58 | construct | 194 | construct |
196 | 59 | { | 195 | { |
197 | 196 | static_assert (11 == Columns.N_COLUMNS); // sync with row_buffer size | ||
198 | 60 | media_art_dir = Path.build_filename ( | 197 | media_art_dir = Path.build_filename ( |
199 | 61 | Environment.get_user_cache_dir (), "media-art"); | 198 | Environment.get_user_cache_dir (), "media-art"); |
200 | 62 | 199 | ||
201 | 200 | variant_store = new HashTable<unowned string, Variant> (str_hash, | ||
202 | 201 | str_equal); | ||
203 | 202 | int_variant_store = new HashTable<int, Variant> (direct_hash, | ||
204 | 203 | direct_equal); | ||
205 | 63 | all_tracks = new SequenceModel (); | 204 | all_tracks = new SequenceModel (); |
206 | 64 | // the columns correspond to the Columns enum | 205 | // the columns correspond to the Columns enum |
208 | 65 | all_tracks.set_schema ("s", "s", "s", "s", "s", "s", "s", "s", "s", "i", "i"); | 206 | all_tracks.set_schema ("s", "s", "s", "s", "s", "s", "s", "s", "i", "i", "i"); |
209 | 207 | assert (all_tracks.get_schema ().length == Columns.N_COLUMNS); | ||
210 | 208 | |||
211 | 66 | 209 | ||
212 | 67 | var filter = Dee.Filter.new_sort ((row1, row2) => | 210 | var filter = Dee.Filter.new_sort ((row1, row2) => |
213 | 68 | { | 211 | { |
214 | @@ -137,116 +280,90 @@ | |||
215 | 137 | return null; | 280 | return null; |
216 | 138 | } | 281 | } |
217 | 139 | 282 | ||
218 | 283 | private Variant cached_variant_for_string (string? input) | ||
219 | 284 | { | ||
220 | 285 | unowned string text = input != null ? input : ""; | ||
221 | 286 | Variant? v = variant_store[text]; | ||
222 | 287 | if (v != null) return v; | ||
223 | 288 | |||
224 | 289 | v = new Variant.string (text); | ||
225 | 290 | // key is owned by value... awesome right? | ||
226 | 291 | variant_store[v.get_string ()] = v; | ||
227 | 292 | return v; | ||
228 | 293 | } | ||
229 | 294 | |||
230 | 295 | private Variant cached_variant_for_int (int input) | ||
231 | 296 | { | ||
232 | 297 | Variant? v = int_variant_store[input]; | ||
233 | 298 | if (v != null) return v; | ||
234 | 299 | |||
235 | 300 | v = new Variant.int32 (input); | ||
236 | 301 | // let's not cache every random integer | ||
237 | 302 | if (input < 128) | ||
238 | 303 | int_variant_store[input] = v; | ||
239 | 304 | return v; | ||
240 | 305 | } | ||
241 | 306 | |||
242 | 307 | private void prepare_row_buffer (Track track) | ||
243 | 308 | { | ||
244 | 309 | Variant uri = new Variant.string (track.uri); | ||
245 | 310 | Variant title = new Variant.string (track.title); | ||
246 | 311 | Variant artist = cached_variant_for_string (track.artist); | ||
247 | 312 | Variant album_artist = cached_variant_for_string (track.album_artist); | ||
248 | 313 | Variant album = cached_variant_for_string (track.album); | ||
249 | 314 | Variant mime_type = cached_variant_for_string (track.mime_type); | ||
250 | 315 | Variant artwork = cached_variant_for_string (track.artwork_path); | ||
251 | 316 | Variant genre = cached_variant_for_string (track.genre); | ||
252 | 317 | Variant track_number = cached_variant_for_int (track.track_number); | ||
253 | 318 | Variant year = cached_variant_for_int (track.year); | ||
254 | 319 | Variant play_count = cached_variant_for_int (track.play_count); | ||
255 | 320 | |||
256 | 321 | row_buffer[0] = uri; | ||
257 | 322 | row_buffer[1] = title; | ||
258 | 323 | row_buffer[2] = artist; | ||
259 | 324 | row_buffer[3] = album; | ||
260 | 325 | row_buffer[4] = artwork; | ||
261 | 326 | row_buffer[5] = mime_type; | ||
262 | 327 | row_buffer[6] = genre; | ||
263 | 328 | row_buffer[7] = album_artist; | ||
264 | 329 | row_buffer[8] = track_number; | ||
265 | 330 | row_buffer[9] = year; | ||
266 | 331 | row_buffer[10] = play_count; | ||
267 | 332 | } | ||
268 | 333 | |||
269 | 140 | public void parse_file (string path) | 334 | public void parse_file (string path) |
270 | 141 | { | 335 | { |
379 | 142 | Xml.Parser.init (); | 336 | var parser = new XmlParser (); |
380 | 143 | 337 | parser.track_info_ready.connect ((track) => | |
381 | 144 | Xml.Doc* doc = Xml.Parser.parse_file (path); | 338 | { |
382 | 145 | if (doc == null) { | 339 | // Get cover art |
383 | 146 | return; | 340 | string albumart = get_albumart (track); |
384 | 147 | } | 341 | if (albumart != null) |
385 | 148 | 342 | track.artwork_path = albumart; | |
386 | 149 | Xml.Node* root = doc->get_root_element (); | 343 | else |
387 | 150 | if (root == null) { | 344 | track.artwork_path = "audio-x-generic"; |
388 | 151 | delete doc; | 345 | |
389 | 152 | return; | 346 | prepare_row_buffer (track); |
390 | 153 | } | 347 | all_tracks.append_row (row_buffer); |
391 | 154 | parse_node (root); | 348 | }); |
392 | 155 | delete doc; | 349 | |
393 | 156 | 350 | var file = File.new_for_path (path); | |
394 | 157 | Xml.Parser.cleanup (); | 351 | |
395 | 158 | } | 352 | try |
396 | 159 | 353 | { | |
397 | 160 | private void parse_node (Xml.Node* node) | 354 | var stream = file.read (null); |
398 | 161 | { | 355 | uint8 buffer[65536]; |
399 | 162 | for (Xml.Node* iter = node->children; iter != null; iter = iter->next) { | 356 | |
400 | 163 | if (iter->type != Xml.ElementType.ELEMENT_NODE) { | 357 | size_t bytes_read; |
401 | 164 | continue; | 358 | while ((bytes_read = stream.read (buffer, null)) > 0) |
402 | 165 | } | 359 | { |
403 | 166 | for (Xml.Attr* prop = iter->properties; prop != null; prop = prop->next) { | 360 | parser.parse ((string) buffer, bytes_read); |
404 | 167 | string attr_content = prop->children->content; | 361 | } |
405 | 168 | if (attr_content == "song") { | 362 | } |
406 | 169 | Track track = new Track (); | 363 | catch (Error err) |
407 | 170 | 364 | { | |
408 | 171 | for (Xml.Node* iter_track = iter->children; iter_track != null; | 365 | warning ("Error while parsing rhythmbox DB: %s", err.message); |
409 | 172 | iter_track = iter_track->next) { | 366 | } |
302 | 173 | if (iter_track->type != Xml.ElementType.ELEMENT_NODE) { | ||
303 | 174 | continue; | ||
304 | 175 | } | ||
305 | 176 | switch (iter_track->name) { | ||
306 | 177 | case "title": | ||
307 | 178 | string node_content = iter_track->get_content (); | ||
308 | 179 | track.title = node_content; | ||
309 | 180 | break; | ||
310 | 181 | case "location": | ||
311 | 182 | string node_content = iter_track->get_content (); | ||
312 | 183 | track.uri = node_content; | ||
313 | 184 | break; | ||
314 | 185 | case "artist": | ||
315 | 186 | string node_content = iter_track->get_content (); | ||
316 | 187 | track.artist = node_content; | ||
317 | 188 | break; | ||
318 | 189 | case "media-type": | ||
319 | 190 | string node_content = iter_track->get_content (); | ||
320 | 191 | track.mime_type = node_content; | ||
321 | 192 | break; | ||
322 | 193 | case "album": | ||
323 | 194 | string node_content = iter_track->get_content (); | ||
324 | 195 | track.album = node_content; | ||
325 | 196 | break; | ||
326 | 197 | case "genre": | ||
327 | 198 | string node_content = iter_track->get_content (); | ||
328 | 199 | track.genre = node_content; | ||
329 | 200 | break; | ||
330 | 201 | case "track-number": | ||
331 | 202 | string node_content = iter_track->get_content (); | ||
332 | 203 | track.track_number = node_content; | ||
333 | 204 | break; | ||
334 | 205 | case "album-artist": | ||
335 | 206 | string node_content = iter_track->get_content (); | ||
336 | 207 | track.album_artist = node_content; | ||
337 | 208 | break; | ||
338 | 209 | case "date": | ||
339 | 210 | string node_content = iter_track->get_content (); | ||
340 | 211 | track.year = int.parse (node_content); | ||
341 | 212 | break; | ||
342 | 213 | case "play-count": | ||
343 | 214 | string node_content = iter_track->get_content (); | ||
344 | 215 | track.play_count = int.parse (node_content); | ||
345 | 216 | break; | ||
346 | 217 | } | ||
347 | 218 | } | ||
348 | 219 | // append to tracks array | ||
349 | 220 | tracks.add (track); | ||
350 | 221 | |||
351 | 222 | // Get cover art | ||
352 | 223 | string albumart = get_albumart (track); | ||
353 | 224 | if (albumart != null) | ||
354 | 225 | track.artwork_path = albumart; | ||
355 | 226 | else | ||
356 | 227 | track.artwork_path = "audio-x-generic"; | ||
357 | 228 | |||
358 | 229 | // Get genre filter id | ||
359 | 230 | track.genre = genre.get_id_for_genre(track.genre.down ()); | ||
360 | 231 | |||
361 | 232 | if (track.year > 0) { | ||
362 | 233 | track.year /= 365; | ||
363 | 234 | } | ||
364 | 235 | |||
365 | 236 | all_tracks.append (track.uri, track.title, | ||
366 | 237 | track.artist, track.album, | ||
367 | 238 | track.artwork_path, | ||
368 | 239 | track.mime_type, | ||
369 | 240 | track.genre, | ||
370 | 241 | track.track_number, | ||
371 | 242 | track.album_artist, | ||
372 | 243 | track.year, | ||
373 | 244 | track.play_count); | ||
374 | 245 | } | ||
375 | 246 | // Next track | ||
376 | 247 | parse_node (iter); | ||
377 | 248 | } | ||
378 | 249 | } | ||
410 | 250 | } | 367 | } |
411 | 251 | 368 | ||
412 | 252 | public void search (LensSearch search, | 369 | public void search (LensSearch search, |
413 | 253 | 370 | ||
414 | === modified file 'src/track.vala' | |||
415 | --- src/track.vala 2012-03-19 18:52:51 +0000 | |||
416 | +++ src/track.vala 2012-03-28 07:32:18 +0000 | |||
417 | @@ -29,7 +29,7 @@ | |||
418 | 29 | public string album { get; set; } | 29 | public string album { get; set; } |
419 | 30 | public string album_artist { get; set; } | 30 | public string album_artist { get; set; } |
420 | 31 | public string genre { get; set; } | 31 | public string genre { get; set; } |
422 | 32 | public string track_number { get; set; } | 32 | public int track_number { get; set; } |
423 | 33 | public int year { get; set; } | 33 | public int year { get; set; } |
424 | 34 | public int play_count { get; set; } | 34 | public int play_count { get; set; } |
425 | 35 | } | 35 | } |
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.