[Libreoffice-commits] online.git: common/Session.cpp common/StringVector.cpp common/StringVector.hpp kit/ChildSession.cpp test/WhiteBoxTests.cpp wsd/ClientSession.cpp wsd/DocumentBroker.cpp wsd/LOOLWSD.cpp
Ashod Nakashian (via logerrit)
logerrit at kemper.freedesktop.org
Mon Jun 1 23:39:56 UTC 2020
common/Session.cpp | 2 -
common/StringVector.cpp | 73 +-----------------------------------------------
common/StringVector.hpp | 71 ++++++++++++++++++++++++++++++++++++----------
kit/ChildSession.cpp | 12 +++----
test/WhiteBoxTests.cpp | 3 +
wsd/ClientSession.cpp | 2 -
wsd/DocumentBroker.cpp | 2 -
wsd/LOOLWSD.cpp | 3 -
8 files changed, 69 insertions(+), 99 deletions(-)
New commits:
commit 784b7dc39d880448e5599c19e65798d7207a412a
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
AuthorDate: Sun May 31 14:16:58 2020 -0400
Commit: Ashod Nakashian <ashnakash at gmail.com>
CommitDate: Tue Jun 2 01:39:37 2020 +0200
wsd: optimize StringVector
StringVector is heavily used for tokenization
and benefits from inlining of small functions.
Also, cat doesn't need to be slower than necessary.
Change-Id: I4ab2ff1b1f1a81092049d2cde64b6df10b34b5f7
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95287
Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
Tested-by: Jenkins
diff --git a/common/Session.cpp b/common/Session.cpp
index 0b3499a05..204573e0f 100644
--- a/common/Session.cpp
+++ b/common/Session.cpp
@@ -200,7 +200,7 @@ void Session::parseDocOptions(const StringVector& tokens, int& part, std::string
if (getTokenString(tokens[offset], "options", _docOptions))
{
if (tokens.size() > offset + 1)
- _docOptions += tokens.cat(std::string(" "), offset + 1);
+ _docOptions += tokens.cat(' ', offset + 1);
}
}
}
diff --git a/common/StringVector.cpp b/common/StringVector.cpp
index aab884881..2fef5aa0b 100644
--- a/common/StringVector.cpp
+++ b/common/StringVector.cpp
@@ -17,7 +17,7 @@ StringVector::StringVector(const std::string& string, const std::vector<StringTo
{
}
-std::string StringVector::operator[](size_t index) const
+std::string StringVector::operator[](std::size_t index) const
{
if (index >= _tokens.size())
{
@@ -28,76 +28,7 @@ std::string StringVector::operator[](size_t index) const
return _string.substr(token._index, token._length);
}
-size_t StringVector::size() const { return _tokens.size(); }
-
-bool StringVector::empty() const { return _tokens.empty(); }
-
-std::vector<StringToken>::const_iterator StringVector::begin() const { return _tokens.begin(); }
-
-std::vector<StringToken>::iterator StringVector::begin() { return _tokens.begin(); }
-
-std::vector<StringToken>::const_iterator StringVector::end() const { return _tokens.end(); }
-
-std::vector<StringToken>::iterator StringVector::end() { return _tokens.end(); }
-
-std::vector<StringToken>::iterator StringVector::erase(std::vector<StringToken>::const_iterator it)
-{
- return _tokens.erase(it);
-}
-
-void StringVector::push_back(const std::string& string)
-{
- StringToken token;
- token._index = _string.length();
- token._length = string.length();
- _tokens.push_back(token);
- _string += string;
-}
-
-std::string StringVector::getParam(const StringToken& token) const
-{
- return _string.substr(token._index, token._length);
-}
-
-std::string StringVector::cat(const std::string& separator, size_t offset) const
-{
- std::string ret;
- bool first = true;
-
- if (offset >= _tokens.size())
- {
- return ret;
- }
-
- for (auto it = _tokens.begin() + offset; it != _tokens.end(); ++it)
- {
- if (first)
- {
- first = false;
- }
- else
- {
- ret += separator;
- }
-
- ret += getParam(*it);
- }
-
- return ret;
-}
-
-bool StringVector::equals(size_t index, const char* string) const
-{
- if (index >= _tokens.size())
- {
- return false;
- }
-
- const StringToken& token = _tokens[index];
- return _string.compare(token._index, token._length, string) == 0;
-}
-
-bool StringVector::equals(size_t index, const StringVector& other, size_t otherIndex)
+bool StringVector::equals(std::size_t index, const StringVector& other, std::size_t otherIndex)
{
if (index >= _tokens.size())
{
diff --git a/common/StringVector.hpp b/common/StringVector.hpp
index 154060bf1..fa0e94491 100644
--- a/common/StringVector.hpp
+++ b/common/StringVector.hpp
@@ -17,12 +17,12 @@
*/
struct StringToken
{
- size_t _index;
- size_t _length;
+ std::size_t _index;
+ std::size_t _length;
StringToken() = default;
- StringToken(size_t index, size_t length)
+ StringToken(std::size_t index, std::size_t length)
: _index(index),
_length(length)
{
@@ -45,35 +45,74 @@ public:
explicit StringVector(const std::string& string, const std::vector<StringToken>& tokens);
/// Unlike std::vector, gives an empty string if index is unexpected.
- std::string operator[](size_t index) const;
+ std::string operator[](std::size_t index) const;
- size_t size() const;
+ std::size_t size() const { return _tokens.size(); }
- bool empty() const;
+ bool empty() const { return _tokens.empty(); }
- std::vector<StringToken>::const_iterator begin() const;
+ std::vector<StringToken>::const_iterator begin() const { return _tokens.begin(); }
- std::vector<StringToken>::iterator begin();
+ std::vector<StringToken>::iterator begin() { return _tokens.begin(); }
- std::vector<StringToken>::const_iterator end() const;
+ std::vector<StringToken>::const_iterator end() const { return _tokens.end(); }
- std::vector<StringToken>::iterator end();
+ std::vector<StringToken>::iterator end() { return _tokens.end(); }
- std::vector<StringToken>::iterator erase(std::vector<StringToken>::const_iterator it);
+ std::vector<StringToken>::iterator erase(std::vector<StringToken>::const_iterator it)
+ {
+ return _tokens.erase(it);
+ }
- void push_back(const std::string& string);
+ void push_back(const std::string& string)
+ {
+ _tokens.emplace_back(_string.size(), string.size());
+ _string += string;
+ }
/// Gets the underlying string of a single token.
- std::string getParam(const StringToken& token) const;
+ std::string getParam(const StringToken& token) const
+ {
+ return _string.substr(token._index, token._length);
+ }
/// Concats tokens starting from begin, using separator as separator.
- std::string cat(const std::string& separator, size_t begin) const;
+ template <typename T> inline std::string cat(const T& separator, std::size_t offset) const
+ {
+ std::string ret;
+
+ if (offset >= _tokens.size())
+ {
+ return ret;
+ }
+
+ ret.reserve(_string.size() * 2);
+ auto it = _tokens.begin() + offset;
+ ret = getParam(*it);
+ for (++it; it != _tokens.end(); ++it)
+ {
+ // Avoid temporary strings, append separately.
+ ret += separator;
+ ret += getParam(*it);
+ }
+
+ return ret;
+ }
/// Compares the nth token with string.
- bool equals(size_t index, const char* string) const;
+ bool equals(std::size_t index, const char* string) const
+ {
+ if (index >= _tokens.size())
+ {
+ return false;
+ }
+
+ const StringToken& token = _tokens[index];
+ return _string.compare(token._index, token._length, string) == 0;
+ }
/// Compares the nth token with the mth token from an other StringVector.
- bool equals(size_t index, const StringVector& other, size_t otherIndex);
+ bool equals(std::size_t index, const StringVector& other, std::size_t otherIndex);
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/kit/ChildSession.cpp b/kit/ChildSession.cpp
index 9b62f1c06..2526819c6 100644
--- a/kit/ChildSession.cpp
+++ b/kit/ChildSession.cpp
@@ -699,7 +699,7 @@ bool ChildSession::sendFontRendering(const char* /*buffer*/, int /*length*/, con
return false;
}
- const std::string response = "renderfont: " + tokens.cat(std::string(" "), 1) + '\n';
+ const std::string response = "renderfont: " + tokens.cat(' ', 1) + '\n';
std::vector<char> output;
output.resize(response.size());
@@ -916,7 +916,7 @@ bool ChildSession::downloadAs(const char* /*buffer*/, int /*length*/, const Stri
{
if (tokens.size() > 5)
{
- filterOptions += tokens.cat(std::string(" "), 5);
+ filterOptions += tokens.cat(' ', 5);
}
}
@@ -1426,7 +1426,7 @@ bool ChildSession::dialogEvent(const char* /*buffer*/, int /*length*/, const Str
unsigned nLOKWindowId = std::stoi(tokens[1].c_str());
getLOKitDocument()->sendDialogEvent(nLOKWindowId,
- tokens.cat(std::string(" "), 2).c_str());
+ tokens.cat(' ', 2).c_str());
return true;
}
@@ -1498,9 +1498,7 @@ bool ChildSession::unoCommand(const char* /*buffer*/, int /*length*/, const Stri
}
else
{
- getLOKitDocument()->postUnoCommand(tokens[1].c_str(),
- tokens.cat(std::string(" "), 2).c_str(),
- bNotify);
+ getLOKitDocument()->postUnoCommand(tokens[1].c_str(), tokens.cat(' ', 2).c_str(), bNotify);
}
return true;
@@ -2033,7 +2031,7 @@ bool ChildSession::saveAs(const char* /*buffer*/, int /*length*/, const StringVe
{
if (tokens.size() > 4)
{
- filterOptions += tokens.cat(std::string(" "), 4);
+ filterOptions += tokens.cat(' ', 4);
}
}
diff --git a/test/WhiteBoxTests.cpp b/test/WhiteBoxTests.cpp
index 66f3e895f..a4be56bb9 100644
--- a/test/WhiteBoxTests.cpp
+++ b/test/WhiteBoxTests.cpp
@@ -815,6 +815,9 @@ void WhiteBoxTests::testStringVector()
// Test cat().
CPPUNIT_ASSERT_EQUAL(std::string("a b"), vector.cat(" ", 0));
+ CPPUNIT_ASSERT_EQUAL(std::string("a b"), vector.cat(' ', 0));
+ CPPUNIT_ASSERT_EQUAL(std::string("a*b"), vector.cat('*', 0));
+ CPPUNIT_ASSERT_EQUAL(std::string("a blah mlah b"), vector.cat(" blah mlah ", 0));
CPPUNIT_ASSERT_EQUAL(std::string(), vector.cat(" ", 3));
CPPUNIT_ASSERT_EQUAL(std::string(), vector.cat(" ", 42));
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index d9e6eb0a4..e3934b1f2 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -885,7 +885,7 @@ bool ClientSession::sendFontRendering(const char *buffer, int length, const Stri
TileCache::Tile cachedTile = docBroker->tileCache().lookupCachedStream(TileCache::StreamType::Font, font+text);
if (cachedTile)
{
- const std::string response = "renderfont: " + tokens.cat(std::string(" "), 1) + '\n';
+ const std::string response = "renderfont: " + tokens.cat(' ', 1) + '\n';
return sendTile(response, cachedTile);
}
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index a3dece823..7dd78664e 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -2069,7 +2069,7 @@ bool DocumentBroker::forwardToChild(const std::string& viewId, const std::string
msg = tokens[0] + ' ' + tokens[1] + ' ' + tokens[2];
msg += " jail=" + _uriJailed;
msg += " xjail=" + _uriJailedAnonym;
- msg += ' ' + tokens.cat(std::string(" "), 3);
+ msg += ' ' + tokens.cat(' ', 3);
}
_childProcess->sendTextFrame(msg);
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 930809bc1..bb5aa4aa4 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1793,8 +1793,7 @@ bool LOOLWSD::createForKit()
// ForKit always spawns one.
++OutstandingForks;
- LOG_INF("Launching forkit process: " << forKitPath << ' ' <<
- args.cat(std::string(" "), 0));
+ LOG_INF("Launching forkit process: " << forKitPath << ' ' << args.cat(' ', 0));
LastForkRequestTime = std::chrono::steady_clock::now();
int child = Util::spawnProcess(forKitPath, args);
More information about the Libreoffice-commits
mailing list