Commit d83fd99b authored by Amir Shalem's avatar Amir Shalem Committed by Facebook Github Bot

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
parent b9e76007
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#pragma once #pragma once
#include <atomic> #include <atomic>
#include <cstddef>
#include <limits> #include <limits>
#include <type_traits> #include <type_traits>
...@@ -527,11 +528,14 @@ private: ...@@ -527,11 +528,14 @@ private:
std::atomic<size_t> refCount_; std::atomic<size_t> refCount_;
Char data_[1]; Char data_[1];
constexpr static size_t getDataOffset() {
return offsetof(RefCounted, data_);
}
static RefCounted * fromData(Char * p) { static RefCounted * fromData(Char * p) {
return static_cast<RefCounted*>( return static_cast<RefCounted*>(static_cast<void*>(
static_cast<void*>( static_cast<unsigned char*>(static_cast<void*>(p)) -
static_cast<unsigned char*>(static_cast<void*>(p)) getDataOffset()));
- sizeof(refCount_)));
} }
static size_t refs(Char * p) { static size_t refs(Char * p) {
...@@ -552,14 +556,11 @@ private: ...@@ -552,14 +556,11 @@ private:
} }
static RefCounted * create(size_t * size) { static RefCounted * create(size_t * size) {
// Don't forget to allocate one extra Char for the terminating const size_t allocSize =
// null. In this case, however, one Char is already part of the goodMallocSize(getDataOffset() + (*size + 1) * sizeof(Char));
// struct.
const size_t allocSize = goodMallocSize(
sizeof(RefCounted) + *size * sizeof(Char));
auto result = static_cast<RefCounted*>(checkedMalloc(allocSize)); auto result = static_cast<RefCounted*>(checkedMalloc(allocSize));
result->refCount_.store(1, std::memory_order_release); result->refCount_.store(1, std::memory_order_release);
*size = (allocSize - sizeof(RefCounted)) / sizeof(Char); *size = (allocSize - getDataOffset()) / sizeof(Char) - 1;
return result; return result;
} }
...@@ -577,20 +578,17 @@ private: ...@@ -577,20 +578,17 @@ private:
const size_t currentCapacity, const size_t currentCapacity,
size_t * newCapacity) { size_t * newCapacity) {
FBSTRING_ASSERT(*newCapacity > 0 && *newCapacity > currentSize); FBSTRING_ASSERT(*newCapacity > 0 && *newCapacity > currentSize);
const size_t allocNewCapacity = goodMallocSize( const size_t allocNewCapacity =
sizeof(RefCounted) + *newCapacity * sizeof(Char)); goodMallocSize(getDataOffset() + (*newCapacity + 1) * sizeof(Char));
auto const dis = fromData(data); auto const dis = fromData(data);
FBSTRING_ASSERT(dis->refCount_.load(std::memory_order_acquire) == 1); FBSTRING_ASSERT(dis->refCount_.load(std::memory_order_acquire) == 1);
// Don't forget to allocate one extra Char for the terminating auto result = static_cast<RefCounted*>(smartRealloc(
// null. In this case, however, one Char is already part of the dis,
// struct. getDataOffset() + (currentSize + 1) * sizeof(Char),
auto result = static_cast<RefCounted*>( getDataOffset() + (currentCapacity + 1) * sizeof(Char),
smartRealloc(dis, allocNewCapacity));
sizeof(RefCounted) + currentSize * sizeof(Char),
sizeof(RefCounted) + currentCapacity * sizeof(Char),
allocNewCapacity));
FBSTRING_ASSERT(result->refCount_.load(std::memory_order_acquire) == 1); FBSTRING_ASSERT(result->refCount_.load(std::memory_order_acquire) == 1);
*newCapacity = (allocNewCapacity - sizeof(RefCounted)) / sizeof(Char); *newCapacity = (allocNewCapacity - getDataOffset()) / sizeof(Char) - 1;
return result; return result;
} }
}; };
......
...@@ -1277,9 +1277,10 @@ TEST(FBString, testFixedBugs) { ...@@ -1277,9 +1277,10 @@ TEST(FBString, testFixedBugs) {
struct { struct {
std::atomic<size_t> refCount_; std::atomic<size_t> refCount_;
char data_[1];
} dummyRefCounted; } dummyRefCounted;
EXPECT_EQ(str.capacity(), goodMallocSize(3840) - sizeof(dummyRefCounted)); EXPECT_EQ(
str.capacity(),
goodMallocSize(3840) - sizeof(dummyRefCounted) - sizeof(char));
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment