Bug#37586193 Use std::string_view for charset/collation lookup

We have various instances of std::unordered_map<std::string, CHARSET_INFO*>
for looking up collations and character sets.  With a slight rewrite,
we can use std::string_view rather than std::string for lookup.
This is more light-weight, as it saves memory allocation and deallocation.

Converting Name::m_normalized from a raw pointer to std::string is done
in order to simplify the code.

Change-Id: Ica799aa4db35dc605bc8208ffc9a1c254ff55bbe

Author: Tor Didriksen

include/mysql/strings/collations.h

@@ -26,6 +26,7 @@
 
 #include <cstddef>
 #include <string>
+#include <string_view>
 
 #include "mysql/strings/api.h"
 
@@ -75,28 +76,22 @@ class MYSQL_STRINGS_EXPORT Name {
     @note throws std::bad_alloc
   */
   Name(const char *name, size_t size);
+  ~Name() = default;
 
-  /**
-    Constructor
-
-    @note throws std::bad_alloc
-  */
-  Name(const Name &);
-
-  Name(Name &&) noexcept;
-
-  ~Name();
-
-  Name &operator=(const Name &);
-  Name &operator=(Name &&) noexcept;
+  // These must be explicitly defined for clang on Windows due to
+  // __declspec(dllexport).
+  Name(const Name &) = default;
+  Name(Name &&) = default;
+  Name &operator=(const Name &) = default;
+  Name &operator=(Name &&) = default;
 
   /**
-    Returns normalized name as std::string
+    Returns normalized name as std::string_view.
   */
-  std::string operator()() const { return m_normalized; }
+  std::string_view to_string_view() const { return m_normalized; }
 
  private:
-  const char *m_normalized{nullptr};
+  std::string m_normalized;
 };
 
 /**

mysys/charset.cc

@@ -294,7 +294,8 @@ CHARSET_INFO *my_collation_get_by_name(const char *collation_name, myf flags,
   if (cs == nullptr && (flags & MY_WME)) {
     char index_file[FN_REFLEN + sizeof(MY_CHARSET_INDEX)];
     my_stpcpy(get_charsets_dir(index_file), MY_CHARSET_INDEX);
-    my_error(EE_UNKNOWN_COLLATION, MYF(0), name().c_str(), index_file);
+    my_error(EE_UNKNOWN_COLLATION, MYF(0),
+             std::string(name.to_string_view()).c_str(), index_file);
   }
   return cs;
 }
@@ -325,7 +326,7 @@ CHARSET_INFO *my_charset_get_by_name(const char *cs_name, uint cs_flags,
   CHARSET_INFO *cs = nullptr;
   if (cs_flags & MY_CS_PRIMARY) {
     cs = entry()->find_primary(name, flags, errmsg);
-    if (cs == nullptr && name() == "utf8") {
+    if (cs == nullptr && name.to_string_view() == "utf8") {
       // The parser does get_charset_by_csname().
       // Also needed for e.g. SET character_set_client= 'utf8'.
       // Also needed by the lexer for: "select _utf8 0xD0B0D0B1D0B2;"
@@ -334,7 +335,7 @@ CHARSET_INFO *my_charset_get_by_name(const char *cs_name, uint cs_flags,
     }
   } else if (cs_flags & MY_CS_BINSORT) {
     cs = entry()->find_default_binary(name, flags, errmsg);
-    if (cs == nullptr && name() == "utf8") {
+    if (cs == nullptr && name.to_string_view() == "utf8") {
       cs = entry()->find_default_binary(mysql::collation::Name("utf8mb3"),
                                         flags, errmsg);
     }

strings/CMakeLists.txt

@@ -128,6 +128,10 @@ ELSEIF(WIN32)
     PUBLIC MYSQL_LIBSTRINGS_DLL
     PRIVATE MYSQL_LIBSTRINGS_EXPORT
     )
+  IF(WIN32_VS)
+    # m_normalized ... needs to have dll-interface to be used by client ...
+    TARGET_COMPILE_OPTIONS(strings_shared PUBLIC /wd4251)
+  ENDIF()
 ELSE()
   ADD_SHARED_LIBRARY(strings_shared
     $<TARGET_OBJECTS:strings_objlib>

strings/collations.cc

@@ -49,53 +49,16 @@ mysql::collation::Name::Name(const char *name, size_t size) {
   }
 
   // TODO(gleb): fail instead of truncating too long names?
-  size_t const truncated_size = std::min(size, MY_CS_BUFFER_SIZE);
-  char *normalized = new char[truncated_size + 1];
+  const size_t truncated_size = std::min(size, MY_CS_BUFFER_SIZE);
+  m_normalized.reserve(truncated_size);
 
   for (size_t i = 0; i < truncated_size; i++) {
     // TODO(gleb): use ASCII instead of Latin1?
-    normalized[i] = static_cast<char>(
-        my_charset_latin1.to_lower[static_cast<uint8_t>(name[i])]);
+    m_normalized.push_back(static_cast<char>(
+        my_charset_latin1.to_lower[static_cast<uint8_t>(name[i])]));
   }
-  normalized[truncated_size] = '\0';
-  m_normalized = normalized;
 }
 
-mysql::collation::Name::Name(const mysql::collation::Name &name) {
-  size_t const size = strlen(name.m_normalized);
-  char *normalized = new char[size + 1];
-  memcpy(normalized, name.m_normalized, size + 1);
-  m_normalized = normalized;
-}
-
-mysql::collation::Name::Name(mysql::collation::Name &&name) noexcept
-    : m_normalized(name.m_normalized) {
-  name.m_normalized = nullptr;
-}
-
-mysql::collation::Name::~Name() { delete[] m_normalized; }
-
-/// @cond Doxygen_is_confused
-mysql::collation::Name &mysql::collation::Name::Name::operator=(
-    const Name &name) {
-  if (this == &name) {
-    return *this;
-  }
-  this->~Name();
-  new (this) Name(name);
-  return *this;
-}
-
-mysql::collation::Name &mysql::collation::Name::Name::operator=(
-    Name &&name) noexcept {
-  if (this != &name) {
-    this->~Name();
-    new (this) Name(std::move(name));
-  }
-  return *this;
-}
-/// @endcond
-
 void mysql::collation::initialize(const char *charset_dir,
                                   MY_CHARSET_LOADER *loader) {
   assert(mysql::collation_internals::entry == nullptr);

strings/collations_internal.cc

@@ -29,6 +29,7 @@
 #include <cstdlib>
 #include <memory>
 #include <new>
+#include <string_view>
 
 #include "mysql/my_loglevel.h"
 #include "mysql/strings/collations.h"
@@ -545,28 +546,36 @@ class Charset_loader : public MY_CHARSET_LOADER {
   void *read_file(const char *, size_t *) override { return nullptr; }
 };
 
-template <typename Key>
-using Hash = std::unordered_map<Key, CHARSET_INFO *>;
+using id_hash_map = mysql::collation_internals::id_hash_map;
+using sv_hash_map = mysql::collation_internals::sv_hash_map;
 
-template <typename Key>
-CHARSET_INFO *find_in_hash(const Hash<Key> &hash, Key key) {
+CHARSET_INFO *find_in_hash(const sv_hash_map &hash, std::string_view key) {
+  auto it = hash.find((key));
+  return it == hash.end() ? nullptr : it->second;
+}
+
+CHARSET_INFO *find_in_hash(const id_hash_map &hash, unsigned key) {
   auto it = hash.find(key);
   return it == hash.end() ? nullptr : it->second;
 }
 
-CHARSET_INFO *find_collation_in_hash(const Hash<std::string> &hash,
-                                     const std::string &key) {
+CHARSET_INFO *find_collation_in_hash(const sv_hash_map &hash,
+                                     std::string_view key) {
   return find_in_hash(hash, key);
 }
 
-CHARSET_INFO *find_cs_in_hash(const Hash<std::string> &hash,
-                              const mysql::collation::Name &key) {
-  auto it = hash.find(key());
+CHARSET_INFO *find_cs_in_hash(const sv_hash_map &hash, std::string_view key) {
+  auto it = hash.find(key);
   return it == hash.end() ? nullptr : it->second;
 }
 
-template <typename Key>
-bool add_to_hash(Hash<Key> *hash, Key key, CHARSET_INFO *cs) {
+bool add_to_hash(id_hash_map *hash, unsigned key, CHARSET_INFO *cs) {
+  //  return !hash->insert({key, cs}).second;
+  (*hash)[key] = cs;
+  return false;
+}
+
+bool add_to_hash(sv_hash_map *hash, std::string key, CHARSET_INFO *cs) {
   //  return !hash->insert({key, cs}).second;
   (*hash)[key] = cs;
   return false;
@@ -639,7 +648,8 @@ Collations::~Collations() {
 CHARSET_INFO *Collations::find_by_name(const mysql::collation::Name &name,
                                        myf flags, MY_CHARSET_ERRMSG *errmsg) {
   return safe_init_when_necessary(
-      find_collation_in_hash(m_all_by_collation_name, name()), flags, errmsg);
+      find_collation_in_hash(m_all_by_collation_name, name.to_string_view()),
+      flags, errmsg);
 }
 
 CHARSET_INFO *Collations::find_by_id(unsigned id, myf flags,
@@ -650,31 +660,36 @@ CHARSET_INFO *Collations::find_by_id(unsigned id, myf flags,
 CHARSET_INFO *Collations::find_primary(const mysql::collation::Name &cs_name,
                                        myf flags, MY_CHARSET_ERRMSG *errmsg) {
   return safe_init_when_necessary(
-      find_cs_in_hash(m_primary_by_cs_name, cs_name), flags, errmsg);
+      find_cs_in_hash(m_primary_by_cs_name, cs_name.to_string_view()), flags,
+      errmsg);
 }
 
 CHARSET_INFO *Collations::find_default_binary(
     const mysql::collation::Name &cs_name, myf flags,
     MY_CHARSET_ERRMSG *errmsg) {
-  return safe_init_when_necessary(find_cs_in_hash(m_binary_by_cs_name, cs_name),
-                                  flags, errmsg);
+  return safe_init_when_necessary(
+      find_cs_in_hash(m_binary_by_cs_name, cs_name.to_string_view()), flags,
+      errmsg);
 }
 
 unsigned Collations::get_collation_id(
     const mysql::collation::Name &name) const {
-  CHARSET_INFO *cs = find_collation_in_hash(m_all_by_collation_name, name());
+  CHARSET_INFO *cs =
+      find_collation_in_hash(m_all_by_collation_name, name.to_string_view());
   return cs ? cs->number : 0;
 }
 
 unsigned Collations::get_primary_collation_id(
     const mysql::collation::Name &name) const {
-  CHARSET_INFO *cs = find_cs_in_hash(m_primary_by_cs_name, name);
+  CHARSET_INFO *cs =
+      find_cs_in_hash(m_primary_by_cs_name, name.to_string_view());
   return cs ? cs->number : 0;
 }
 
 unsigned Collations::get_default_binary_collation_id(
     const mysql::collation::Name &name) const {
-  CHARSET_INFO *cs = find_cs_in_hash(m_binary_by_cs_name, name);
+  CHARSET_INFO *cs =
+      find_cs_in_hash(m_binary_by_cs_name, name.to_string_view());
   return cs ? cs->number : 0;
 }
 
@@ -719,7 +734,8 @@ CHARSET_INFO *Collations::unsafe_init(CHARSET_INFO *cs,
 bool Collations::add_internal_collation(CHARSET_INFO *cs) {
   assert(cs->number != 0);
 
-  std::string const normalized_name{mysql::collation::Name{cs->m_coll_name}()};
+  const std::string normalized_name{
+      mysql::collation::Name{cs->m_coll_name}.to_string_view()};
 
   if (add_to_hash(&m_all_by_collation_name, normalized_name, cs) ||
       add_to_hash(&m_all_by_id, cs->number, cs)) {
@@ -746,7 +762,7 @@ bool Collations::add_internal_collation(CHARSET_INFO *cs) {
 
 CHARSET_INFO *Collations::find_by_name_unsafe(
     const mysql::collation::Name &name) {
-  return find_collation_in_hash(m_all_by_collation_name, name());
+  return find_collation_in_hash(m_all_by_collation_name, name.to_string_view());
 }
 
 }  // namespace mysql::collation_internals

strings/collations_internal.h

@@ -27,6 +27,7 @@
 #include <functional>
 #include <mutex>
 #include <string>
+#include <string_view>
 #include <unordered_map>
 #include <utility>
 
@@ -44,6 +45,18 @@ class Name;
 
 namespace collation_internals {
 
+using id_hash_map = std::unordered_map<unsigned, CHARSET_INFO *>;
+
+struct string_hash {
+  using is_transparent = void;
+  [[nodiscard]] size_t operator()(std::string_view txt) const {
+    return std::hash<std::string_view>{}(txt);
+  }
+};
+
+using sv_hash_map = std::unordered_map<std::string, CHARSET_INFO *, string_hash,
+                                       std::equal_to<>>;
+
 /**
   Helper class: implementation of character set/collation library
 
@@ -238,22 +251,10 @@ class Collations final {
   */
   const std::string m_charset_dir;
 
