Decouple Session from MidiPatchManager and reduce parsing of midnam xml files

The MidiPatchManager only requires a reference to the session to get the path
to the Session midnam directory so change it so that the path is passed to
MidiPatchManager::add_search_path on Session construction and removed on
Session Destruction. This will also make it easier to test and reduce compile
times etc.

For the common case where the Session doesn't have a Session specific midnam
patch files directory(for instance a new session) it won't cause a refresh and
reparsing of all the midnam files. This saves about 2 seconds to load a Session
on my machine(fast machine with SSD), or about half the time spent in the
Session constructor for a new session.

There is still going to be that initial cost of parsing the midnam files when
the first session is created after starting Ardour. Options to remove that
would be to parse the files asynchronously and or use a faster xml
parser(eventually), neither of which seem worth doing at this stage.

This change will cause a performance regression for the uncommon case where a
Session with Session specific midnam files is unloaded and then another Session
with Session specific midnam files is loaded as it will cause the common midnam
files in midi_patch_path to be parsed twice(unload and load).

diff --git a/libs/ardour/ardour/midi_patch_manager.h b/libs/ardour/ardour/midi_patch_manager.h
index 97f9060a5d21c0fa135115b18cfebea9eec3df37..c7154c49a62972aa544805170677892b089b4a36 100644 (file)
--- a/libs/ardour/ardour/midi_patch_manager.h
+++ b/libs/ardour/ardour/midi_patch_manager.h
@@ -22,12 +22,11 @@
 #define MIDI_PATCH_MANAGER_H_
 
 #include "midi++/midnam_patch.h"
+
 #include "pbd/signals.h"
-#include "ardour/session_handle.h"
+#include "pbd/search_path.h"
 
