comphelper/source/misc/configuration.cxx | 90 ++++++++++++++++++++++++-------
1 file changed, 70 insertions(+), 20 deletions(-)
New commits:
commit 7df433cdc33b4d6ba38eafad9282d015571433ef
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Mon Feb 28 13:01:16 2022 +0100
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Mon Mar 7 15:37:41 2022 +0100
optimize ConfigurationProperty::get()
E.g. officecfg::Office::Common::Save::ODF::DefaultVersion::get()
gets called quite often during ODS export, taking ~14% of total CPU
time, ~%5 in comphelper::getProcessComponentContext (handled by
previous commit) and the rest trying to read the property.
But it's possible to cache the final property values if there's
a notifier that clears the cache on any configuration change (it
can't be hidden as a static in the file like the other data
becase that way it'd get destroyed too late and crash). This
gets it to ~3%, most of which is spent in computing the path hash
(which could be presumably made const if the OUString hash function
were constexpr).
Change-Id: I29f3cefc8442716403d4df8bdf253f4fe847a09c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130705
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/comphelper/source/misc/configuration.cxx
b/comphelper/source/misc/configuration.cxx
index 46ac8ab6c407..11c3f97ad1cd 100644
--- a/comphelper/source/misc/configuration.cxx
+++ b/comphelper/source/misc/configuration.cxx
@@ -10,11 +10,11 @@
#include <sal/config.h>
#include <cassert>
-#include <map>
-#include <memory>
#include <mutex>
#include <string_view>
+#include <unordered_map>
+#include <com/sun/star/beans/NamedValue.hpp>
#include <com/sun/star/beans/PropertyAttribute.hpp>
#include <com/sun/star/configuration/ReadOnlyAccess.hpp>
#include <com/sun/star/configuration/ReadWriteAccess.hpp>
@@ -27,9 +27,12 @@
#include <com/sun/star/lang/XLocalizable.hpp>
#include <com/sun/star/uno/Any.hxx>
#include <com/sun/star/uno/Reference.hxx>
+#include <com/sun/star/util/XChangesListener.hpp>
+#include <com/sun/star/util/XChangesNotifier.hpp>
#include <comphelper/solarmutex.hxx>
#include <comphelper/configuration.hxx>
#include <comphelper/configurationlistener.hxx>
+#include <cppuhelper/implbase.hxx>
#include <rtl/ustring.hxx>
#include <sal/log.hxx>
#include <i18nlangtag/languagetag.hxx>
@@ -111,13 +114,67 @@ comphelper::detail::ConfigurationWrapper::get()
return GetTheConfigurationWrapper();
}
+namespace
+{
+std::mutex gMutex;
+std::unordered_map<OUString, css::uno::Any> gPropertyCache;
+css::uno::Reference< css::util::XChangesNotifier > gNotifier;
+css::uno::Reference< css::util::XChangesListener > gListener;
+
+class ConfigurationChangesListener
+ : public ::cppu::WeakImplHelper<css::util::XChangesListener>
+{
+public:
+ ConfigurationChangesListener()
+ {}
+ // util::XChangesListener
+ virtual void SAL_CALL changesOccurred( const css::util::ChangesEvent& )
override
+ {
+ std::scoped_lock aGuard(gMutex);
+ gPropertyCache.clear();
+ }
+ virtual void SAL_CALL disposing(const css::lang::EventObject&) override {}
+};
+
+} // namespace
+
comphelper::detail::ConfigurationWrapper::ConfigurationWrapper(
css::uno::Reference< css::uno::XComponentContext > const & context):
context_(context),
access_(css::configuration::ReadWriteAccess::create(context, "*"))
-{}
+{
+ // Set up a configuration notifier to invalidate the cache as needed.
+ try
+ {
+ css::uno::Reference< css::lang::XMultiServiceFactory > xConfigProvider(
+ css::configuration::theDefaultProvider::get( context ) );
+
+ // set root path
+ css::uno::Sequence< css::uno::Any > params {
+ css::uno::makeAny( css::beans::NamedValue{ "nodepath",
css::uno::makeAny( OUString("/"))} ),
+ css::uno::makeAny( css::beans::NamedValue{ "locale",
css::uno::makeAny( OUString("*"))} ) };
+
+ css::uno::Reference< css::uno::XInterface > xCfg
+ =
xConfigProvider->createInstanceWithArguments(u"com.sun.star.configuration.ConfigurationAccess",
+ params);
+
+ gNotifier = css::uno::Reference< css::util::XChangesNotifier >(xCfg,
css::uno::UNO_QUERY);
+ assert(gNotifier.is());
+ gListener = css::uno::Reference< ConfigurationChangesListener >(new
ConfigurationChangesListener());
+ gNotifier->addChangesListener(gListener);
+ }
+ catch(const css::uno::Exception&)
+ {
+ assert(false);
+ }
+}
-comphelper::detail::ConfigurationWrapper::~ConfigurationWrapper() {}
+comphelper::detail::ConfigurationWrapper::~ConfigurationWrapper()
+{
+ gPropertyCache.clear();
+ gNotifier.clear();
+ gListener.clear();
+}
bool comphelper::detail::ConfigurationWrapper::isReadOnly(OUString const &
path)
const
@@ -130,29 +187,22 @@ bool
comphelper::detail::ConfigurationWrapper::isReadOnly(OUString const & path)
css::uno::Any
comphelper::detail::ConfigurationWrapper::getPropertyValue(OUString const&
path) const
{
+ std::scoped_lock aGuard(gMutex);
// Cache the configuration access, since some of the keys are used in hot
code.
- // Note that this cache is only used by the officecfg:: auto-generated
code, using it for anything
- // else would be unwise because the cache could end up containing stale
entries.
- static std::mutex gMutex;
- static std::map<OUString, css::uno::Reference< css::container::XNameAccess
>> gAccessMap;
+ auto it = gPropertyCache.find(path);
+ if( it != gPropertyCache.end())
+ return it->second;
sal_Int32 idx = path.lastIndexOf("/");
assert(idx!=-1);
OUString parentPath = path.copy(0, idx);
OUString childName = path.copy(idx+1);
- std::scoped_lock aGuard(gMutex);
-
- // check cache
- auto it = gAccessMap.find(parentPath);
- if (it == gAccessMap.end())
- {
- // not in the cache, look it up
- css::uno::Reference<css::container::XNameAccess> access(
- access_->getByHierarchicalName(parentPath),
css::uno::UNO_QUERY_THROW);
- it = gAccessMap.emplace(parentPath, access).first;
- }
- return it->second->getByName(childName);
+ css::uno::Reference<css::container::XNameAccess> access(
+ access_->getByHierarchicalName(parentPath), css::uno::UNO_QUERY_THROW);
+ css::uno::Any property = access->getByName(childName);
+ gPropertyCache.emplace(path, property);
+ return property;
}
void comphelper::detail::ConfigurationWrapper::setPropertyValue(