-  /**
-    Common parametric type to map character set/collation names or their ids
-    to CHARSET_INFO object pointers
-
-    @tparam Key         Name or id type (std::string or unsigned respectively)
-
-    TODO(gleb): it would be good to use mysql::collation::Name instead of
-                std::string for Key.
-  */
-  template <typename Key>
-  using Hash = std::unordered_map<Key, CHARSET_INFO *>;
-
   /**
     Maps collation ids to CHARSET_INFO object pointers
   */
-  Hash<unsigned> m_all_by_id;
+  id_hash_map m_all_by_id;
 
   /**
     Maps normalized strings of all known character set names, collation names,
@@ -262,7 +263,7 @@ class Collations final {
     @note see old_conv and get_old_charset_by_name() for exclusions
     @see old_conv(), get_old_charset_by_name()
   */
-  Hash<std::string> m_all_by_collation_name;
+  sv_hash_map m_all_by_collation_name;
 
   /**
     Maps normalized strings of character set names to CHARSET_INFO object
@@ -271,7 +272,7 @@ class Collations final {
     @note In MySQL, CHARSET_INFO object of character set is also an object
     of its primary collation.
   */
-  Hash<std::string> m_primary_by_cs_name;
+  sv_hash_map m_primary_by_cs_name;
 
   /**
     Maps normalized strings of character set names to CHARSET_INFO objects
@@ -280,7 +281,7 @@ class Collations final {
     @note utf8mb4 has two separate binary collations, so m_binary_by_cs_name
           contains a reference to utf8mb4_bin only.
   */
