ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

Revert some string_view pessimization

Browse Source 
This reverts the configmgr part of 0a7eac8576 "use
more string_view in accessibility..configmgr":  All calls to Data::parseSegment
(either directly, or indirectly either via Access::getSubChild or via
parseSegment in partial.cxx) pass in an OUString path, and in some cases (see
below)

>         *name = path.substr(index, i - index);

in Data::parseSegment reconstructs an OUString from the full path (i.e.,
index == 0 and i == path.size()).  And I see no code that actually benefited
from the switch to string_view.

One example call stack for such an expensive reconstruction of an OUString from
full path is

> #0 in configmgr::Data::parseSegment(path=u"ooSetupSystemLocale", index=0, name=0x7ffff5299280, setElement=0x7ffff52992a0, templateName=0x7ffff52992b0) in core/configmgr/source/data.cxx
> #1 in configmgr::Access::getSubChild(this=0x619000028f80, path=u"ooSetupSystemLocale") in core/configmgr/source/access.cxx
> #2 in configmgr::Access::getByHierarchicalName(this=0x619000028f80, aName="ooSetupSystemLocale") in core/configmgr/source/access.cxx
> #3 in utl::ConfigItem::GetProperties(xHierarchyAccess=uno::Reference to (configmgr::RootAccess *) 0x619000028fb8, rNames=uno::Sequence of length 6 = {...}, bAllLocales=false) in core/unotools/source/config/configitem.cxx
> #4 in utl::ConfigItem::GetProperties(this=0x61100000cad0, rNames=uno::Sequence of length 6 = {...}) in core/unotools/source/config/configitem.cxx
> #5 in SvtSysLocaleOptions_Impl::SvtSysLocaleOptions_Impl(this=0x61100000cad0) in core/unotools/source/config/syslocaleoptions.cxx
[...]

