Reduce std::string copies (jbeder#924)

- Don't eagerly convert key to std::string - Make const char* keys streamable when exception is thrown - Don't create a temporary string when comparing a const char* key
zixin96 · Jul 23, 2020 · bc9874c · bc9874c
1 parent 06b99f5
commit bc9874c
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 55 deletions.
diff --git a/include/yaml-cpp/exceptions.h b/include/yaml-cpp/exceptions.h
@@ -100,6 +100,12 @@ inline const std::string KEY_NOT_FOUND_WITH_KEY(const std::string& key) {
   return stream.str();
 }
 
+inline const std::string KEY_NOT_FOUND_WITH_KEY(const char* key) {
+  std::stringstream stream;
+  stream << KEY_NOT_FOUND << ": " << key;
+  return stream.str();
+}
+
 template <typename T>
 inline const std::string KEY_NOT_FOUND_WITH_KEY(
     const T& key, typename enable_if<is_numeric<T>>::type* = 0) {
@@ -120,6 +126,12 @@ inline const std::string BAD_SUBSCRIPT_WITH_KEY(const std::string& key) {
   return stream.str();
 }
 
+inline const std::string BAD_SUBSCRIPT_WITH_KEY(const char* key) {
+  std::stringstream stream;
+  stream << BAD_SUBSCRIPT << " (key: \"" << key << "\")";
+  return stream.str();
+}
+
 template <typename T>
 inline const std::string BAD_SUBSCRIPT_WITH_KEY(
     const T& key, typename enable_if<is_numeric<T>>::type* = nullptr) {

diff --git a/include/yaml-cpp/node/convert.h b/include/yaml-cpp/node/convert.h
@@ -74,12 +74,17 @@ struct convert<std::string> {
 // C-strings can only be encoded
 template <>
 struct convert<const char*> {
-  static Node encode(const char*& rhs) { return Node(rhs); }
+  static Node encode(const char* rhs) { return Node(rhs); }
+};
+
+template <>
+struct convert<char*> {
+  static Node encode(const char* rhs) { return Node(rhs); }
 };
 
 template <std::size_t N>
-struct convert<const char[N]> {
-  static Node encode(const char(&rhs)[N]) { return Node(rhs); }
+struct convert<char[N]> {
+  static Node encode(const char* rhs) { return Node(rhs); }
 };
 
 template <>

diff --git a/include/yaml-cpp/node/detail/impl.h b/include/yaml-cpp/node/detail/impl.h
@@ -106,7 +106,11 @@ inline bool node::equals(const T& rhs, shared_memory_holder pMemory) {
 }
 
 inline bool node::equals(const char* rhs, shared_memory_holder pMemory) {
-  return equals<std::string>(rhs, pMemory);
+  std::string lhs;
+  if (convert<std::string>::decode(Node(*this, std::move(pMemory)), lhs)) {
+    return lhs == rhs;
+  }
+  return false;
 }
 
 // indexing

diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h
@@ -315,51 +315,6 @@ inline void Node::push_back(const Node& rhs) {
   m_pMemory->merge(*rhs.m_pMemory);
 }
 
-// helpers for indexing
-namespace detail {
-template <typename T>
-struct to_value_t {
-  explicit to_value_t(const T& t_) : t(t_) {}
-  const T& t;
-  using return_type = const T &;
-
-  const T& operator()() const { return t; }
-};
-
-template <>
-struct to_value_t<const char*> {
-  explicit to_value_t(const char* t_) : t(t_) {}
-  const char* t;
-  using return_type = std::string;
-
-  const std::string operator()() const { return t; }
-};
-
-template <>
-struct to_value_t<char*> {
-  explicit to_value_t(char* t_) : t(t_) {}
-  const char* t;
-  using return_type = std::string;
-
-  const std::string operator()() const { return t; }
-};
-
-template <std::size_t N>
-struct to_value_t<char[N]> {
-  explicit to_value_t(const char* t_) : t(t_) {}
-  const char* t;
-  using return_type = std::string;
-
-  const std::string operator()() const { return t; }
-};
-
-// converts C-strings to std::strings so they can be copied
-template <typename T>
-inline typename to_value_t<T>::return_type to_value(const T& t) {
-  return to_value_t<T>(t)();
-}
-}  // namespace detail
-
 template<typename Key>
 std::string key_to_string(const Key& key) {
   return streamable_to_string<Key, is_streamable<std::stringstream, Key>::value>().impl(key);
@@ -369,8 +324,8 @@ std::string key_to_string(const Key& key) {
 template <typename Key>
 inline const Node Node::operator[](const Key& key) const {
   EnsureNodeExists();
-  detail::node* value = static_cast<const detail::node&>(*m_pNode).get(
-      detail::to_value(key), m_pMemory);
+  detail::node* value =
+      static_cast<const detail::node&>(*m_pNode).get(key, m_pMemory);
   if (!value) {
     return Node(ZombieNode, key_to_string(key));
   }
@@ -380,14 +335,14 @@ inline const Node Node::operator[](const Key& key) const {
 template <typename Key>
 inline Node Node::operator[](const Key& key) {
   EnsureNodeExists();
-  detail::node& value = m_pNode->get(detail::to_value(key), m_pMemory);
+  detail::node& value = m_pNode->get(key, m_pMemory);
   return Node(value, m_pMemory);
 }
 
 template <typename Key>
 inline bool Node::remove(const Key& key) {
   EnsureNodeExists();
-  return m_pNode->remove(detail::to_value(key), m_pMemory);
+  return m_pNode->remove(key, m_pMemory);
 }
 
 inline const Node Node::operator[](const Node& key) const {
@@ -420,8 +375,7 @@ inline bool Node::remove(const Node& key) {
 template <typename Key, typename Value>
 inline void Node::force_insert(const Key& key, const Value& value) {
   EnsureNodeExists();
-  m_pNode->force_insert(detail::to_value(key), detail::to_value(value),
-                        m_pMemory);
+  m_pNode->force_insert(key, value, m_pMemory);
 }
 
 // free functions