Skip to content

Make JsonEncode() a bit faster #10400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/base/json.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class JsonEncoder
String GetResult();

private:
std::vector<char> m_Result;
std::string m_Result;
String m_CurrentKey;
std::stack<std::bitset<2>> m_CurrentSubtree;

Expand Down Expand Up @@ -450,14 +450,14 @@ template<bool prettyPrint>
inline
String JsonEncoder<prettyPrint>::GetResult()
{
return String(m_Result.begin(), m_Result.end());
return std::move(m_Result);
}

template<bool prettyPrint>
inline
void JsonEncoder<prettyPrint>::AppendChar(char c)
{
m_Result.emplace_back(c);
m_Result.push_back(c);
}

template<bool prettyPrint>
Expand Down
10 changes: 10 additions & 0 deletions lib/base/string.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -333,11 +333,21 @@ String icinga::operator+(const String& lhs, const char *rhs)
return lhs.GetData() + rhs;
}

String icinga::operator+(String&& lhs, const char *rhs)
{
return std::move(lhs.GetData()) + rhs;
}

String icinga::operator+(const char *lhs, const String& rhs)
{
return lhs + rhs.GetData();
}

String icinga::operator+(const char *lhs, String&& rhs)
{
return lhs + std::move(rhs.GetData());
}
Comment on lines +336 to +349
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you get here? Like how did you get to the conclusion that this will be a useful optimization?

(Also, I really wonder if there's actually an implementation of std::string that allows prepending without reallocation (unless you prepend the empty string), but well, a corresponding overload of operator+ is provided for std::string.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you get here? Like how did you get to the conclusion that this will be a useful optimization?

Checkout 27e1850 and Cmd+LClick the + sign in

return stateMachine.GetResult() + "\n";

CLion will open String icinga::operator+(const String&,const char*) which copies the string.

(Also, I really wonder if there's actually an implementation of std::string that allows prepending without reallocation

As long as there's enough free capacity, there's no reason to reallocate.

a corresponding overload of operator+ is provided for std::string.)

Exactly.


bool icinga::operator==(const String& lhs, const String& rhs)
{
return lhs.GetData() == rhs.GetData();
Expand Down
2 changes: 2 additions & 0 deletions lib/base/string.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ std::istream& operator>>(std::istream& stream, String& str);

String operator+(const String& lhs, const String& rhs);
String operator+(const String& lhs, const char *rhs);
String operator+(String&& lhs, const char *rhs);
String operator+(const char *lhs, const String& rhs);
String operator+(const char *lhs, String&& rhs);

bool operator==(const String& lhs, const String& rhs);
bool operator==(const String& lhs, const char *rhs);
Expand Down
Loading