reduce reference counting traffic in configmgr

no need to construct a vector when iterating over children

Change-Id: I717e92be3c576a6e5d877f4333264a5bed9daadf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158728
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 55 insertions, 29 deletions

diff --git a/configmgr/source/access.cxx b/configmgr/source/access.cxx
index 46657374030a..4bb66cb5f025 100644
--- a/configmgr/source/access.cxx
+++ b/configmgr/source/access.cxx
@@ -354,7 +354,7 @@ sal_Bool Access::hasElements() {
     assert(thisIs(IS_ANY));
     osl::MutexGuard g(*lock_);
     checkLocalizedPropertyAccess();
-    return !getAllChildren().empty(); //TODO: optimize
+    return !isAllChildrenEmpty();
 }
 
 bool Access::getByNameFast(const OUString & name, css::uno::Any & value)
@@ -410,14 +410,13 @@ css::uno::Sequence< OUString > Access::getElementNames()
     assert(thisIs(IS_ANY));
     osl::MutexGuard g(*lock_);
     checkLocalizedPropertyAccess();
-    std::vector< rtl::Reference< ChildAccess > > children(getAllChildren());
-    css::uno::Sequence<OUString> names(children.size());
-    OUString* pArray = names.getArray();
-    for (auto const& child : children)
+    std::vector<OUString> childNames;
+    forAllChildren([&childNames] (ChildAccess& rChild)
     {
-        *pArray++ = child->getNameInternal();
-    }
-    return names;
+        childNames.push_back(rChild.getNameInternal());
+        return true;
+    });
+    return comphelper::containerToSequence(childNames);
 }
 
 sal_Bool Access::hasByName(OUString const & aName)
@@ -537,13 +536,12 @@ css::uno::Sequence< css::beans::Property > Access::getProperties()
 {
     assert(thisIs(IS_GROUP));
     osl::MutexGuard g(*lock_);
-    std::vector< rtl::Reference< ChildAccess > > children(getAllChildren());
     std::vector< css::beans::Property > properties;
-    properties.reserve(children.size());
-    for (auto const& child : children)
+    forAllChildren([&properties] (ChildAccess& rChild)
     {
-        properties.push_back(child->asProperty());
-    }
+        properties.push_back(rChild.asProperty());
+        return true;
+    });
     return comphelper::containerToSequence(properties);
 }
 
@@ -1423,19 +1421,22 @@ rtl::Reference< ChildAccess > Access::getChild(OUString const & name) {
                 locale = locale.copy(0, i);
             }
             assert(!locale.isEmpty());
-            std::vector< rtl::Reference< ChildAccess > > children(
-                getAllChildren());
-            for (auto const& child : children)
+            rtl::Reference< ChildAccess > foundChild;
+            forAllChildren([&foundChild, &locale] (ChildAccess& rChild)
             {
-                const OUString & name2(child->getNameInternal());
+                const OUString & name2(rChild.getNameInternal());
                 if (name2.startsWith(locale) &&
                     (name2.getLength() == locale.getLength() ||
                      name2[locale.getLength()] == '-' ||
                      name2[locale.getLength()] == '_'))
                 {
-                    return child;
+                    foundChild = &rChild;
+                    return false;
                 }
-            }
+                return true;
+            });
+            if (foundChild)
+                return foundChild;
         }
         // Defaults are the "en-US" locale, the "en" locale, the empty string locale, the first child (if
         // any, and if the property is non-nillable), or a null ChildAccess, in that order:
@@ -1452,9 +1453,15 @@ rtl::Reference< ChildAccess > Access::getChild(OUString const & name) {
             return child;
         }
         if (!static_cast<LocalizedPropertyNode *>(getNode().get())->isNillable()) {
-            std::vector< rtl::Reference< ChildAccess > > children(getAllChildren());
-            if (!children.empty()) {
-                return children.front();
+            // look for first child in list
+            rtl::Reference< ChildAccess > foundChild;
+            forAllChildren([&foundChild] (ChildAccess& rChild)
+            {
+                foundChild = &rChild;
+                return false;
+            });
+            if (foundChild) {
+                return foundChild;
             }
         }
         return rtl::Reference< ChildAccess >();
@@ -1464,14 +1471,14 @@ rtl::Reference< ChildAccess > Access::getChild(OUString const & name) {
         ? getUnmodifiedChild(name) : getModifiedChild(i);
 }
 
-std::vector< rtl::Reference< ChildAccess > > Access::getAllChildren() {
-    std::vector< rtl::Reference< ChildAccess > > vec;
+void Access::forAllChildren(const std::function<bool(ChildAccess&)> & func) {
     NodeMap const & members = getNode()->getMembers();
     for (auto const& member : members)
     {
         if (modifiedChildren_.find(member.first) == modifiedChildren_.end()) {
-            vec.push_back(getUnmodifiedChild(member.first));
-            assert(vec.back().is());
+            bool bContinue = func(*getUnmodifiedChild(member.first));
+            if (!bContinue)
+                return;
         }
     }
     for (ModifiedChildren::iterator i(modifiedChildren_.begin());
@@ -1479,10 +1486,28 @@ std::vector< rtl::Reference< ChildAccess > > Access::getAllChildren() {
     {
         rtl::Reference< ChildAccess > child(getModifiedChild(i));
         if (child.is()) {
-            vec.push_back(child);
+            bool bContinue = func(*child);
+            if (!bContinue)
+                return;
         }
     }
-    return vec;
+}
+
+bool Access::isAllChildrenEmpty() {
+    NodeMap const & members = getNode()->getMembers();
+    for (auto const& member : members)
+    {
+        if (modifiedChildren_.find(member.first) == modifiedChildren_.end())
+            return false;
+    }
+    for (ModifiedChildren::iterator i(modifiedChildren_.begin());
+         i != modifiedChildren_.end(); ++i)
+    {
+        rtl::Reference< ChildAccess > child(getModifiedChild(i));
+        if (child.is())
+            return false;
+    }
+    return true;
 }
 
 void Access::checkValue(css::uno::Any const & value, Type type, bool nillable) {
diff --git a/configmgr/source/access.hxx b/configmgr/source/access.hxx
index 51e43d5bcfcc..7b59e81cfdc8 100644
--- a/configmgr/source/access.hxx
+++ b/configmgr/source/access.hxx
@@ -323,7 +323,8 @@ protected:
 
     rtl::Reference< Node > getParentNode();
     rtl::Reference< ChildAccess > getChild(OUString const & name);
-    std::vector< rtl::Reference< ChildAccess > > getAllChildren();
+    void forAllChildren(const std::function<bool(ChildAccess&)> & f);
+    bool isAllChildrenEmpty();
 
     void checkValue(
         css::uno::Any const & value, Type type, bool nillable);