-  Hash<std::string> m_binary_by_cs_name;
+  sv_hash_map m_binary_by_cs_name;
 
   /**
     False if m_loader references external MY_CHARSET_LOADER, otherwise true.

strings/ctype.cc

@@ -564,8 +564,9 @@ static int cs_value(MY_XML_PARSER *st, const char *attr, size_t len) {
       // Replace "utf8_" with "utf8mb3_" for external character sets.
       // Convert to lowercase first.
       mysql::collation::Name normalized_name(attr, len);
-      if (normalized_name().starts_with("utf8_")) {
-        std::string collation_name_string = normalized_name();
+      if (normalized_name.to_string_view().starts_with("utf8_")) {
+        std::string collation_name_string =
+            std::string(normalized_name.to_string_view());
         // insert "mb3" to get "utf8mb3_xxx"
         collation_name_string.insert(4, "mb3");
         i->cs.m_coll_name =
@@ -579,7 +580,7 @@ static int cs_value(MY_XML_PARSER *st, const char *attr, size_t len) {
       // Replace "utf8" with "utf8mb3" for external character sets.
       // Convert to lowercase first.
       mysql::collation::Name normalized_name(attr, len);
-      if (normalized_name() == "utf8") {
+      if (normalized_name.to_string_view() == "utf8") {
         i->cs.csname =
             mstr(i->csname, STRING_WITH_LEN("utf8mb3"), MY_CS_NAME_SIZE - 1);
       } else {