diff options
author | Florin Malita <fmalita@chromium.org> | 2018-06-14 18:03:26 -0400 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2018-06-15 12:27:04 +0000 |
commit | d7bfcaf57c1fe1d84880e0d31fab9c3d303cdb79 (patch) | |
tree | 298fe73fcf9402a0212ed66e5c3f3b6bb3a05add /modules | |
parent | 3f13bcba3e5c8234ddd0c36e7758513bb08a72e4 (diff) |
[skjson] Some short string optimizations
1) skip redundant \0 terminator => Value is always zero-initialized
2) skip storing a len record => strlen is cheap for short strings
Change-Id: I3c10c9b9cf6155b95124e2c0194c59e9531a7ca4
Reviewed-on: https://skia-review.googlesource.com/135049
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Diffstat (limited to 'modules')
-rw-r--r-- | modules/skjson/include/SkJSON.h | 19 | ||||
-rw-r--r-- | modules/skjson/src/SkJSON.cpp | 23 |
2 files changed, 17 insertions, 25 deletions
diff --git a/modules/skjson/include/SkJSON.h b/modules/skjson/include/SkJSON.h index 1b46c4a100..8baf9fb8fc 100644 --- a/modules/skjson/include/SkJSON.h +++ b/modules/skjson/include/SkJSON.h @@ -12,6 +12,8 @@ #include "SkTo.h" #include "SkTypes.h" +#include <cstring> + class SkString; class SkWStream; @@ -282,7 +284,11 @@ public: size_t size() const { switch (this->getTag()) { case Tag::kShortString: - return kMaxInlineStringSize - SkToSizeT(this->cast<char>()[kMaxInlineStringSize]); + // We don't bother storing a length for short strings on the assumption + // that strlen is fast in this case. If this becomes problematic, we + // can either go back to storing (7-len) in the tag byte or write a fast + // short_strlen. + return strlen(this->cast<char>()); case Tag::kString: return this->cast<VectorValue<char, Value::Type::kString>>()->size(); default: @@ -297,15 +303,10 @@ public: } const char* end() const { - if (this->getTag() == Tag::kShortString) { - const auto* payload = this->cast<char>(); - return payload + kMaxInlineStringSize - SkToSizeT(payload[kMaxInlineStringSize]); - } - return this->cast<VectorValue<char, Value::Type::kString>>()->end(); + return this->getTag() == Tag::kShortString + ? strchr(this->cast<char>(), '\0') + : this->cast<VectorValue<char, Value::Type::kString>>()->end(); } - -private: - static constexpr size_t kMaxInlineStringSize = sizeof(Value) - 1; }; struct Member { diff --git a/modules/skjson/src/SkJSON.cpp b/modules/skjson/src/SkJSON.cpp index d6585e098c..7f082d7294 100644 --- a/modules/skjson/src/SkJSON.cpp +++ b/modules/skjson/src/SkJSON.cpp @@ -7,6 +7,7 @@ #include "SkJSON.h" +#include "SkMalloc.h" #include "SkStream.h" #include "SkString.h" @@ -82,12 +83,9 @@ static void* MakeVector(const void* src, size_t size, SkArenaAlloc& alloc) { // The Ts are already in memory, so their size should be safe. const auto total_size = sizeof(size_t) + size * sizeof(T) + extra_alloc_size; auto* size_ptr = reinterpret_cast<size_t*>(alloc.makeBytesAlignedTo(total_size, kRecAlign)); - *size_ptr = size; - if (size) { - auto* data_ptr = reinterpret_cast<void*>(size_ptr + 1); - memcpy(data_ptr, src, size * sizeof(T)); - } + *size_ptr = size; + sk_careful_memcpy(size_ptr + 1, src, size * sizeof(T)); return size_ptr; } @@ -112,6 +110,7 @@ ArrayValue::ArrayValue(const Value* src, size_t size, SkArenaAlloc& alloc) { // The string data plus a null-char terminator are copied over. // StringValue::StringValue(const char* src, size_t size, SkArenaAlloc& alloc) { + static constexpr size_t kMaxInlineStringSize = sizeof(Value) - 1; if (size > kMaxInlineStringSize) { this->init_tagged_pointer(Tag::kString, MakeVector<char, 1>(src, size, alloc)); @@ -122,19 +121,11 @@ StringValue::StringValue(const char* src, size_t size, SkArenaAlloc& alloc) { } this->init_tagged(Tag::kShortString); + sk_careful_memcpy(this->cast<char>(), src, size); - auto* payload = this->cast<char>(); - if (size) { - memcpy(payload, src, size); - payload[size] = '\0'; - } - - const auto len_tag = SkTo<char>(kMaxInlineStringSize - size); - // This technically overwrites the tag, but is safe because - // 1) kShortString == 0 - // 2) 0 <= len_tag <= 7 + // Null terminator provided by init_tagged() above (fData8 is zero-initialized). + // This is safe because kShortString is also 0 and can act as a terminator when size == 7. static_assert(static_cast<uint8_t>(Tag::kShortString) == 0, "please don't break this"); - payload[kMaxInlineStringSize] = len_tag; SkASSERT(this->getTag() == Tag::kShortString); } |