-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
short string optimization #131
short string optimization #131
Conversation
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.
Instead of replicating the functionality of `GetString()` and `GetStringLength()` in `StringEqual()` it now calls these methods instead.
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).
And by reworking the |
@@ -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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
len
is unused?
} else { | ||
flags_ = kCopyStringFlag; | ||
data_.s.length = s.length; | ||
str = (Ch *)allocator.Malloc((s.length + 1) * sizeof(Ch)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems necessary to handle short string in ~GenericValue()
, as copy-string is malloc here but short-string does not need to. Shall generate crash in unit test when using CrtAllocator
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, in line 1488 I use the new flag kShortStringFlag
and the dtor checks for kCopyStringFlag
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right.
sorry for the failing unit tests spam; I haven't setup |
LGTM 👍 . It would be good to explicitly test the implementation for other encodings, especially those with bigger character types ( |
Is it then possible to represent a zero-length string and still compute a correct length without checking if |
@drewnoakes yes, a zero-length string stores |
@Kosta-Github -- that's a neat trick which I didn't grok on the first read. Nice. So the 'length' can really be thought of as the padding length, rather than the string length. |
I tried the nativejson-benchmark on a 32-bit Linux machine (using Clang 3.6) and obtained the following preliminary results:
So it seems to be a small but promising optimization for RapidJSON. When using the |
…tion short string optimization
Since the payload (the
Data
union) of the current implementation ofGenericValue
is12 bytes
(32 bit mode) or16 bytes
(64 bit mode) it could storeUTF8
-encoded strings up to11
or15
chars plus theterminating zero
character plus the actual string length.The
ShortString
can represent zero-terminated strings up toMaxSize
chars (excluding the terminating zero) and store a value to determine the length of the contained string in the last characterstr[LenPos]
by storingMaxSize - length
there. If the string to store has the maximal length ofMaxSize
(excluding the terminating zero) thenstr[LenPos]
will store0
and therefore act as the string terminator as well. For getting the string length back from that value just useMaxSize - str[LenPos]
.This allows to store
11
-chars strings in 32-bit mode and15
-chars strings in 64-bit mode inline (forUTF8
-encoded strings).The integration into
GenericValue
is done by introducing additionalkInlineStrFlag
andkShortStringFlag
flags. When setting a new string value inSetStringRaw(s, alloc)
it is first checked if the string is short enough to fit into theinline string buffer
and if so the given source string will be copied into the newShortString
target instead of allocating additional memory for it.