Use a map of region names to speed up

RegionFactory::new_region_name; should help with #2982.

git-svn-id: svn://localhost/ardour2/branches/3.0@12753 d708f5d6-7413-0410-9779-e7cbd77b26cf

diff --git a/libs/ardour/ardour/region_factory.h b/libs/ardour/ardour/region_factory.h
index 4ba261f80c8c3a3981f2236a42237471f326b750..471f172440509548d5aaf24fed11ef92127323d3 100644 (file)
--- a/libs/ardour/ardour/region_factory.h
+++ b/libs/ardour/ardour/region_factory.h
@@ -31,6 +31,7 @@
 #include "ardour/types.h"
 
 class XMLNode;
+class RegionNamingTest;
 
 namespace ARDOUR {
 
@@ -119,6 +120,7 @@ public:
        static void map_add (boost::shared_ptr<Region>);
 
   private:
+       friend class ::RegionNamingTest;
 
        static void region_changed (PBD::PropertyChange const &, boost::weak_ptr<Region>);
 
@@ -126,10 +128,15 @@ public:
 
        static RegionMap region_map;
 
-       static Glib::StaticMutex region_name_map_lock;
-
-       static std::map<std::string, uint32_t> region_name_map;
-       static void update_region_name_map (boost::shared_ptr<Region>);
+       static Glib::StaticMutex region_name_maps_mutex;
+       /** map of partial region names and suffix numbers */
+       static std::map<std::string, uint32_t> region_name_number_map;
+       /** map of complete region names with their region ID */
+       static std::map<std::string, PBD::ID> region_name_map;
+       static void add_to_region_name_maps (boost::shared_ptr<Region>);
+       static void rename_in_region_name_maps (boost::shared_ptr<Region>);
+       static void update_region_name_number_map (boost::shared_ptr<Region>);
+       static void remove_from_region_name_map (std::string);
 
        static PBD::ScopedConnectionList* region_list_connections;
        static CompoundAssociations _compound_associations;

diff --git a/libs/ardour/region_factory.cc b/libs/ardour/region_factory.cc
index fa948844abcf2adc224127a03b1115fc5f8eb555..ddc0451523c356884442f3c8405d14d4e36aac23 100644 (file)
--- a/libs/ardour/region_factory.cc
+++ b/libs/ardour/region_factory.cc
@@ -39,8 +39,9 @@ PBD::Signal1<void,boost::shared_ptr<Region> > RegionFactory::CheckNewRegion;
 Glib::StaticMutex                             RegionFactory::region_map_lock;
 RegionFactory::RegionMap                      RegionFactory::region_map;
 PBD::ScopedConnectionList*                    RegionFactory::region_list_connections = 0;
-Glib::StaticMutex                             RegionFactory::region_name_map_lock;
-std::map<std::string, uint32_t>               RegionFactory::region_name_map;
+Glib::StaticMutex                             RegionFactory::region_name_maps_mutex;
+std::map<std::string, uint32_t>               RegionFactory::region_name_number_map;
+std::map<std::string, PBD::ID>                RegionFactory::region_name_map;
 RegionFactory::CompoundAssociations           RegionFactory::_compound_associations;
 
 boost::shared_ptr<Region>
@@ -320,7 +321,7 @@ RegionFactory::map_add (boost::shared_ptr<Region> r)
        r->DropReferences.connect_same_thread (*region_list_connections, boost::bind (&RegionFactory::map_remove, boost::weak_ptr<Region> (r)));
        r->PropertyChanged.connect_same_thread (*region_list_connections, boost::bind (&RegionFactory::region_changed, _1, boost::weak_ptr<Region> (r)));
 
-       update_region_name_map (r);
+       add_to_region_name_maps (r);
 }
 
 void
@@ -335,6 +336,7 @@ RegionFactory::map_remove (boost::weak_ptr<Region> w)
        RegionMap::iterator i = region_map.find (r->id());
 
        if (i != region_map.end()) {
+               remove_from_region_name_map (i->second->name ());
                region_map.erase (i);
        }
 }
@@ -384,6 +386,7 @@ RegionFactory::clear_map ()
                Glib::Mutex::Lock lm (region_map_lock);
                region_map.clear ();
                _compound_associations.clear ();
+               region_name_map.clear ();
        }
 }
 
@@ -418,8 +421,49 @@ RegionFactory::nregions ()
        return region_map.size ();
 }
 
+/** Add a region to the two region name maps */
 void
