GNOME Bugzilla – Bug 778778
[PATCH] Optimize startup time by avoiding reading all mime-types from files again
Last modified: 2017-02-19 12:53:00 UTC
Created attachment 345971 [details] [review] patch Dear maintainer(s), I am a long-standing Rygel user, and for me it's one of the best solutions for streaming videos in my household with almost no configuration. Thanks for your effort! However, I have a quite large amount of files which get indexed by the media export plugin (thousands of videos, tens of thousands of music files, tens of thousands of picture files...). All of that is stored on a slow rotational drive, and a cold startup of Rygel can take up to 10 minutes for me, with a lot of disk thrashing. By using: sudo bash -c "echo 3 > /proc/sys/vm/drop_caches" and strace'ing rygel, I noticed that all files (even if already in the cache) were opened at each startup to determine their mime type. Here is a patch that always re-uses the mime type from the cache if it is up-to-date (followed by a trivial patch that fixes my build from a clean directory). This is my first attempt to a patch to Rygel and in Vala, so comments are welcome: I might have gotten something wrong. However, with it, I see at least a ten-fold performance improvement on a cold start. Cheers!
Created attachment 345973 [details] [review] build fix patch
Review of attachment 345973 [details] [review]: This will break out of tree build
Review of attachment 345971 [details] [review]: Hi, thanks for the patch, some minor style adjustments, then it's fine to go in :) ::: src/plugins/media-export/rygel-media-export-harvester.vala @@ +82,2 @@ if (is_blacklisted) { + debug ("URI %s is not eligble due to blacklisting", WS-only change? ::: src/plugins/media-export/rygel-media-export-harvesting-task.vala @@ +152,3 @@ */ private bool push_if_changed_or_unknown (File file, + FileInfo info) { indent wrong @@ +175,3 @@ } else { + if (info.get_content_type() != "") + { Curly to the previous line, please @@ +219,2 @@ if (info.get_file_type () == FileType.DIRECTORY) { + What's this change doing? @@ +325,3 @@ private void on_extracted_cb (File file, Variant? info) { + Same here?
Review of attachment 345971 [details] [review]: ::: src/plugins/media-export/rygel-media-export-harvester.vala @@ +82,2 @@ if (is_blacklisted) { + debug ("URI %s is not eligble due to blacklisting", Nope, there was a spelling error ;-) blacklising vs. blacklisting
Created attachment 345999 [details] [review] patch Here we go, I made another small optimization while I was at it.
(In reply to Jens Georg from comment #2) > Review of attachment 345973 [details] [review] [review]: > > This will break out of tree build For me it does build only in-tree WITHOUT this patch... example: make[3]: Entering directory '/home/matteo/.cache/gnome-builder/builds/Rygel/local/x86_64-linux-gnu/data/xml' cd /home/matteo/Progetti/rygel && /bin/bash /home/matteo/Progetti/rygel/build-aux/missing automake-1.15 --gnu data/xml/Makefile cd ../.. && /bin/bash ./config.status data/xml/Makefile config.status: creating data/xml/Makefile make[3]: *** No rule to make target 'MediaServer3.xml', needed by 'all-am'. Stop. make[3]: Leaving directory '/home/matteo/.cache/gnome-builder/builds/Rygel/local/x86_64-linux-gnu/data/xml' Makefile:775: recipe for target 'all-recursive' failed make[2]: *** [all-recursive] Error 1 make[2]: Leaving directory '/home/matteo/.cache/gnome-builder/builds/Rygel/local/x86_64-linux-gnu/data' Makefile:596: recipe for target 'all-recursive' failed make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory '/home/matteo/.cache/gnome-builder/builds/Rygel/local/x86_64-linux-gnu' Makefile:497: recipe for target 'all' failed make: *** [all] Error 2 ---------------------- matteo@settenvini:~/.cache/gnome-builder/builds/Rygel/local/x86_64-linux-gnu/data/xml$ make --debug=i GNU Make 4.1 Built for x86_64-pc-linux-gnu Copyright (C) 1988-2014 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Reading makefiles... Updating goal targets.... File 'all' does not exist. File 'all-am' does not exist. Looking for an implicit rule for 'EnergyManagement.xml'. Trying pattern rule with stem 'EnergyManagement'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/EnergyManagement.xml.in'. Found an implicit rule for 'EnergyManagement.xml'. Looking for an implicit rule for '/home/matteo/Progetti/rygel/data/xml/EnergyManagement.xml.in'. Trying pattern rule with stem 'EnergyManagement.xml.in'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/EnergyManagement.xml.in,v'. Trying pattern rule with stem 'EnergyManagement.xml.in'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/RCS/EnergyManagement.xml.in,v'. Trying pattern rule with stem 'EnergyManagement.xml.in'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/RCS/EnergyManagement.xml.in'. Trying pattern rule with stem 'EnergyManagement.xml.in'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/s.EnergyManagement.xml.in'. Trying pattern rule with stem 'EnergyManagement.xml.in'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/SCCS/s.EnergyManagement.xml.in'. No implicit rule found for '/home/matteo/Progetti/rygel/data/xml/EnergyManagement.xml.in'. File 'MediaServer3.xml' does not exist. Looking for an implicit rule for 'MediaServer3.xml'. Trying pattern rule with stem 'MediaServer3'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/MediaServer3.xml.in'. Trying pattern rule with stem 'MediaServer3.xml'. Trying implicit prerequisite 'MediaServer3.xml,v'. Trying pattern rule with stem 'MediaServer3.xml'. Trying implicit prerequisite 'RCS/MediaServer3.xml,v'. Trying pattern rule with stem 'MediaServer3.xml'. Trying implicit prerequisite 'RCS/MediaServer3.xml'. Trying pattern rule with stem 'MediaServer3.xml'. Trying implicit prerequisite 's.MediaServer3.xml'. Trying pattern rule with stem 'MediaServer3.xml'. Trying implicit prerequisite 'SCCS/s.MediaServer3.xml'. Trying pattern rule with stem 'MediaServer3'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/MediaServer3.xml.in'. Looking for a rule with intermediate file '/home/matteo/Progetti/rygel/data/xml/MediaServer3.xml.in'. Avoiding implicit rule recursion. Trying pattern rule with stem 'MediaServer3.xml.in'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/MediaServer3.xml.in,v'. Trying pattern rule with stem 'MediaServer3.xml.in'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/RCS/MediaServer3.xml.in,v'. Trying pattern rule with stem 'MediaServer3.xml.in'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/RCS/MediaServer3.xml.in'. Trying pattern rule with stem 'MediaServer3.xml.in'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/s.MediaServer3.xml.in'. Trying pattern rule with stem 'MediaServer3.xml.in'. Trying implicit prerequisite '/home/matteo/Progetti/rygel/data/xml/SCCS/s.MediaServer3.xml.in'. No implicit rule found for 'MediaServer3.xml'. Must remake target 'MediaServer3.xml'. make: *** No rule to make target 'MediaServer3.xml', needed by 'all-am'. Stop.
(In reply to Matteo Settenvini from comment #6) > (In reply to Jens Georg from comment #2) > > Review of attachment 345973 [details] [review] [review] [review]: > > > > This will break out of tree build > > For me it does build only in-tree WITHOUT this patch... example: > > make[3]: Entering directory > '/home/matteo/.cache/gnome-builder/builds/Rygel/local/x86_64-linux-gnu/data/ > xml' Ah, nasty. I usually don't do out-of-tree builds from git since vala will (or used to) put the C files into the source dir anyway. Can you make this a separate ticket, please?
Review of attachment 345999 [details] [review]: Minor style issues, otherwise good to go ::: src/plugins/media-export/rygel-media-export-harvesting-task.vala @@ +173,3 @@ } } else { + if (info.get_content_type() != "") { Space before ( @@ +177,3 @@ + (HARVESTER_MIME_TYPE_ATTRIBUTES, + FileQueryInfoFlags.NONE); + info.set_content_type(extended_info.get_content_type()); Minor nitpick: Space before (
Created attachment 346068 [details] [review] patch Whitespace fixes. Obsoleting also build fix patch, moving to a separate bug.
THere's an issue there: without the patch, I get rygel-media-export-metadata-extractor.vala:268: Sent command to extractor process: EXTRACT file:///home/jens/Pictures/2013/05/25/DSCF0524.RAF|image/x-fuji-raf With the patch: rygel-media-export-metadata-extractor.vala:268: Sent command to extractor process: EXTRACT file:///home/jens/Pictures/2013/05/25/DSCF0524.RAF|directory/inode Which breaks the blacklisting
> Which breaks the blacklisting Ah. No it doesn't but the rest still applies
But it seems to break the image fast path for the initial crawl
Created attachment 346146 [details] [review] patch Thanks for catching that; it seems I was overzealous with the previous refactoring, and was querying the mime_type of the origin folder instead of the current file. This should fix it.
Thanks! Before: MediaExport-Message: rygel-media-export-harvesting-task.vala:300: Harvesting of file:///home/jens/Videos done in 0,604746 MediaExport-Message: “file:///home/jens/Music” harvested MediaExport-Message: rygel-media-export-harvesting-task.vala:300: Harvesting of file:///home/jens/Music done in 1,677375 MediaExport-Message: “file:///home/jens/Pictures” harvested MediaExport-Message: rygel-media-export-harvesting-task.vala:300: Harvesting of file:///home/jens/Pictures done in 2,103084 After: MediaExport-Message: rygel-media-export-harvesting-task.vala:309: Harvesting of file:///home/jens/Videos done in 0,273338 MediaExport-Message: “file:///home/jens/Music” harvested MediaExport-Message: rygel-media-export-harvesting-task.vala:309: Harvesting of file:///home/jens/Music done in 1,006804 MediaExport-Message: “file:///home/jens/Pictures” harvested MediaExport-Message: rygel-media-export-harvesting-task.vala:309: Harvesting of file:///home/jens/Pictures done in 1,398075