From f63b080574f6a139c952fe2a0c06f16fdf170042 Mon Sep 17 00:00:00 2001 From: Giuseppe Ottaviano Date: Sun, 4 Oct 2015 21:22:12 -0700 Subject: [PATCH] Improve performance of fbstring copy and move ctors on small strings Summary: The move constructor of `fbstring` generates a large amount of code, preventing it to be inlined, and it is inefficient for small strings. This diff fixes both problems. Reviewed By: @yfeldblum Differential Revision: D2505762 --- folly/FBString.h | 55 ++++++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/folly/FBString.h b/folly/FBString.h index 3af49b67..a9618aa4 100644 --- a/folly/FBString.h +++ b/folly/FBString.h @@ -290,16 +290,7 @@ protected: static_assert( kIsLittleEndian || kIsBigEndian, "unable to identify endianness"); public: - fbstring_core() noexcept { - // Only initialize the tag, will set the MSBs (i.e. the small - // string size) to zero too - ml_.capacity_ = kIsLittleEndian - ? maxSmallSize << (8 * (sizeof(size_t) - sizeof(Char))) - : ml_.capacity_ = maxSmallSize << 2; - // or: setSmallSize(0); - writeTerminator(); - assert(category() == Category::isSmall && size() == 0); - } + fbstring_core() noexcept { reset(); } fbstring_core(const fbstring_core & rhs) { assert(&rhs != this); @@ -311,18 +302,12 @@ public: "fbstring layout failure"); static_assert(offsetof(MediumLarge, capacity_) == 2 * sizeof(ml_.data_), "fbstring layout failure"); - const size_t size = rhs.smallSize(); - if (size == 0) { - ml_.capacity_ = rhs.ml_.capacity_; - writeTerminator(); - } else { - // Just write the whole thing, don't look at details. In - // particular we need to copy capacity anyway because we want - // to set the size (don't forget that the last character, - // which stores a short string's length, is shared with the - // ml_.capacity field). - ml_ = rhs.ml_; - } + // Just write the whole thing, don't look at details. In + // particular we need to copy capacity anyway because we want + // to set the size (don't forget that the last character, + // which stores a short string's length, is shared with the + // ml_.capacity field). + ml_ = rhs.ml_; assert(category() == Category::isSmall && this->size() == rhs.size()); } else if (rhs.category() == Category::isLarge) { // Large strings are just refcounted @@ -350,14 +335,11 @@ public: } fbstring_core(fbstring_core&& goner) noexcept { - if (goner.category() == Category::isSmall) { - // Just copy, leave the goner in peace - new(this) fbstring_core(goner.small_, goner.smallSize()); - } else { - // Take goner's guts - ml_ = goner.ml_; + // Take goner's guts + ml_ = goner.ml_; + if (goner.category() != Category::isSmall) { // Clean goner's carcass - goner.setSmallSize(0); + goner.reset(); } } @@ -463,7 +445,7 @@ public: } else { // No need for the memory free(data); - setSmallSize(0); + reset(); } } @@ -723,6 +705,19 @@ private: // Disabled fbstring_core & operator=(const fbstring_core & rhs); + // Equivalent to setSmallSize(0), but with specialized + // writeTerminator which doesn't re-check the category after + // capacity_ is overwritten. + void reset() { + // Only initialize the tag, will set the MSBs (i.e. the small + // string size) to zero too. + ml_.capacity_ = kIsLittleEndian + ? maxSmallSize << (8 * (sizeof(size_t) - sizeof(Char))) + : maxSmallSize << 2; + small_[0] = '\0'; + assert(category() == Category::isSmall && size() == 0); + } + struct RefCounted { std::atomic refCount_; Char data_[1]; -- 2.34.1