-RegionFactory::update_region_name_map (boost::shared_ptr<Region> region)
+RegionFactory::add_to_region_name_maps (boost::shared_ptr<Region> region)
+{
+       update_region_name_number_map (region);
+
+       Glib::Mutex::Lock lm (region_name_maps_mutex);
+       region_name_map[region->name()] = region->id ();
+}
+
+/** Account for a region rename in the two region name maps */
+void
+RegionFactory::rename_in_region_name_maps (boost::shared_ptr<Region> region)
+{
+       update_region_name_number_map (region);
+
+       Glib::Mutex::Lock lm (region_name_maps_mutex);
+       
+       map<string, PBD::ID>::iterator i = region_name_map.begin();
+       while (i != region_name_map.end() && i->second != region->id ()) {
+               ++i;
+       }
+
+       /* Erase the entry for the old name and put in a new one */
+       if (i != region_name_map.end()) {
+               region_name_map.erase (i);
+               region_name_map[region->name()] = region->id ();
+       }
+}
+
+/** Remove a region's details from the region_name_map */
+void
+RegionFactory::remove_from_region_name_map (string n)
+{
+       map<string, PBD::ID>::iterator i = region_name_map.find (n);
+       if (i != region_name_map.end ()) {
+               region_name_map.erase (i);
+       }
+}
+
+/** Update a region's entry in the region_name_number_map */
+void
+RegionFactory::update_region_name_number_map (boost::shared_ptr<Region> region)
 {
        string::size_type const last_period = region->name().find_last_of ('.');
 
@@ -432,8 +476,8 @@ RegionFactory::update_region_name_map (boost::shared_ptr<Region> region)
                   which is just fine
                */
 
-               Glib::Mutex::Lock lm (region_name_map_lock);
-               region_name_map[base] = atoi (number.c_str ());
+               Glib::Mutex::Lock lm (region_name_maps_mutex);
+               region_name_number_map[base] = atoi (number.c_str ());
        }
 }
 
@@ -446,7 +490,7 @@ RegionFactory::region_changed (PropertyChange const & what_changed, boost::weak_
        }
 
        if (what_changed.contains (Properties::name)) {
-               update_region_name_map (r);
+               rename_in_region_name_maps (r);
        }
 }
 
@@ -482,15 +526,15 @@ RegionFactory::region_name (string& result, string base, bool newlevel)
                }
 
                {
-                       Glib::Mutex::Lock lm (region_name_map_lock);
+                       Glib::Mutex::Lock lm (region_name_maps_mutex);
 
                        map<string,uint32_t>::iterator x;
 
                        result = subbase;
 
-                       if ((x = region_name_map.find (subbase)) == region_name_map.end()) {
+                       if ((x = region_name_number_map.find (subbase)) == region_name_number_map.end()) {
                                result += ".1";
-                               region_name_map[subbase] = 1;
+                               region_name_number_map[subbase] = 1;
                        } else {
                                x->second++;
                                snprintf (buf, sizeof (buf), ".%d", x->second);
@@ -555,8 +599,6 @@ RegionFactory::new_region_name (string old)
 
        while (number < (UINT_MAX-1)) {
 
-               const RegionMap& regions (RegionFactory::regions());
-               RegionMap::const_iterator i;
                string sbuf;
 
                number++;
@@ -564,13 +606,7 @@ RegionFactory::new_region_name (string old)
                snprintf (buf, len, "%s%" PRIu32 "%s", old.substr (0, last_period + 1).c_str(), number, remainder.c_str());
                sbuf = buf;
 
-               for (i = regions.begin(); i != regions.end(); ++i) {
-                       if (i->second->name() == sbuf) {
-                               break;
-                       }
-               }
-
-               if (i == regions.end()) {
+               if (region_name_map.find (sbuf) == region_name_map.end ()) {
                        break;
                }
        }
@@ -607,6 +643,7 @@ RegionFactory::remove_regions_using_source (boost::shared_ptr<Source> src)
                ++j;
                
                if (i->second->uses_source (src)) {
+                       remove_from_region_name_map (i->second->name ());
                        region_map.erase (i);
                 }
 

diff --git a/libs/ardour/test/region_naming_test.cc b/libs/ardour/test/region_naming_test.cc
index 464d3225b0f5f97785158a4aafde591925ef60dc..bd32ae64168194bc15c33ba263713aff6a8afa45 100644 (file)
--- a/libs/ardour/test/region_naming_test.cc
+++ b/libs/ardour/test/region_naming_test.cc
@@ -56,3 +56,15 @@ RegionNamingTest::basicsTest ()
                CPPUNIT_ASSERT_EQUAL (s.str(), rA->name());
        }
 }
+
+void
+RegionNamingTest::cacheTest ()
+{
+       /* Check that all the regions in the map are on the name list */
+
+       CPPUNIT_ASSERT_EQUAL (RegionFactory::region_map.size(), RegionFactory::region_name_map.size());
+
+       for (RegionFactory::RegionMap::iterator i = RegionFactory::region_map.begin(); i != RegionFactory::region_map.end(); ++i) {
+               CPPUNIT_ASSERT (RegionFactory::region_name_map.find (i->second->name()) != RegionFactory::region_name_map.end ());
+       }
+}

diff --git a/libs/ardour/test/region_naming_test.h b/libs/ardour/test/region_naming_test.h
index 695792548a144b7887eb73310c68ca43132e27d1..b5be107b3d51f8813c1fd6c328783648c115b0d8 100644 (file)
--- a/libs/ardour/test/region_naming_test.h
+++ b/libs/ardour/test/region_naming_test.h
@@ -22,8 +22,10 @@ class RegionNamingTest : public AudioRegionTest
 {
        CPPUNIT_TEST_SUITE (RegionNamingTest);
        CPPUNIT_TEST (basicsTest);
+       CPPUNIT_TEST (cacheTest);
        CPPUNIT_TEST_SUITE_END ();
 
 public:
        void basicsTest ();
+       void cacheTest ();
 };