From 3caa86c9239ee0f88a9d5bc4a0bc655b37041307 Mon Sep 17 00:00:00 2001 From: Kosta Date: Mon, 1 Sep 2014 11:11:26 +0200 Subject: [PATCH 1/9] short string optimization Since the payload (the `Data` union) of the current implementation of `GenericValue` is `12 bytes` (32 bit) or `16 bytes` (64 bit) it could store `UTF8`-encoded strings up to `10` or `14` chars plus the `terminating zero` character plus the string length: ``` C++ struct ShortString { enum { MaxSize = sizeof(GenericValue::String) / sizeof(Ch) - sizeof(unsigned char) }; Ch str[MaxSize]; unsigned char length; }; // at most as many bytes as "String" above => 12 bytes in 32-bit mode, 16 bytes in 64-bit mode ``` This is achieved by introducing additional `kInlineStrFlag` and `kShortStringFlag` flags. When setting a new string value in `SetStringRaw(s, alloc)` it is first checked if the string is short enough to fit into the `inline string buffer` and if so the given source string will be copied into the new `ShortString` target instead of allocating additional memory for it. --- include/rapidjson/document.h | 48 +++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index ec7f8a6..edab98d 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -1247,12 +1247,12 @@ int z = a[0u].GetInt(); // This works too. //!@name String //@{ - const Ch* GetString() const { RAPIDJSON_ASSERT(IsString()); return data_.s.str; } + const Ch* GetString() const { RAPIDJSON_ASSERT(IsString()); return ((flags_ & kInlineStrFlag) ? data_.ss.str : data_.s.str); } //! Get the length of string. /*! Since rapidjson permits "\\u0000" in the json string, strlen(v.GetString()) may not equal to v.GetStringLength(). */ - SizeType GetStringLength() const { RAPIDJSON_ASSERT(IsString()); return data_.s.length; } + SizeType GetStringLength() const { RAPIDJSON_ASSERT(IsString()); return ((flags_ & kInlineStrFlag) ? data_.ss.length : data_.s.length); } //! Set this value as a string without copying source string. /*! This version has better performance with supplied length, and also support string containing null character. @@ -1320,7 +1320,7 @@ int z = a[0u].GetInt(); // This works too. if (!handler.StartObject()) return false; for (ConstMemberIterator m = MemberBegin(); m != MemberEnd(); ++m) { - if (!handler.String(m->name.data_.s.str, m->name.data_.s.length, (m->name.flags_ & kCopyFlag) != 0)) + if (!handler.String(m->name.GetString(), m->name.GetStringLength(), (m->name.flags_ & kCopyFlag) != 0)) return false; if (!m->value.Accept(handler)) return false; @@ -1336,7 +1336,7 @@ int z = a[0u].GetInt(); // This works too. return handler.EndArray(data_.a.size); case kStringType: - return handler.String(data_.s.str, data_.s.length, (flags_ & kCopyFlag) != 0); + return handler.String(data_.GetString(), data_.GetStringLength(), (flags_ & kCopyFlag) != 0); case kNumberType: if (IsInt()) return handler.Int(data_.n.i.i); @@ -1365,6 +1365,7 @@ private: kDoubleFlag = 0x4000, kStringFlag = 0x100000, kCopyFlag = 0x200000, + kInlineStrFlag = 0x400000, // Initial flags of different types. kNullFlag = kNullType, @@ -1378,6 +1379,7 @@ private: kNumberAnyFlag = kNumberType | kNumberFlag | kIntFlag | kInt64Flag | kUintFlag | kUint64Flag | kDoubleFlag, kConstStringFlag = kStringType | kStringFlag, kCopyStringFlag = kStringType | kStringFlag | kCopyFlag, + kShortStringFlag = kStringType | kStringFlag | kCopyFlag | kInlineStrFlag, kObjectFlag = kObjectType, kArrayFlag = kArrayType, @@ -1393,6 +1395,12 @@ private: unsigned hashcode; //!< reserved }; // 12 bytes in 32-bit mode, 16 bytes in 64-bit mode + struct ShortString { + enum { MaxSize = sizeof(String) / sizeof(Ch) - sizeof(unsigned char) }; + Ch str[MaxSize]; + unsigned char length; + }; // at most as many bytes as "String" above => 12 bytes in 32-bit mode, 16 bytes in 64-bit mode + // By using proper binary layout, retrieval of different integer types do not need conversions. union Number { #if RAPIDJSON_ENDIAN == RAPIDJSON_LITTLEENDIAN @@ -1433,6 +1441,7 @@ private: union Data { String s; + ShortString ss; Number n; Object o; Array a; @@ -1463,11 +1472,19 @@ private: //! Initialize this value as copy string with initial data, without calling destructor. void SetStringRaw(StringRefType s, Allocator& allocator) { - flags_ = kCopyStringFlag; - data_.s.str = (Ch *)allocator.Malloc((s.length + 1) * sizeof(Ch)); - data_.s.length = s.length; - memcpy(const_cast(data_.s.str), s, s.length * sizeof(Ch)); - const_cast(data_.s.str)[s.length] = '\0'; + Ch* str = NULL; + if(s.length < ShortString::MaxSize) { + flags_ = kShortStringFlag; + data_.ss.length = s.length; + str = data_.ss.str; + } else { + flags_ = kCopyStringFlag; + data_.s.length = s.length; + str = (Ch *)allocator.Malloc((s.length + 1) * sizeof(Ch)); + data_.s.str = str; + } + memcpy(str, s, s.length * sizeof(Ch)); + str[s.length] = '\0'; } //! Assignment without calling destructor @@ -1480,9 +1497,16 @@ private: bool StringEqual(const GenericValue& rhs) const { RAPIDJSON_ASSERT(IsString()); RAPIDJSON_ASSERT(rhs.IsString()); - return data_.s.length == rhs.data_.s.length && - (data_.s.str == rhs.data_.s.str // fast path for constant string - || memcmp(data_.s.str, rhs.data_.s.str, sizeof(Ch) * data_.s.length) == 0); + + const SizeType len1 = (flags_ == kShortStringFlag) ? data_.ss.length : data_.s.length; + const SizeType len2 = (rhs.flags_ == kShortStringFlag) ? rhs.data_.ss.length : rhs.data_.s.length; + if(len1 != len2) { return false; } + + const Ch* const str1 = (flags_ == kShortStringFlag) ? data_.ss.str : data_.s.str; + const Ch* const str2 = (rhs.flags_ == kShortStringFlag) ? rhs.data_.ss.str : rhs.data_.s.str; + if(str1 == str2) { return true; } // fast path for constant string + + return (memcmp(str1, str2, sizeof(Ch) * len1) == 0); } Data data_; From b92d0ebd1b8b96c11a50d88215a6ccd38013ee6f Mon Sep 17 00:00:00 2001 From: Kosta Date: Mon, 1 Sep 2014 11:15:52 +0200 Subject: [PATCH 2/9] code cleanup for `StringEqual()` Instead of replicating the functionality of `GetString()` and `GetStringLength()` in `StringEqual()` it now calls these methods instead. --- include/rapidjson/document.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index edab98d..20e4f4c 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -1498,12 +1498,12 @@ private: RAPIDJSON_ASSERT(IsString()); RAPIDJSON_ASSERT(rhs.IsString()); - const SizeType len1 = (flags_ == kShortStringFlag) ? data_.ss.length : data_.s.length; - const SizeType len2 = (rhs.flags_ == kShortStringFlag) ? rhs.data_.ss.length : rhs.data_.s.length; + const SizeType len1 = GetStringLength(); + const SizeType len2 = rhs.GetStringLength(); if(len1 != len2) { return false; } - const Ch* const str1 = (flags_ == kShortStringFlag) ? data_.ss.str : data_.s.str; - const Ch* const str2 = (rhs.flags_ == kShortStringFlag) ? rhs.data_.ss.str : rhs.data_.s.str; + const Ch* const str1 = GetString(); + const Ch* const str2 = rhs.GetString(); if(str1 == str2) { return true; } // fast path for constant string return (memcmp(str1, str2, sizeof(Ch) * len1) == 0); From d2a374b40c916127b3ce390c00bc126334e60982 Mon Sep 17 00:00:00 2001 From: Kosta Date: Mon, 1 Sep 2014 11:52:09 +0200 Subject: [PATCH 3/9] allow the short string optimization to store one more character The `ShortString` can represent zero-terminated strings up to `MaxSize` chars (excluding the terminating zero) and store a value to determine the length of the contained string in the last character `str[LenPos]` by storing `MaxSize - length` there. If the string to store has the maximal length of `MaxSize` (excluding the terminating zero) then `str[LenPos]` will store `0` and therefore act as the string terminator as well. For getting the string length back from that value just use `MaxSize - str[LenPos]`. This allows to store `11`-chars strings in 32-bit mode and `15`-chars strings in 64-bit mode inline (for `UTF8`-encoded strings). --- include/rapidjson/document.h | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index 20e4f4c..b33ea07 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -1252,7 +1252,7 @@ int z = a[0u].GetInt(); // This works too. //! Get the length of string. /*! Since rapidjson permits "\\u0000" in the json string, strlen(v.GetString()) may not equal to v.GetStringLength(). */ - SizeType GetStringLength() const { RAPIDJSON_ASSERT(IsString()); return ((flags_ & kInlineStrFlag) ? data_.ss.length : data_.s.length); } + SizeType GetStringLength() const { RAPIDJSON_ASSERT(IsString()); return ((flags_ & kInlineStrFlag) ? (data_.ss.GetLength()) : data_.s.length); } //! Set this value as a string without copying source string. /*! This version has better performance with supplied length, and also support string containing null character. @@ -1395,10 +1395,21 @@ private: unsigned hashcode; //!< reserved }; // 12 bytes in 32-bit mode, 16 bytes in 64-bit mode + // implementation detail: ShortString can represent zero-terminated strings up to MaxSize chars + // (excluding the terminating zero) and store a value to determine the length of the contained + // string in the last character str[LenPos] by storing "MaxSize - length" there. If the string + // to store has the maximal length of MaxSize then str[LenPos] will be 0 and therefore act as + // the string terminator as well. For getting the string length back from that value just use + // "MaxSize - str[LenPos]". + // This allows to store 11-chars strings in 32-bit mode and 15-chars strings in 64-bit mode + // inline (for `UTF8`-encoded strings). struct ShortString { - enum { MaxSize = sizeof(String) / sizeof(Ch) - sizeof(unsigned char) }; - Ch str[MaxSize]; - unsigned char length; + enum { MaxChars = sizeof(String) / sizeof(Ch), MaxSize = MaxChars - 1, LenPos = MaxSize }; + Ch str[MaxChars]; + + inline static bool Usable(SizeType len) { return (MaxSize >= len); } + inline void SetLength(SizeType len) { str[LenPos] = (Ch)(MaxSize - len); } + inline SizeType GetLength() const { return (SizeType)(MaxSize - str[LenPos]); } }; // at most as many bytes as "String" above => 12 bytes in 32-bit mode, 16 bytes in 64-bit mode // By using proper binary layout, retrieval of different integer types do not need conversions. @@ -1473,9 +1484,9 @@ private: //! Initialize this value as copy string with initial data, without calling destructor. void SetStringRaw(StringRefType s, Allocator& allocator) { Ch* str = NULL; - if(s.length < ShortString::MaxSize) { + if(ShortString::Usable(s.length)) { flags_ = kShortStringFlag; - data_.ss.length = s.length; + data_.ss.SetLength(s.length); str = data_.ss.str; } else { flags_ = kCopyStringFlag; From 697cf407c2312a4ab51ae10b5d7a9b3ea116debf Mon Sep 17 00:00:00 2001 From: Kosta Date: Mon, 1 Sep 2014 12:01:25 +0200 Subject: [PATCH 4/9] fixed a compiler error not caught by VS2012... --- include/rapidjson/document.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/rapidjson/document.h b/include/rapidjson/document.h index b33ea07..9c3d9ac 100644 --- a/include/rapidjson/document.h +++ b/include/rapidjson/document.h @@ -1336,7 +1336,7 @@ int z = a[0u].GetInt(); // This works too. return handler.EndArray(data_.a.size); case kStringType: - return handler.String(data_.GetString(), data_.GetStringLength(), (flags_ & kCopyFlag) != 0); + return handler.String(GetString(), GetStringLength(), (flags_ & kCopyFlag) != 0); case kNumberType: if (IsInt()) return handler.Int(data_.n.i.i); From 056d0dafe4854a8136d7dc17dfbb5e4551176ce9 Mon Sep 17 00:00:00 2001 From: Kosta Date: Mon, 1 Sep 2014 12:34:43 +0200 Subject: [PATCH 5/9] add unit test for testing edge cases of the `short string optimization` --- test/unittest/valuetest.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/unittest/valuetest.cpp b/test/unittest/valuetest.cpp index e165452..f5d06c3 100644 --- a/test/unittest/valuetest.cpp +++ b/test/unittest/valuetest.cpp @@ -1084,3 +1084,26 @@ TEST(Document, CrtAllocator) { V a(kArrayType); a.PushBack(1, allocator); // Should not call destructor on uninitialized Value of newly allocated elements. } + +static void TestShortStringOptimization(const char* str) { + const int len = (int)strlen(str); + + rapidjson::Document doc; + rapidjson::Document::AllocatorType& allocator = doc.GetAllocator(); + rapidjson::Value objVal(rapidjson::kObjectType); + + objVal.AddMember(str, str, allocator); + EXPECT_TRUE(objVal.HasMember(str)); + + const rapidjson::Value& member = objVal[str]; + EXPECT_EQ(member.GetStringLength(), strlen(str)); +} + +TEST(Value, AllocateShortString) { + TestShortStringOptimization(""); // edge case: empty string + TestShortStringOptimization("12345678"); // regular case for short strings: 8 chars + TestShortStringOptimization("12345678901"); // edge case: 11 chars in 32-bit mode (=> short string) + TestShortStringOptimization("123456789012"); // edge case: 12 chars in 32-bit mode (=> regular string) + TestShortStringOptimization("123456789012345"); // edge case: 15 chars in 64-bit mode (=> short string) + TestShortStringOptimization("1234567890123456"); // edge case: 16 chars in 64-bit mode (=> regular string) +} From 88debcf02edadd8ad7c0a2a8b566bb40414541d0 Mon Sep 17 00:00:00 2001 From: Kosta Date: Mon, 1 Sep 2014 12:40:28 +0200 Subject: [PATCH 6/9] typo fixed for the unit test implementation --- test/unittest/valuetest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unittest/valuetest.cpp b/test/unittest/valuetest.cpp index f5d06c3..8e3fe8f 100644 --- a/test/unittest/valuetest.cpp +++ b/test/unittest/valuetest.cpp @@ -1092,7 +1092,7 @@ static void TestShortStringOptimization(const char* str) { rapidjson::Document::AllocatorType& allocator = doc.GetAllocator(); rapidjson::Value objVal(rapidjson::kObjectType); - objVal.AddMember(str, str, allocator); + objVal.AddMember(str, len, allocator); EXPECT_TRUE(objVal.HasMember(str)); const rapidjson::Value& member = objVal[str]; From 609997565ca355cb0abad60542293e47beebac03 Mon Sep 17 00:00:00 2001 From: Kosta Date: Mon, 1 Sep 2014 12:46:04 +0200 Subject: [PATCH 7/9] unit test simplification for `short string optimization` --- test/unittest/valuetest.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/test/unittest/valuetest.cpp b/test/unittest/valuetest.cpp index 8e3fe8f..0df5f34 100644 --- a/test/unittest/valuetest.cpp +++ b/test/unittest/valuetest.cpp @@ -1089,14 +1089,11 @@ static void TestShortStringOptimization(const char* str) { const int len = (int)strlen(str); rapidjson::Document doc; - rapidjson::Document::AllocatorType& allocator = doc.GetAllocator(); - rapidjson::Value objVal(rapidjson::kObjectType); + rapidjson::Value val; + val.SetString(str, len, doc.GetAllocator()); - objVal.AddMember(str, len, allocator); - EXPECT_TRUE(objVal.HasMember(str)); - - const rapidjson::Value& member = objVal[str]; - EXPECT_EQ(member.GetStringLength(), strlen(str)); + EXPECT_EQ(val.GetStringLength(), len); + EXPECT_STREQ(val.GetString(), str); } TEST(Value, AllocateShortString) { From ba05ea52cf3930143de8aee681519f186dc58aaa Mon Sep 17 00:00:00 2001 From: Kosta Date: Mon, 1 Sep 2014 12:52:36 +0200 Subject: [PATCH 8/9] use `rapidjson::Value::SizeType` as the type for storing and comparing the string length --- test/unittest/valuetest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unittest/valuetest.cpp b/test/unittest/valuetest.cpp index 0df5f34..1f698e7 100644 --- a/test/unittest/valuetest.cpp +++ b/test/unittest/valuetest.cpp @@ -1086,7 +1086,7 @@ TEST(Document, CrtAllocator) { } static void TestShortStringOptimization(const char* str) { - const int len = (int)strlen(str); + const rapidjson::Value::SizeType len = (rapidjson::Value::SizeType)strlen(str); rapidjson::Document doc; rapidjson::Value val; From 08e81097eb0debb5b1f3b7159401ba69f67a30af Mon Sep 17 00:00:00 2001 From: Kosta Date: Mon, 1 Sep 2014 12:54:50 +0200 Subject: [PATCH 9/9] final fix for the unit test case... --- test/unittest/valuetest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unittest/valuetest.cpp b/test/unittest/valuetest.cpp index 1f698e7..7cc81dc 100644 --- a/test/unittest/valuetest.cpp +++ b/test/unittest/valuetest.cpp @@ -1086,7 +1086,7 @@ TEST(Document, CrtAllocator) { } static void TestShortStringOptimization(const char* str) { - const rapidjson::Value::SizeType len = (rapidjson::Value::SizeType)strlen(str); + const rapidjson::SizeType len = (rapidjson::SizeType)strlen(str); rapidjson::Document doc; rapidjson::Value val;