Skip to content

Commit 54de1cd

Browse files
MuniuDevMuniuDev
committed
Improving strings
1 parent 2650e10 commit 54de1cd

File tree

9 files changed

+92
-78
lines changed

9 files changed

+92
-78
lines changed

‎PolyEngine/Core/Src/pe/core/storage/IndexedString.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,23 @@
66
namespace pe::core::storage
77
{
88

9-
IndexedString::IndexedString(const char* str)
9+
IndexedString::IndexedString(std::string_view str)
1010
: m_entry(impl::IndexedStringManager::get().registerString(str))
1111
{
1212
ASSERTE(m_entry, "Entry is null after string creation!");
1313
}
1414

15+
IndexedString::IndexedString(const impl::IndexedStringEntry* entry)
16+
: m_entry(entry)
17+
{
18+
ASSERTE(m_entry, "Entry is null after string creation!");
19+
}
20+
21+
IndexedString IndexedString::FromRString(core::storage::String&& str)
22+
{
23+
return IndexedString(impl::IndexedStringManager::get().registerString(std::move(str)));
24+
}
25+
1526
IndexedString::~IndexedString()
1627
{
1728
impl::IndexedStringManager::get().unregisterString(m_entry);

‎PolyEngine/Core/Src/pe/core/storage/IndexedString.hpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,12 @@ class CORE_DLLEXPORT IndexedString final : public core::BaseObjectLiteralType<>
2222
public:
2323
/// @brief IndexedString constructor. Registers the string in the IndexedStringManager.
2424
/// @param[in] str String to be represented by the IndexedString instance.
25-
explicit IndexedString(const char* str);
25+
explicit IndexedString(std::string_view str);
26+
27+
/// @brief IndexedString factory function. Registers the string in the IndexedStringManager.
28+
/// @note If the string is not registered yet, the memory is reused.
29+
/// @param[in] str String to be represented by the IndexedString instance.
30+
static IndexedString FromRString(core::storage::String&& str);
2631

2732
/// @brief IndexedString destructor. Unregisters the string from the IndexedStringManager.
2833
~IndexedString();
@@ -59,6 +64,8 @@ class CORE_DLLEXPORT IndexedString final : public core::BaseObjectLiteralType<>
5964

6065
friend std::ostream& operator<< (std::ostream& stream, const IndexedString& rhs) { return stream << rhs.get(); }
6166
private:
67+
explicit IndexedString(const impl::IndexedStringEntry* entry);
68+
6269
const impl::IndexedStringEntry* m_entry = nullptr;
6370

6471
friend struct std::hash<::pe::core::storage::IndexedString>;

‎PolyEngine/Core/Src/pe/core/storage/String.cpp

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,10 @@ const String String::EMPTY = String();
99

1010
static const std::vector<char> WHITESPACES { ' ', '\t', '\r', '\n', '\0' };
1111

12-
namespace pe::core::storage
13-
{
14-
15-
size_t StrLen(const char* str) {
16-
size_t len = 0;
17-
while (str[len] != 0)
18-
++len;
19-
return len;
20-
}
21-
22-
}
23-
24-
String::String(const char* data) {
25-
size_t length = StrLen(data);
12+
String::String(std::string_view str) {
13+
size_t length = str.length();
2614
Data.resize(length + 1);
27-
std::memcpy(Data.data(), data, sizeof(char) * length);
15+
std::memcpy(Data.data(), str.data(), sizeof(char) * length);
2816
Data[length] = 0;
2917
}
3018

@@ -42,8 +30,7 @@ String String::From(float var, size_t precision) { return StringBuilder().Append
4230
String String::From(double var) { return StringBuilder().Append(var).StealString(); }
4331
String String::From(double var, size_t precision) { return StringBuilder().Append(var, precision).StealString(); }
4432
String String::From(char var) { return StringBuilder().Append(var).StealString(); }
45-
String String::From(const char* var) { return StringBuilder().Append(var).StealString(); }
46-
String String::From(const std::string& var) { return StringBuilder().Append(var).StealString(); }
33+
String String::From(std::string_view var) { return StringBuilder().Append(var).StealString(); }
4734

4835
bool String::Contains(const String& var) const
4936
{
@@ -202,30 +189,26 @@ String& String::operator=(String&& rhs) {
202189
return *this;
203190
}
204191

205-
bool String::operator==(const char* str) const {
206-
if (GetLength() != StrLen(str))
192+
bool String::operator==(std::string_view str) const {
193+
if (GetLength() != str.length())
207194
return false;
208195
for (size_t k = 0; k < GetLength(); ++k)
209196
if (Data[k] != str[k])
210197
return false;
211198
return true;
212199
}
213200

214-
bool String::operator==(const String& str) const {
215-
return Data == str.Data;
216-
}
217-
218-
bool String::operator<(const String& rhs) const {
219-
if (GetLength() < rhs.GetLength())
201+
bool String::operator<(std::string_view rhs) const {
202+
if (GetLength() < rhs.length())
220203
return true;
221-
else if (GetLength() > rhs.GetLength())
204+
else if (GetLength() > rhs.length())
222205
return false;
223206

224207
for (size_t i = 0; i < GetLength(); ++i)
225208
{
226-
if (Data[i] < rhs.Data[i])
209+
if (Data[i] < rhs[i])
227210
return true;
228-
else if (Data[i] > rhs.Data[i])
211+
else if (Data[i] > rhs[i])
229212
return false;
230213
}
231214
return false;

‎PolyEngine/Core/Src/pe/core/storage/String.hpp

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,6 @@
44

55
namespace pe::core::storage
66
{
7-
8-
CORE_DLLEXPORT size_t StrLen(const char* str);
9-
107
class CORE_DLLEXPORT String final : public BaseObjectLiteralType<>
118
{
129
public:
@@ -17,17 +14,20 @@ namespace pe::core::storage
1714

1815
/// <summary>String constructor that creates String based on provided Cstring</summary>
1916
/// <param name="data"></param>
20-
String(const char* data);
17+
String(const char* str) : String(std::string_view(str)) {}
18+
19+
/// <summary>String constructor that creates String based on provided string_view</summary>
20+
/// <param name="data"></param>
21+
explicit String(std::string_view str);
2122

2223
/// <summary>String copy constructor</summary>
2324
/// <param name="rhs">Reference to String instance which state should be copied</param>
24-
String(const String& rhs);
25+
explicit String(const String& rhs);
2526

2627
/// <summary>String move constructor</summary>
2728
/// <param name="rhs">Reference to String instance which state should be moved</param>
2829
String(String&& rhs);
2930

30-
3131
/// <summary>Casts int to String</summary>
3232
/// <param name="var">Integer value which should be used to make String instance</param>
3333
/// <returns>String containing integer value</returns>
@@ -62,15 +62,10 @@ namespace pe::core::storage
6262
/// <returns>String conaining only one char</returns>
6363
static String From(char var);
6464

65-
/// <summary>Casts Cstring to String</summary>
66-
/// <param name="var">Cstring value which should be used to make String instance</param>
67-
/// <returns>String containing given Cstring</returns>
68-
static String From(const char* var);
69-
70-
/// <summary>Casts std::string to String</summary>
71-
/// <param name="var">std::string reference which should be used to make String instance</param>
72-
/// <returns>String containing given std::string</returns>
73-
static String From(const std::string& var);
65+
/// <summary>Casts string_view to String</summary>
66+
/// <param name="var">string_view value which should be used to make String instance</param>
67+
/// <returns>String containing given content</returns>
68+
static String From(std::string_view str);
7469

7570

7671
/// <summary>Checks if String instance contains another String instance</summary>
@@ -164,18 +159,12 @@ namespace pe::core::storage
164159
/// <returns>Moved String reference</returns>
165160
String& operator=(String&& rhs);
166161

167-
/// <summary>Compares String with Cstring</summary>
168-
/// <param name="str">Cstring to be compared with</param>
169-
bool operator==(const char* str) const;
170-
171-
/// <summary>Compares two String references</summary>
172-
/// <param name="str">String to be compared with</param>
173-
bool operator==(const String& str) const;
174-
175-
bool operator!=(const char* str) const { return !(*this == str); }
176-
bool operator!=(const String& str) const { return !(*this == str); }
162+
/// <summary>Compares String with string_view</summary>
163+
/// <param name="str">string_view to be compared with</param>
164+
bool operator==(std::string_view str) const;
165+
bool operator!=(std::string_view str) const { return !(*this == str); }
177166

178-
bool operator<(const String& rhs) const;
167+
bool operator<(std::string_view rhs) const;
179168

180169
/// <summary>Appends one String to another</summary>
181170
/// <param name="rhs">String instance to be appended to source String</param>
@@ -197,6 +186,7 @@ namespace pe::core::storage
197186

198187
friend std::ostream& operator<< (std::ostream& stream, const String& rhs) { return stream << rhs.GetCStr(); }
199188

189+
operator std::string_view() const { return std::string_view(GetCStr()); }
200190
private:
201191

202192
String(std::vector<char> rawData) : Data(std::move(rawData)) { Data.push_back('\0'); }

‎PolyEngine/Core/Src/pe/core/storage/StringBuilder.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
using namespace pe::core::storage;
66

77
//----------------------------------------------------------------------------
8-
StringBuilder& StringBuilder::Append(const char* str, const size_t length)
8+
StringBuilder& StringBuilder::Append(const char* str, size_t length)
99
{
1010
Buffer.reserve(Buffer.size() + length);
1111
for (size_t i = 0; i < length; ++i)

‎PolyEngine/Core/Src/pe/core/storage/StringBuilder.hpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,15 @@ namespace pe::core::storage
1616
/// @param val Value to append
1717
/// @return Instance reference for chainlinking
1818
inline StringBuilder& Append(char val) { Buffer.push_back(val); return *this; }
19-
inline StringBuilder& Append(const char* val) { return Append(val, strlen(val)); }
20-
inline StringBuilder& Append(const std::string& val) { return Append(val.c_str(), val.length()); }
21-
inline StringBuilder& Append(const String& val) { return Append(val.GetCStr(), val.GetLength()); }
2219
inline StringBuilder& Append(int val) { FillBufferWithFormat(val, "%d"); return *this; }
2320
inline StringBuilder& Append(long long val) { FillBufferWithFormat(val, "%lld"); return *this; }
2421
inline StringBuilder& Append(unsigned val) { FillBufferWithFormat(val, "%u"); return *this; }
2522
inline StringBuilder& Append(unsigned long long val) { FillBufferWithFormat(val, "%llu"); return *this; }
2623
inline StringBuilder& Append(long val) { return Append((long long)val); }
2724
inline StringBuilder& Append(unsigned long val) { return Append((unsigned long long)val); }
25+
inline StringBuilder& Append(std::string_view str) { return Append(str.data(), str.length()); }
2826

29-
StringBuilder& Append(const char* str, const size_t length);
27+
StringBuilder& Append(const char* str, size_t length);
3028
StringBuilder& Append(f32 val, size_t precission = DEFAULT_FLT_PRECISION);
3129
StringBuilder& Append(f64 val, size_t precission = DEFAULT_FLT_PRECISION);
3230

‎PolyEngine/Core/Src/pe/core/storage/impl/IndexedStringEntry.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ class IndexedStringEntry final : public core::BaseObjectLiteralType<>
1212
{
1313
public:
1414
IndexedStringEntry(const char* str) : m_str(str) {}
15+
IndexedStringEntry(core::storage::String&& str) : m_str(std::move(str)) {}
1516

1617
void incrementRefCount() const { ++m_refCount; }
1718
void decrementRefCount() const { --m_refCount; }

‎PolyEngine/Core/Src/pe/core/storage/impl/IndexedStringManager.cpp

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,32 +11,52 @@ IndexedStringManager& IndexedStringManager::get()
1111
return instance;
1212
}
1313

14-
const IndexedStringEntry* IndexedStringManager::registerString(const char* str)
14+
const IndexedStringEntry* IndexedStringManager::registerString(std::string_view str)
1515
{
16-
IndexedStringEntry* ret = nullptr;
16+
auto it = m_entries.find(str);
17+
if (it == m_entries.end())
18+
{
19+
return shareEntry(createEntry(String(str)));
20+
}
21+
else
22+
{
23+
return shareEntry(it->second.get());
24+
}
25+
}
1726

27+
const IndexedStringEntry* IndexedStringManager::registerString(core::storage::String&& str)
28+
{
1829
auto it = m_entries.find(str);
1930
if (it == m_entries.end())
2031
{
21-
// @todo(muniu) There are two allocations happening here.
22-
// Consider fixing it if the performace is affected by this.
23-
auto entry = std::make_unique<IndexedStringEntry>(str);
24-
ret = entry.get();
25-
// We need to create a new string_view, which points to the string with the same lifetime as entry.
26-
// Otherwise we could have some memory issues.
27-
std::string_view strView(entry.get()->get().GetCStr());
28-
m_entries.emplace(strView, std::move(entry));
32+
return shareEntry(createEntry(std::move(str)));
2933
}
3034
else
3135
{
32-
ret = it->second.get();
36+
return shareEntry(it->second.get());
3337
}
34-
35-
ret->incrementRefCount();
36-
ret->resetRemovalTimePoint();
38+
}
39+
40+
const IndexedStringEntry* IndexedStringManager::createEntry(core::storage::String&& str)
41+
{
42+
IndexedStringEntry* ret = nullptr;
43+
auto entry = std::make_unique<IndexedStringEntry>(std::move(str));
44+
ret = entry.get();
45+
// We need to create a new string_view, which points to the string with the same lifetime as entry.
46+
// Otherwise we could have some memory issues.
47+
std::string_view strView(entry.get()->get().GetCStr());
48+
m_entries.emplace(strView, std::move(entry));
49+
3750
return ret;
3851
}
3952

53+
const IndexedStringEntry* IndexedStringManager::shareEntry(const IndexedStringEntry* entry)
54+
{
55+
entry->incrementRefCount();
56+
entry->resetRemovalTimePoint();
57+
return entry;
58+
}
59+
4060
void IndexedStringManager::unregisterString(const IndexedStringEntry* entry)
4161
{
4262
entry->decrementRefCount();
@@ -75,7 +95,7 @@ void IndexedStringManager::setTTLMode(bool enabled)
7595
m_ttlEnabled = enabled;
7696
}
7797

78-
bool IndexedStringManager::isRegistered(const char* str) const
98+
bool IndexedStringManager::isRegistered(std::string_view str) const
7999
{
80100
auto it = m_entries.find(str);
81101
return it != m_entries.end();

‎PolyEngine/Core/Src/pe/core/storage/impl/IndexedStringManager.hpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,14 @@ class CORE_DLLEXPORT IndexedStringManager final : public core::BaseObjectLiteral
2121
public:
2222
static IndexedStringManager& get();
2323

24-
const IndexedStringEntry* registerString(const char* str);
24+
const IndexedStringEntry* registerString(std::string_view str);
25+
const IndexedStringEntry* registerString(core::storage::String&& str);
2526
void unregisterString(const IndexedStringEntry* entry);
2627

2728
void setTTLMode(bool enabled);
2829
void tickTTL(size_t ttlTickCount = 1);
2930

30-
bool isRegistered(const char* str) const;
31+
bool isRegistered(std::string_view str) const;
3132
private:
3233
IndexedStringManager() = default;
3334

@@ -40,6 +41,9 @@ class CORE_DLLEXPORT IndexedStringManager final : public core::BaseObjectLiteral
4041
void erase(const IndexedStringEntry* entry);
4142
void scheduleErase(const IndexedStringEntry* entry);
4243

44+
const IndexedStringEntry* createEntry(core::storage::String&& str);
45+
const IndexedStringEntry* shareEntry(const IndexedStringEntry* entry);
46+
4347
std::unordered_map<std::string_view, std::unique_ptr<IndexedStringEntry>> m_entries;
4448
PriorityQueue<TTLEntry> m_ttlEntries;
4549
bool m_ttlEnabled = false;

0 commit comments

Comments
 (0)