Core/DataStores: Simplified string memory allocation in db2 files, dropped unneccessary level of indirection

4 files changed, 36 insertions, 55 deletions

diff --git a/src/server/shared/DataStores/DB2DatabaseLoader.cpp b/src/server/shared/DataStores/DB2DatabaseLoader.cpp
index 9386f7726bf..3c46fb66506 100644
--- a/src/server/shared/DataStores/DB2DatabaseLoader.cpp
+++ b/src/server/shared/DataStores/DB2DatabaseLoader.cpp
@@ -22,10 +22,6 @@
 #include "Errors.h"
 #include "Log.h"
 
-DB2LoadInfo::DB2LoadInfo() : DB2FileLoadInfo(), Statement(MAX_HOTFIXDATABASE_STATEMENTS)
-{
-}
-
 DB2LoadInfo::DB2LoadInfo(DB2FieldMeta const* fields, std::size_t fieldCount, DB2Meta const* meta, HotfixDatabaseStatements statement)
     : DB2FileLoadInfo(fields, fieldCount, meta), Statement(statement)
 {
@@ -84,6 +80,7 @@ char* DB2DatabaseLoader::Load(bool custom, uint32& records, char**& indexTable,
     }
 
     char* tempDataTable = new char[result->GetRowCount() * recordSize];
+    memset(tempDataTable, 0, result->GetRowCount() * recordSize);
     uint32* newIndexes = new uint32[result->GetRowCount()];
     if (stringFields)
         stringPool.reserve(std::max(stringPool.capacity(), stringPool.size() + stringFields * result->GetRowCount() + 1));
@@ -95,9 +92,9 @@ char* DB2DatabaseLoader::Load(bool custom, uint32& records, char**& indexTable,
     {
         Field* fields = result->Fetch();
         uint32 offset = 0;
-        uint32 stringFieldOffset = 0;
 
         uint32 indexValue = fields[indexField].GetUInt32();
+        bool isNew = false;
 
         // Attempt to overwrite existing data
         char* dataValue = indexTable[indexValue];
@@ -105,6 +102,7 @@ char* DB2DatabaseLoader::Load(bool custom, uint32& records, char**& indexTable,
         {
             newIndexes[newRecords] = indexValue;
             dataValue = &tempDataTable[newRecords++ * recordSize];
+            isNew = true;
         }
 
         uint32 f = 0;
@@ -143,29 +141,28 @@ char* DB2DatabaseLoader::Load(bool custom, uint32& records, char**& indexTable,
                         break;
                     case FT_STRING:
                     {
-                        LocalizedString** slot = (LocalizedString**)(&dataValue[offset]);
-                        *slot = (LocalizedString*)(&stringHolders[stringHoldersRecordPoolSize * rec + stringFieldOffset]);
-                        ASSERT(*slot);
+                        LocalizedString* slot = (LocalizedString*)(&dataValue[offset]);
+                        if (isNew)
+                            for (char const*& localeStr : slot->Str)
+                                localeStr = nullStr;
 
                         // Value in database in main table field must be for enUS locale
-                        if (char* str = AddString(&(*slot)->Str[LOCALE_enUS], fields[f].GetString()))
+                        if (char* str = AddString(&slot->Str[LOCALE_enUS], fields[f].GetString()))
                             stringPool.push_back(str);
 
-                        stringFieldOffset += sizeof(LocalizedString);
-                        offset += sizeof(char*);
+                        offset += sizeof(LocalizedString);
                         break;
                     }
                     case FT_STRING_NOT_LOCALIZED:
                     {
                         char const** slot = (char const**)(&dataValue[offset]);
-                        *slot = (char*)(&stringHolders[stringHoldersRecordPoolSize * rec + stringFieldOffset]);
-                        ASSERT(*slot);
 
                         // Value in database in main table field must be for enUS locale
                         if (char* str = AddString(slot, fields[f].GetString()))
                             stringPool.push_back(str);
+                        else
+                            *slot = nullStr;
 
-                        stringFieldOffset += sizeof(char*);
                         offset += sizeof(char*);
                         break;
                     }
@@ -205,7 +202,7 @@ char* DB2DatabaseLoader::Load(bool custom, uint32& records, char**& indexTable,
     return dataTable;
 }
 
-void DB2DatabaseLoader::LoadStrings(bool custom, uint32 locale, uint32 records, char** indexTable, std::vector<char*>& stringPool)
+void DB2DatabaseLoader::LoadStrings(bool custom, LocaleConstant locale, uint32 records, char** indexTable, std::vector<char*>& stringPool)
 {
     HotfixDatabasePreparedStatement* stmt = HotfixDatabase.GetPreparedStatement(HotfixDatabaseStatements(_loadInfo->Statement + 2));
     stmt->setBool(0, !custom);
@@ -265,13 +262,12 @@ void DB2DatabaseLoader::LoadStrings(bool custom, uint32 locale, uint32 records,
                         case FT_STRING:
                         {
                             // fill only not filled entries
-                            LocalizedString* db2str = *(LocalizedString**)(&dataValue[offset]);
-                            if (db2str->Str[locale] == nullStr)
-                                if (char* str = AddString(&db2str->Str[locale], fields[1 + stringFieldNumInRecord].GetString()))
-                                    stringPool.push_back(str);
+                            LocalizedString* db2str = (LocalizedString*)(&dataValue[offset]);
+                            if (char* str = AddString(&db2str->Str[locale], fields[1 + stringFieldNumInRecord].GetString()))
+                                stringPool.push_back(str);
 
                             ++stringFieldNumInRecord;
-                            offset += sizeof(LocalizedString*);
+                            offset += sizeof(LocalizedString);
                             break;
                         }
                         case FT_STRING_NOT_LOCALIZED:
@@ -298,16 +294,6 @@ char* DB2DatabaseLoader::AddString(char const** holder, std::string const& value
 {
     if (!value.empty())
     {
-        std::size_t existingLength = strlen(*holder);
-        if (existingLength >= value.length())
-        {
-            // Reuse existing storage if there is enough space
-            char* str = const_cast<char*>(*holder);
-            memcpy(str, value.c_str(), value.length());
-            str[value.length()] = '\0';
-            return nullptr;
-        }
-
         char* str = new char[value.length() + 1];
         memcpy(str, value.c_str(), value.length());
         str[value.length()] = '\0';
diff --git a/src/server/shared/DataStores/DB2DatabaseLoader.h b/src/server/shared/DataStores/DB2DatabaseLoader.h
index 7038bbcb770..a8389e7cde7 100644
--- a/src/server/shared/DataStores/DB2DatabaseLoader.h
+++ b/src/server/shared/DataStores/DB2DatabaseLoader.h
@@ -26,7 +26,6 @@ enum HotfixDatabaseStatements : uint32;
 
 struct TC_SHARED_API DB2LoadInfo : public DB2FileLoadInfo
 {
-    DB2LoadInfo();
     DB2LoadInfo(DB2FieldMeta const* fields, std::size_t fieldCount, DB2Meta const* meta, HotfixDatabaseStatements statement);
 
     HotfixDatabaseStatements Statement;
@@ -38,7 +37,7 @@ public:
     DB2DatabaseLoader(std::string const& storageName, DB2LoadInfo const* loadInfo) : _storageName(storageName), _loadInfo(loadInfo) { }
 
     char* Load(bool custom, uint32& records, char**& indexTable, char*& stringHolders, std::vector<char*>& stringPool);
-    void LoadStrings(bool custom, uint32 locale, uint32 records, char** indexTable, std::vector<char*>& stringPool);
+    void LoadStrings(bool custom, LocaleConstant locale, uint32 records, char** indexTable, std::vector<char*>& stringPool);
     static char* AddString(char const** holder, std::string const& value);
 
 private:
diff --git a/src/server/shared/DataStores/DB2Store.cpp b/src/server/shared/DataStores/DB2Store.cpp
index 8445f4177bb..0c99462ec5d 100644
--- a/src/server/shared/DataStores/DB2Store.cpp
+++ b/src/server/shared/DataStores/DB2Store.cpp
@@ -35,7 +35,7 @@ DB2StorageBase::~DB2StorageBase()
         delete[] strings;
 }
 
-void DB2StorageBase::WriteRecordData(char const* entry, uint32 locale, ByteBuffer& buffer) const
+void DB2StorageBase::WriteRecordData(char const* entry, LocaleConstant locale, ByteBuffer& buffer) const
 {
     if (!_loadInfo->Meta->HasIndexFieldInData())
         entry += 4;
@@ -68,12 +68,8 @@ void DB2StorageBase::WriteRecordData(char const* entry, uint32 locale, ByteBuffe
                     break;
                 case FT_STRING:
                 {
-                    LocalizedString* locStr = *(LocalizedString**)entry;
-                    if (locStr->Str[locale][0] == '\0')
-                        locale = 0;
-
-                    buffer << locStr->Str[locale];
-                    entry += sizeof(LocalizedString*);
+                    buffer << (*(LocalizedString*)entry)[locale];
+                    entry += sizeof(LocalizedString);
                     break;
                 }
                 case FT_STRING_NOT_LOCALIZED:
@@ -87,7 +83,7 @@ void DB2StorageBase::WriteRecordData(char const* entry, uint32 locale, ByteBuffe
     }
 }
 
-bool DB2StorageBase::Load(std::string const& path, uint32 locale, char**& indexTable)
+bool DB2StorageBase::Load(std::string const& path, LocaleConstant locale, char**& indexTable)
 {
     indexTable = nullptr;
     DB2FileLoader db2;
@@ -114,7 +110,7 @@ bool DB2StorageBase::Load(std::string const& path, uint32 locale, char**& indexT
     return indexTable != nullptr;
 }
 
-bool DB2StorageBase::LoadStringsFrom(std::string const& path, uint32 locale, char** indexTable)
+bool DB2StorageBase::LoadStringsFrom(std::string const& path, LocaleConstant locale, char** indexTable)
 {
     // DB2 must be already loaded using Load
     if (!indexTable)
@@ -151,7 +147,7 @@ void DB2StorageBase::LoadFromDB(char**& indexTable)
     _stringPool.shrink_to_fit();
 }
 
-void DB2StorageBase::LoadStringsFromDB(uint32 locale, char** indexTable)
+void DB2StorageBase::LoadStringsFromDB(LocaleConstant locale, char** indexTable)
 {
     if (!_loadInfo->GetStringFieldCount(true))
         return;
diff --git a/src/server/shared/DataStores/DB2Store.h b/src/server/shared/DataStores/DB2Store.h
index 3c479aa3d0c..c670d418b13 100644
--- a/src/server/shared/DataStores/DB2Store.h
+++ b/src/server/shared/DataStores/DB2Store.h
@@ -37,24 +37,24 @@ public:
     uint32 GetLayoutHash() const { return _layoutHash; }
 
     virtual bool HasRecord(uint32 id) const = 0;
-    virtual void WriteRecord(uint32 id, uint32 locale, ByteBuffer& buffer) const = 0;
+    virtual void WriteRecord(uint32 id, LocaleConstant locale, ByteBuffer& buffer) const = 0;
     virtual void EraseRecord(uint32 id) = 0;
 
     std::string const& GetFileName() const { return _fileName; }
     uint32 GetFieldCount() const { return _fieldCount; }
     DB2LoadInfo const* GetLoadInfo() const { return _loadInfo; }
 
-    virtual bool Load(std::string const& path, uint32 locale) = 0;
-    virtual bool LoadStringsFrom(std::string const& path, uint32 locale) = 0;
+    virtual bool Load(std::string const& path, LocaleConstant locale) = 0;
+    virtual bool LoadStringsFrom(std::string const& path, LocaleConstant locale) = 0;
     virtual void LoadFromDB() = 0;
-    virtual void LoadStringsFromDB(uint32 locale) = 0;
+    virtual void LoadStringsFromDB(LocaleConstant locale) = 0;
 
 protected:
-    void WriteRecordData(char const* entry, uint32 locale, ByteBuffer& buffer) const;
-    bool Load(std::string const& path, uint32 locale, char**& indexTable);
-    bool LoadStringsFrom(std::string const& path, uint32 locale, char** indexTable);
+    void WriteRecordData(char const* entry, LocaleConstant locale, ByteBuffer& buffer) const;
+    bool Load(std::string const& path, LocaleConstant locale, char**& indexTable);
+    bool LoadStringsFrom(std::string const& path, LocaleConstant locale, char** indexTable);
     void LoadFromDB(char**& indexTable);
-    void LoadStringsFromDB(uint32 locale, char** indexTable);
+    void LoadStringsFromDB(LocaleConstant locale, char** indexTable);
 
     uint32 _tableHash;
     uint32 _layoutHash;
@@ -73,7 +73,7 @@ class DB2Storage : public DB2StorageBase
     static_assert(std::is_standard_layout<T>::value, "T in DB2Storage must have standard layout.");
 
 public:
-    typedef DBStorageIterator<T> iterator;
+    using iterator = DBStorageIterator<T>;
 
     DB2Storage(char const* fileName, DB2LoadInfo const* loadInfo) : DB2StorageBase(fileName, loadInfo)
     {
@@ -86,7 +86,7 @@ public:
     }
 
     bool HasRecord(uint32 id) const override { return id < _indexTableSize && _indexTable.AsT[id] != nullptr; }
-    void WriteRecord(uint32 id, uint32 locale, ByteBuffer& buffer) const override
+    void WriteRecord(uint32 id, LocaleConstant locale, ByteBuffer& buffer) const override
     {
         WriteRecordData(reinterpret_cast<char const*>(AssertEntry(id)), locale, buffer);
     }
@@ -98,12 +98,12 @@ public:
     T const* operator[](uint32 id) const { return LookupEntry(id); }
 
     uint32 GetNumRows() const { return _indexTableSize; }
-    bool Load(std::string const& path, uint32 locale) override
+    bool Load(std::string const& path, LocaleConstant locale) override
     {
         return DB2StorageBase::Load(path, locale, _indexTable.AsChar);
     }
 
-    bool LoadStringsFrom(std::string const& path, uint32 locale) override
+    bool LoadStringsFrom(std::string const& path, LocaleConstant locale) override
     {
         return DB2StorageBase::LoadStringsFrom(path, locale, _indexTable.AsChar);
     }
@@ -113,7 +113,7 @@ public:
         DB2StorageBase::LoadFromDB(_indexTable.AsChar);
     }
 
-    void LoadStringsFromDB(uint32 locale) override
+    void LoadStringsFromDB(LocaleConstant locale) override
     {
         DB2StorageBase::LoadStringsFromDB(locale, _indexTable.AsChar);
     }