From: Amir Shalem <amirshalem@fb.com>
Date: Thu, 22 Dec 2016 00:47:55 +0000 (-0800)
Subject: FBString: remove unnecessary 7-byte padding in large strings
X-Git-Tag: v2017.03.06.00~173
X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=d83fd99b038fff6cea7bffd4729e86e971f5502e;p=folly.git

FBString: remove unnecessary 7-byte padding in large strings

Summary:
RefCounted struct contains a pointer to `Char data_[1]`
This saved us a +1 when calculating sizes for the null terminator,
but the compiler made the struct size to be 16, instead of a 8+1.

Reviewed By: Gownta, ot

Differential Revision: D4356429

fbshipit-source-id: 420694feb4b367b0c73d44f83c21a9559ac5e7a3
---

diff --git a/folly/FBString.h b/folly/FBString.h
index 351b0de0..b486bf33 100644
--- a/folly/FBString.h
+++ b/folly/FBString.h
@@ -20,6 +20,7 @@
 #pragma once
 
 #include <atomic>
+#include <cstddef>
 #include <limits>
 #include <type_traits>
 
@@ -527,11 +528,14 @@ private:
     std::atomic<size_t> refCount_;
     Char data_[1];
 
+    constexpr static size_t getDataOffset() {
+      return offsetof(RefCounted, data_);
+    }
+
     static RefCounted * fromData(Char * p) {
-      return static_cast<RefCounted*>(
-        static_cast<void*>(
-          static_cast<unsigned char*>(static_cast<void*>(p))
-          - sizeof(refCount_)));
+      return static_cast<RefCounted*>(static_cast<void*>(
+          static_cast<unsigned char*>(static_cast<void*>(p)) -
+          getDataOffset()));
     }
 
     static size_t refs(Char * p) {
@@ -552,14 +556,11 @@ private:
     }
 
     static RefCounted * create(size_t * size) {
-      // Don't forget to allocate one extra Char for the terminating
-      // null. In this case, however, one Char is already part of the
-      // struct.
-      const size_t allocSize = goodMallocSize(
-        sizeof(RefCounted) + *size * sizeof(Char));
+      const size_t allocSize =
+          goodMallocSize(getDataOffset() + (*size + 1) * sizeof(Char));
       auto result = static_cast<RefCounted*>(checkedMalloc(allocSize));
       result->refCount_.store(1, std::memory_order_release);
-      *size = (allocSize - sizeof(RefCounted)) / sizeof(Char);
+      *size = (allocSize - getDataOffset()) / sizeof(Char) - 1;
       return result;
     }
 
@@ -577,20 +578,17 @@ private:
                                    const size_t currentCapacity,
                                    size_t * newCapacity) {
       FBSTRING_ASSERT(*newCapacity > 0 && *newCapacity > currentSize);
-      const size_t allocNewCapacity = goodMallocSize(
-        sizeof(RefCounted) + *newCapacity * sizeof(Char));
+      const size_t allocNewCapacity =
+          goodMallocSize(getDataOffset() + (*newCapacity + 1) * sizeof(Char));
       auto const dis = fromData(data);
       FBSTRING_ASSERT(dis->refCount_.load(std::memory_order_acquire) == 1);
-      // Don't forget to allocate one extra Char for the terminating
-      // null. In this case, however, one Char is already part of the
-      // struct.
-      auto result = static_cast<RefCounted*>(
-             smartRealloc(dis,
-                          sizeof(RefCounted) + currentSize * sizeof(Char),
-                          sizeof(RefCounted) + currentCapacity * sizeof(Char),
-                          allocNewCapacity));
+      auto result = static_cast<RefCounted*>(smartRealloc(
+          dis,
+          getDataOffset() + (currentSize + 1) * sizeof(Char),
+          getDataOffset() + (currentCapacity + 1) * sizeof(Char),
+          allocNewCapacity));
       FBSTRING_ASSERT(result->refCount_.load(std::memory_order_acquire) == 1);
-      *newCapacity = (allocNewCapacity - sizeof(RefCounted)) / sizeof(Char);
+      *newCapacity = (allocNewCapacity - getDataOffset()) / sizeof(Char) - 1;
       return result;
     }
   };
diff --git a/folly/test/FBStringTest.cpp b/folly/test/FBStringTest.cpp
index 6c6f099c..fba17f68 100644
--- a/folly/test/FBStringTest.cpp
+++ b/folly/test/FBStringTest.cpp
@@ -1277,9 +1277,10 @@ TEST(FBString, testFixedBugs) {
 
     struct {
       std::atomic<size_t> refCount_;
-      char data_[1];
     } dummyRefCounted;
-    EXPECT_EQ(str.capacity(), goodMallocSize(3840) - sizeof(dummyRefCounted));
+    EXPECT_EQ(
+        str.capacity(),
+        goodMallocSize(3840) - sizeof(dummyRefCounted) - sizeof(char));
   }
 }