-namespace ARDOUR {
-       class Session;
-}
+#include "ardour/libardour_visibility.h"
 
 namespace MIDI
 {
@@ -35,7 +34,7 @@ namespace MIDI
 namespace Name
 {
 
-class LIBARDOUR_API MidiPatchManager : public PBD::ScopedConnectionList, public ARDOUR::SessionHandlePtr
+class LIBARDOUR_API MidiPatchManager
 {
        /// Singleton
 private:
@@ -58,7 +57,9 @@ public:
                return *_manager;
        }
 
-       void set_session (ARDOUR::Session*);
+       void add_search_path (const PBD::Searchpath& search_path);
+
+       void remove_search_path (const PBD::Searchpath& search_path);
 
        boost::shared_ptr<MIDINameDocument> document_by_model(std::string model_name)
                { return _documents[model_name]; }
@@ -138,12 +139,13 @@ public:
        const DeviceNamesByMaker& devices_by_manufacturer() const { return _devices_by_manufacturer; }
 
 private:
-       void session_going_away();
        void refresh();
-       void add_session_patches();
 
        bool add_midi_name_document(const std::string& file_path);
 
+private:
+       PBD::Searchpath                         _search_path;
+
        MidiNameDocuments                       _documents;
        MIDINameDocument::MasterDeviceNamesList _master_devices_by_model;
        DeviceNamesByMaker                      _devices_by_manufacturer;

diff --git a/libs/ardour/midi_patch_manager.cc b/libs/ardour/midi_patch_manager.cc
index c6e70b2a6cd096dffe58d58b3694ad58329ec697..83a7a20c7f1f8d349c3dd606403c3d681bfc4740 100644 (file)
--- a/libs/ardour/midi_patch_manager.cc
+++ b/libs/ardour/midi_patch_manager.cc
@@ -25,8 +25,6 @@
 #include "pbd/file_utils.h"
 #include "pbd/error.h"
 
-#include "ardour/session.h"
-#include "ardour/session_directory.h"
 #include "ardour/midi_patch_manager.h"
 
 #include "ardour/search_paths.h"
@@ -43,13 +41,56 @@ MidiPatchManager* MidiPatchManager::_manager = 0;
 
 MidiPatchManager::MidiPatchManager ()
 {
+       add_search_path(midi_patch_search_path ());
 }
 
 void
-MidiPatchManager::set_session (Session* s)
+MidiPatchManager::add_search_path (const Searchpath& search_path)
 {
-       SessionHandlePtr::set_session (s);
-       refresh ();
+       bool do_refresh = false;
+
+       for (Searchpath::const_iterator i = search_path.begin(); i != search_path.end(); ++i) {
+
+               if (_search_path.contains(*i)) {
+                       // already processed files from this path
+                       continue;
+               }
+
+               if (!Glib::file_test (*i, Glib::FILE_TEST_EXISTS)) {
+                       continue;
+               }
+
+               if (!Glib::file_test (*i, Glib::FILE_TEST_IS_DIR)) {
+                       continue;
+               }
+
+               _search_path.add_directory (*i);
+               do_refresh = true;
+       }
+
+       if (do_refresh) {
+               refresh();
+       }
+}
+
+void
+MidiPatchManager::remove_search_path (const Searchpath& search_path)
+{
+       bool do_refresh = false;
+
+       for (Searchpath::const_iterator i = search_path.begin(); i != search_path.end(); ++i) {
+
+               if (!_search_path.contains(*i)) {
+                       continue;
+               }
+
+               _search_path.remove_directory (*i);
+               do_refresh = true;
+       }
+
+       if (do_refresh) {
+               refresh();
+       }
 }
 
 bool
@@ -94,32 +135,6 @@ MidiPatchManager::add_midi_name_document (const std::string& file_path)
        return true;
 }
 
-void
-MidiPatchManager::add_session_patches ()
-{
-       if (!_session) {
-               return;
-       }
-
-       std::string path_to_patches = _session->session_directory().midi_patch_path();
-
-       if (!Glib::file_test (path_to_patches, Glib::FILE_TEST_EXISTS)) {
-               return;
-       }
-
-       assert (Glib::file_test (path_to_patches, Glib::FILE_TEST_IS_DIR));
-
-       vector<std::string> result;
-
-       find_files_matching_pattern (result, path_to_patches, "*.midnam");
-
-       info << "Loading " << result.size() << " MIDI patches from " << path_to_patches << endmsg;
-
-       for (vector<std::string>::iterator i = result.begin(); i != result.end(); ++i) {
-               add_midi_name_document(*i);
-       }
-}
-
 void
 MidiPatchManager::refresh()
 {
@@ -128,28 +143,14 @@ MidiPatchManager::refresh()
        _all_models.clear();
        _devices_by_manufacturer.clear();
 
-       Searchpath search_path = midi_patch_search_path ();
        vector<std::string> result;
 
-       find_files_matching_pattern (result, search_path, "*.midnam");
+       find_files_matching_pattern (result, _search_path, "*.midnam");
 
-       info << "Loading " << result.size() << " MIDI patches from " << search_path.to_string() << endmsg;
+       info << "Loading " << result.size() << " MIDI patches from "
+            << _search_path.to_string() << endmsg;
 
        for (vector<std::string>::iterator i = result.begin(); i != result.end(); ++i) {
                add_midi_name_document (*i);
        }
-
-       if (_session) {
-               add_session_patches ();
-       }
-}
-
-void
-MidiPatchManager::session_going_away ()
-{
-       SessionHandlePtr::session_going_away ();
-       _documents.clear();
-       _master_devices_by_model.clear();
-       _all_models.clear();
-       _devices_by_manufacturer.clear();
 }

diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc
index 1231212d2ebdd7ea2e1e0a21a07dedabfa56b20d..a3b82d9d4ba281bf2ed54bd7ae29db9b53e150f6 100644 (file)
--- a/libs/ardour/session.cc
+++ b/libs/ardour/session.cc
@@ -74,6 +74,7 @@
 #include "ardour/graph.h"
 #include "ardour/midiport_manager.h"
 #include "ardour/scene_changer.h"
+#include "ardour/midi_patch_manager.h"
 #include "ardour/midi_track.h"
 #include "ardour/midi_ui.h"
 #include "ardour/operations.h"
@@ -555,6 +556,8 @@ Session::destroy ()
 
        ControlProtocolManager::instance().drop_protocols ();
 
+       MIDI::Name::MidiPatchManager::instance().remove_search_path(session_directory().midi_patch_path());
+
        _engine.remove_session ();
 
 #ifdef USE_TRACKS_CODE_FEATURES

diff --git a/libs/ardour/session_state.cc b/libs/ardour/session_state.cc
index 036e023dff91999a8ee40cf297fe5ca33fca9df7..0cbb550d0252e0d4e976ee1634707dcb82b48fb3 100644 (file)
--- a/libs/ardour/session_state.cc
+++ b/libs/ardour/session_state.cc
@@ -343,7 +343,7 @@ Session::post_engine_init ()
        send_immediate_mmc (MIDI::MachineControlCommand (MIDI::MachineControl::cmdMmcReset));
        send_immediate_mmc (MIDI::MachineControlCommand (Timecode::Time ()));
 
-       MIDI::Name::MidiPatchManager::instance().set_session (this);
+       MIDI::Name::MidiPatchManager::instance().add_search_path (session_directory().midi_patch_path() );
 
        ltc_tx_initialize();
        /* initial program change will be delivered later; see ::config_changed() */