From e84fbfe6e5a48915e2e3b01b7867e0f3ff2a64ce Mon Sep 17 00:00:00 2001 From: Tim Mayberry Date: Fri, 23 Sep 2016 22:56:36 +1000 Subject: [PATCH] Remove PropertyMap from XMLNode class It appears that there is no performance benefit from storing properties in a map for faster lookup or it is counteracted by the penalty of storing and maintaining the additional data structure. Timing results before changes with an optimized build: XMLTest::testPerfMediumXMLDocumentTiming Create : Count: 10 Min: 41293 Max: 63746 Total: 564448 Avg: 56444 (56 msecs) Write : Count: 10 Min: 42932 Max: 49221 Total: 453955 Avg: 45395 (45 msecs) Read : Count: 10 Min: 80160 Max: 84678 Total: 824506 Avg: 82450 (82 msecs) XMLTest::testPerfLargeXMLDocumentTiming Create : Count: 10 Min: 228759 Max: 420236 Total: 3587597 Avg: 358759 (358 msecs) Write : Count: 10 Min: 307095 Max: 348767 Total: 3205704 Avg: 320570 (320 msecs) Read : Count: 10 Min: 572400 Max: 657219 Total: 5959630 Avg: 595963 (595 msecs) Perf results after changes: XMLTest::testPerfMediumXMLDocumentTiming Create : Count: 10 Min: 30610 Max: 42656 Total: 376672 Avg: 37667 (37 msecs) Write : Count: 10 Min: 42804 Max: 54277 Total: 460455 Avg: 46045 (46 msecs) Read : Count: 10 Min: 70364 Max: 85484 Total: 750909 Avg: 75090 (75 msecs) XMLTest::testPerfLargeXMLDocumentTiming Create : Count: 10 Min: 164360 Max: 356995 Total: 3064482 Avg: 306448 (306 msecs) Write : Count: 10 Min: 308655 Max: 372953 Total: 3226707 Avg: 322670 (322 msecs) Read : Count: 10 Min: 517243 Max: 541839 Total: 5289950 Avg: 528995 (528 msecs) --- libs/pbd/pbd/xml++.h | 3 -- libs/pbd/xml++.cc | 103 ++++++++++++++++++++++++------------------- 2 files changed, 58 insertions(+), 48 deletions(-) diff --git a/libs/pbd/pbd/xml++.h b/libs/pbd/pbd/xml++.h index fa55f11c35..8e348a4483 100644 --- a/libs/pbd/pbd/xml++.h +++ b/libs/pbd/pbd/xml++.h @@ -29,7 +29,6 @@ #include #include -#include #include #include @@ -50,7 +49,6 @@ typedef XMLNodeList::const_iterator XMLNodeConstIterator; typedef std::vector XMLPropertyList; typedef XMLPropertyList::iterator XMLPropertyIterator; typedef XMLPropertyList::const_iterator XMLPropertyConstIterator; -typedef std::map XMLPropertyMap; class LIBPBD_API XMLTree { public: @@ -149,7 +147,6 @@ private: std::string _content; XMLNodeList _children; XMLPropertyList _proplist; - XMLPropertyMap _propmap; mutable XMLNodeList _selected_children; void clear_lists (); diff --git a/libs/pbd/xml++.cc b/libs/pbd/xml++.cc index 2db08eb3e2..3b0da66dbc 100644 --- a/libs/pbd/xml++.cc +++ b/libs/pbd/xml++.cc @@ -245,7 +245,6 @@ XMLNode::clear_lists () XMLPropertyIterator curprop; _selected_children.clear (); - _propmap.clear (); for (curchild = _children.begin(); curchild != _children.end(); ++curchild) { delete *curchild; @@ -469,87 +468,99 @@ XMLNode::add_content(const string& c) } XMLProperty const * -XMLNode::property(const char* n) const +XMLNode::property(const char* name) const { - string ns(n); - map::const_iterator iter; + XMLPropertyConstIterator iter = _proplist.begin(); - if ((iter = _propmap.find(ns)) != _propmap.end()) { - return iter->second; + while (iter != _proplist.end()) { + if ((*iter)->name() == name) { + return *iter; + } + ++iter; } return 0; } XMLProperty const * -XMLNode::property(const string& ns) const +XMLNode::property(const string& name) const { - map::const_iterator iter; + XMLPropertyConstIterator iter = _proplist.begin(); - if ((iter = _propmap.find(ns)) != _propmap.end()) { - return iter->second; + while (iter != _proplist.end()) { + if ((*iter)->name() == name) { + return *iter; + } + ++iter; } - return 0; } XMLProperty * -XMLNode::property(const char* n) +XMLNode::property(const char* name) { - string ns(n); - map::iterator iter; + XMLPropertyIterator iter = _proplist.begin(); - if ((iter = _propmap.find(ns)) != _propmap.end()) { - return iter->second; + while (iter != _proplist.end()) { + if ((*iter)->name() == name) { + return *iter; + } + ++iter; } - return 0; } XMLProperty * -XMLNode::property(const string& ns) +XMLNode::property(const string& name) { - map::iterator iter; + XMLPropertyIterator iter = _proplist.begin(); - if ((iter = _propmap.find(ns)) != _propmap.end()) { - return iter->second; + while (iter != _proplist.end()) { + if ((*iter)->name() == name) { + return *iter; + } + ++iter; } return 0; } bool -XMLNode::has_property_with_value (const string& key, const string& value) const +XMLNode::has_property_with_value (const string& name, const string& value) const { - map::const_iterator iter = _propmap.find(key); - if (iter != _propmap.end()) { - const XMLProperty* p = (iter->second); - return (p && p->value() == value); + XMLPropertyConstIterator iter = _proplist.begin(); + + while (iter != _proplist.end()) { + if ((*iter)->name() == name && (*iter)->value() == value) { + return true; + } + ++iter; } return false; } XMLProperty* -XMLNode::add_property(const char* n, const string& v) +XMLNode::add_property(const char* name, const string& value) { - string ns(n); - map::iterator iter; + XMLPropertyIterator iter = _proplist.begin(); - if ((iter = _propmap.find(ns)) != _propmap.end()) { - iter->second->set_value (v); - return iter->second; + while (iter != _proplist.end()) { + if ((*iter)->name() == name) { + (*iter)->set_value (value); + return *iter; + } + ++iter; } - XMLProperty* tmp = new XMLProperty(ns, v); + XMLProperty* new_property = new XMLProperty(name, value); - if (!tmp) { + if (!new_property) { return 0; } - _propmap[tmp->name()] = tmp; - _proplist.insert(_proplist.end(), tmp); + _proplist.insert(_proplist.end(), new_property); - return tmp; + return new_property; } XMLProperty* @@ -568,16 +579,18 @@ XMLNode::add_property(const char* name, const long value) } void -XMLNode::remove_property(const string& n) +XMLNode::remove_property(const string& name) { - if (_propmap.find(n) != _propmap.end()) { - XMLProperty* p = _propmap[n]; - XMLPropertyIterator i = std::find(_proplist.begin(), _proplist.end(), p); - if (i != _proplist.end ()) { - _proplist.erase (i); + XMLPropertyIterator iter = _proplist.begin(); + + while (iter != _proplist.end()) { + if ((*iter)->name() == name) { + XMLProperty* property = *iter; + _proplist.erase (iter); + delete property; + break; } - delete p; - _propmap.erase(n); + ++iter; } }