Change-Id: I51127d82aea927dd9aaf374880c406dbafaddcde
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140658
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
This commit is contained in:
Stephan Bergmann
2022-09-27 19:47:55 +02:00
parent 1f3879db3d
commit 5c425ca130
5 changed files with 23 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
configmgr/source/access.cxx
+7 -8
View File
@@ -74,7 +74,6 @@
#include <cppuhelper/queryinterface.hxx>
#include <cppuhelper/supportsservice.hxx>
#include <cppuhelper/weak.hxx>
#include <o3tl/string_view.hxx>
#include <osl/interlck.h>
#include <osl/mutex.hxx>
#include <rtl/character.hxx>
@@ -1971,10 +1970,10 @@ rtl::Reference< ChildAccess > Access::getUnmodifiedChild(
return createUnmodifiedChild(name,node);
}
rtl::Reference< ChildAccess > Access::getSubChild(std::u16string_view path) {
rtl::Reference< ChildAccess > Access::getSubChild(OUString const & path) {
sal_Int32 i = 0;
// For backwards compatibility, allow absolute paths where meaningful:
if( o3tl::starts_with(path, u"/") ) {
if( path.startsWith("/") ) {
++i;
if (!getRootAccess().is()) {
return rtl::Reference< ChildAccess >();
@@ -1987,7 +1986,7 @@ rtl::Reference< ChildAccess > Access::getSubChild(std::u16string_view path) {
OUString templateName1;
i = Data::parseSegment(
path, i, &name1, &setElement1, &templateName1);
if (i == -1 || (i != static_cast<sal_Int32>(path.size()) && path[i] != '/')) {
if (i == -1 || (i != path.getLength() && path[i] != '/')) {
return rtl::Reference< ChildAccess >();
}
OUString name2;
@@ -2000,7 +1999,7 @@ rtl::Reference< ChildAccess > Access::getSubChild(std::u16string_view path) {
{
return rtl::Reference< ChildAccess >();
}
if (i != static_cast<sal_Int32>(path.size())) {
if (i != path.getLength()) {
++i;
}
}
@@ -2010,7 +2009,7 @@ rtl::Reference< ChildAccess > Access::getSubChild(std::u16string_view path) {
bool setElement;
OUString templateName;
i = Data::parseSegment(path, i, &name, &setElement, &templateName);
if (i == -1 || (i != static_cast<sal_Int32>(path.size()) && path[i] != '/')) {
if (i == -1 || (i != path.getLength() && path[i] != '/')) {
return rtl::Reference< ChildAccess >();
}
rtl::Reference< ChildAccess > child(parent->getChild(name));
@@ -2042,9 +2041,9 @@ rtl::Reference< ChildAccess > Access::getSubChild(std::u16string_view path) {
// For backwards compatibility, ignore a final slash after non-value
// nodes:
if (child->isValue()) {
return i == static_cast<sal_Int32>(path.size())
return i == path.getLength()
? child : rtl::Reference< ChildAccess >();
} else if (i >= static_cast<sal_Int32>(path.size()) - 1) {
} else if (i >= path.getLength() - 1) {
return child;
}
++i;
configmgr/source/access.hxx
+1 -1
View File
@@ -366,7 +366,7 @@ private:
rtl::Reference< ChildAccess > getUnmodifiedChild(
OUString const & name);
rtl::Reference< ChildAccess > getSubChild(std::u16string_view path);
rtl::Reference< ChildAccess > getSubChild(OUString const & path);
bool setChildProperty(
OUString const & name, css::uno::Any const & value,
configmgr/source/data.cxx
+10 -10
View File
@@ -110,18 +110,18 @@ OUString Data::createSegment(
}
sal_Int32 Data::parseSegment(
std::u16string_view path, sal_Int32 index, OUString * name,
OUString const & path, sal_Int32 index, OUString * name,
bool * setElement, OUString * templateName)
{
assert(
index >= 0 && index <= static_cast<sal_Int32>(path.size()) && name != nullptr &&
index >= 0 && index <= path.getLength() && name != nullptr &&
setElement != nullptr);
size_t i = index;
while (i < path.size() && path[i] != '/' && path[i] != '[') {
sal_Int32 i = index;
while (i < path.getLength() && path[i] != '/' && path[i] != '[') {
++i;
}
if (i == path.size() || path[i] == '/') {
*name = path.substr(index, i - index);
if (i == path.getLength() || path[i] == '/') {
*name = path.copy(index, i - index);
*setElement = false;
return i;
}
@@ -129,18 +129,18 @@ sal_Int32 Data::parseSegment(
if (i - index == 1 && path[index] == '*') {
templateName->clear();
} else {
*templateName = path.substr(index, i - index);
*templateName = path.copy(index, i - index);
}
}
if (++i == path.size()) {
if (++i == path.getLength()) {
return -1;
}
sal_Unicode del = path[i++];
if (del != '\'' && del != '"') {
return -1;
}
size_t j = path.find(del, i);
if (j == std::u16string_view::npos || j + 1 == path.size() || path[j + 1] != ']' ||
sal_Int32 j = path.indexOf(del, i);
if (j == -1 || j + 1 == path.getLength() || path[j + 1] != ']' ||
!decode(path, i, j, name))
{
return -1;
configmgr/source/data.hxx
+1 -1
View File
@@ -54,7 +54,7 @@ struct Data {
std::u16string_view templateName, OUString const & name);
static sal_Int32 parseSegment(
std::u16string_view path, sal_Int32 index, OUString * name,
OUString const & path, sal_Int32 index, OUString * name,
bool * setElement, OUString * templateName);
static OUString fullTemplateName(
configmgr/source/partial.cxx
+4 -4
View File
@@ -34,10 +34,10 @@ namespace configmgr {
namespace {
bool parseSegment(
std::u16string_view path, sal_Int32 * index, OUString * segment)
OUString const & path, sal_Int32 * index, OUString * segment)
{
assert(
index != nullptr && *index >= 0 && *index <= static_cast<sal_Int32>(path.size()) &&
index != nullptr && *index >= 0 && *index <= path.getLength() &&
segment != nullptr);
if (path[(*index)++] == '/') {
OUString name;
@@ -47,10 +47,10 @@ bool parseSegment(
path, *index, &name, &setElement, &templateName);
if (*index != -1) {
*segment = Data::createSegment(templateName, name);
return *index == static_cast<sal_Int32>(path.size());
return *index == path.getLength();
}
}
throw css::uno::RuntimeException(OUString::Concat("bad path ") + path);
throw css::uno::RuntimeException("bad path " + path);
}
}