Use vector for dynamic data in MapBufferBuilder
Summary: Replaces dynamic manually managed array with a vector of bytes, removing the need for managing memory manually. This approach is a little bit slower than before, as vector is allocating memory more conservatively. This can be improved in the future by providing an estimate for the data size. Changelog: [Internal] Reviewed By: javache Differential Revision: D33611759 fbshipit-source-id: a0e5e57c4e413206a9f891cd5630ecc9088a9bf7
This commit is contained in:
Andrei Shikov
Facebook GitHub Bot
Родитель
b1ef4405ee
Коммит
5b489d5f95
|
|
@ -15,7 +15,6 @@ namespace react {
|
|||
// TODO T83483191: Extend MapBuffer C++ implementation to support basic random
|
||||
// access
|
||||
MapBuffer::MapBuffer(std::vector<uint8_t> data) : bytes_(std::move(data)) {
|
||||
count_ = 0;
|
||||
memcpy(
|
||||
reinterpret_cast<uint8_t *>(&count_),
|
||||
bytes_.data() + HEADER_COUNT_OFFSET,
|
||||
|
|
@ -23,7 +22,7 @@ MapBuffer::MapBuffer(std::vector<uint8_t> data) : bytes_(std::move(data)) {
|
|||
|
||||
// TODO T83483191: extract memcpy calls into an inline function to simplify
|
||||
// the code
|
||||
int32_t dataSize;
|
||||
uint32_t dataSize;
|
||||
memcpy(
|
||||
reinterpret_cast<uint8_t *>(&dataSize),
|
||||
bytes_.data() + HEADER_BUFFER_SIZE_OFFSET,
|
||||
|
|
@ -113,23 +112,16 @@ bool MapBuffer::isNull(Key key) const {
|
|||
return getInt(key) == NULL_VALUE;
|
||||
}
|
||||
|
||||
int32_t MapBuffer::getBufferSize() const {
|
||||
uint32_t MapBuffer::size() const {
|
||||
return bytes_.size();
|
||||
}
|
||||
|
||||
void MapBuffer::copy(uint8_t *output) const {
|
||||
memcpy(output, bytes_.data(), bytes_.size());
|
||||
uint8_t const *MapBuffer::data() const {
|
||||
return bytes_.data();
|
||||
}
|
||||
|
||||
uint16_t MapBuffer::getCount() const {
|
||||
uint16_t size = 0;
|
||||
|
||||
memcpy(
|
||||
reinterpret_cast<uint8_t *>(&size),
|
||||
bytes_.data() + HEADER_COUNT_OFFSET,
|
||||
UINT16_SIZE);
|
||||
|
||||
return size;
|
||||
uint16_t MapBuffer::count() const {
|
||||
return count_;
|
||||
}
|
||||
|
||||
} // namespace react
|
||||
|
|
|
|||
|
|
@ -65,14 +65,13 @@ class MapBuffer {
|
|||
// TODO T83483191: review this declaration
|
||||
MapBuffer getMapBuffer(Key key) const;
|
||||
|
||||
int32_t getBufferSize() const;
|
||||
|
||||
// TODO T83483191: review parameters of copy method
|
||||
void copy(uint8_t *output) const;
|
||||
|
||||
bool isNull(Key key) const;
|
||||
|
||||
uint16_t getCount() const;
|
||||
uint32_t size() const;
|
||||
|
||||
uint8_t const *data() const;
|
||||
|
||||
uint16_t count() const;
|
||||
};
|
||||
|
||||
} // namespace react
|
||||
|
|
|
|||
|
|
@ -13,21 +13,17 @@ namespace facebook {
|
|||
namespace react {
|
||||
|
||||
MapBuffer MapBufferBuilder::EMPTY() {
|
||||
return MapBufferBuilder().build();
|
||||
return MapBufferBuilder(0).build();
|
||||
}
|
||||
|
||||
MapBufferBuilder::MapBufferBuilder(uint32_t initialSize) {
|
||||
buckets_.reserve(initialSize);
|
||||
|
||||
dynamicDataSize_ = 0;
|
||||
dynamicDataValues_ = nullptr;
|
||||
dynamicDataOffset_ = 0;
|
||||
}
|
||||
|
||||
void MapBufferBuilder::storeKeyValue(
|
||||
Key key,
|
||||
uint8_t *value,
|
||||
int32_t valueSize) {
|
||||
uint8_t const *value,
|
||||
uint32_t valueSize) {
|
||||
if (key < minKeyToStore_) {
|
||||
LOG(ERROR) << "Error: key out of order - key: " << key;
|
||||
abort();
|
||||
|
|
@ -54,7 +50,7 @@ void MapBufferBuilder::putBool(Key key, bool value) {
|
|||
}
|
||||
|
||||
void MapBufferBuilder::putDouble(Key key, double value) {
|
||||
uint8_t *bytePointer = reinterpret_cast<uint8_t *>(&value);
|
||||
auto const *bytePointer = reinterpret_cast<uint8_t *>(&value);
|
||||
storeKeyValue(key, bytePointer, DOUBLE_SIZE);
|
||||
}
|
||||
|
||||
|
|
@ -63,107 +59,58 @@ void MapBufferBuilder::putNull(Key key) {
|
|||
}
|
||||
|
||||
void MapBufferBuilder::putInt(Key key, int32_t value) {
|
||||
uint8_t *bytePointer = reinterpret_cast<uint8_t *>(&(value));
|
||||
auto const *bytePointer = reinterpret_cast<uint8_t *>(&(value));
|
||||
storeKeyValue(key, bytePointer, INT_SIZE);
|
||||
}
|
||||
|
||||
void MapBufferBuilder::ensureDynamicDataSpace(int32_t size) {
|
||||
if (dynamicDataValues_ == nullptr) {
|
||||
dynamicDataSize_ = size;
|
||||
dynamicDataValues_ = new Byte[dynamicDataSize_];
|
||||
dynamicDataOffset_ = 0;
|
||||
return;
|
||||
}
|
||||
|
||||
if (dynamicDataOffset_ + size >= dynamicDataSize_) {
|
||||
int32_t oldDynamicDataSize = dynamicDataSize_;
|
||||
react_native_assert(
|
||||
(dynamicDataSize_ < std::numeric_limits<int32_t>::max() / 2) &&
|
||||
"Error: trying to assign a value beyond the capacity of int");
|
||||
dynamicDataSize_ *= dynamicDataSize_;
|
||||
|
||||
react_native_assert(
|
||||
(dynamicDataSize_ < std::numeric_limits<int32_t>::max() - size) &&
|
||||
"Error: trying to assign a value beyond the capacity of int");
|
||||
|
||||
// sum size to ensure that the size always fit into newDynamicDataValues
|
||||
dynamicDataSize_ += size;
|
||||
uint8_t *newDynamicDataValues = new Byte[dynamicDataSize_];
|
||||
uint8_t *oldDynamicDataValues = dynamicDataValues_;
|
||||
memcpy(newDynamicDataValues, dynamicDataValues_, oldDynamicDataSize);
|
||||
dynamicDataValues_ = newDynamicDataValues;
|
||||
delete[] oldDynamicDataValues;
|
||||
}
|
||||
}
|
||||
|
||||
void MapBufferBuilder::putString(Key key, std::string const &value) {
|
||||
int32_t strLength = static_cast<int32_t>(value.length());
|
||||
const char *cstring = getCstring(&value);
|
||||
int32_t strSize = value.size();
|
||||
const char *strData = value.data();
|
||||
|
||||
// format [lenght of string (int)] + [Array of Characters in the string]
|
||||
int32_t sizeOfLength = INT_SIZE;
|
||||
// TODO T83483191: review if map.getBufferSize() should be an int32_t or long
|
||||
auto offset = dynamicData_.size();
|
||||
// format [length of string (int)] + [Array of Characters in the string]
|
||||
// TODO T83483191: review if map.size() should be an int32_t or long
|
||||
// instead of an int16 (because strings can be longer than int16);
|
||||
|
||||
int32_t sizeOfDynamicData = sizeOfLength + strLength;
|
||||
ensureDynamicDataSpace(sizeOfDynamicData);
|
||||
memcpy(dynamicDataValues_ + dynamicDataOffset_, &strLength, sizeOfLength);
|
||||
memcpy(
|
||||
dynamicDataValues_ + dynamicDataOffset_ + sizeOfLength,
|
||||
cstring,
|
||||
strLength);
|
||||
dynamicData_.resize(offset + INT_SIZE + strSize, 0);
|
||||
memcpy(dynamicData_.data() + offset, &strSize, INT_SIZE);
|
||||
memcpy(dynamicData_.data() + offset + INT_SIZE, strData, strSize);
|
||||
|
||||
// Store Key and pointer to the string
|
||||
putInt(key, dynamicDataOffset_);
|
||||
|
||||
dynamicDataOffset_ += sizeOfDynamicData;
|
||||
putInt(key, offset);
|
||||
}
|
||||
|
||||
void MapBufferBuilder::putMapBuffer(Key key, MapBuffer const &map) {
|
||||
int32_t mapBufferSize = map.getBufferSize();
|
||||
int32_t mapBufferSize = map.size();
|
||||
|
||||
// format [lenght of buffer (int)] + [bytes of MapBuffer]
|
||||
int32_t sizeOfDynamicData = mapBufferSize + INT_SIZE;
|
||||
auto offset = dynamicData_.size();
|
||||
|
||||
// format [Array of bytes of the mapBuffer]
|
||||
ensureDynamicDataSpace(sizeOfDynamicData);
|
||||
|
||||
memcpy(dynamicDataValues_ + dynamicDataOffset_, &mapBufferSize, INT_SIZE);
|
||||
// Copy the content of the map into dynamicDataValues_
|
||||
map.copy(dynamicDataValues_ + dynamicDataOffset_ + INT_SIZE);
|
||||
// format [length of buffer (int)] + [bytes of MapBuffer]
|
||||
dynamicData_.resize(offset + INT_SIZE + mapBufferSize, 0);
|
||||
memcpy(dynamicData_.data() + offset, &mapBufferSize, INT_SIZE);
|
||||
// Copy the content of the map into dynamicData_
|
||||
memcpy(dynamicData_.data() + offset + INT_SIZE, map.data(), mapBufferSize);
|
||||
|
||||
// Store Key and pointer to the string
|
||||
putInt(key, dynamicDataOffset_);
|
||||
|
||||
dynamicDataOffset_ += sizeOfDynamicData;
|
||||
putInt(key, offset);
|
||||
}
|
||||
|
||||
MapBuffer MapBufferBuilder::build() {
|
||||
// Create buffer: [header] + [key, values] + [dynamic data]
|
||||
auto bucketSize = buckets_.size() * BUCKET_SIZE;
|
||||
int32_t bufferSize = HEADER_SIZE + bucketSize + dynamicDataOffset_;
|
||||
uint32_t bufferSize = HEADER_SIZE + bucketSize + dynamicData_.size();
|
||||
|
||||
_header.bufferSize = bufferSize;
|
||||
|
||||
std::vector<uint8_t> buffer(bufferSize);
|
||||
memcpy(buffer.data(), &_header, HEADER_SIZE);
|
||||
memcpy(buffer.data() + HEADER_SIZE, buckets_.data(), bucketSize);
|
||||
|
||||
if (dynamicDataValues_ != nullptr) {
|
||||
memcpy(
|
||||
buffer.data() + HEADER_SIZE + bucketSize,
|
||||
dynamicDataValues_,
|
||||
dynamicDataOffset_);
|
||||
}
|
||||
memcpy(
|
||||
buffer.data() + HEADER_SIZE + bucketSize,
|
||||
dynamicData_.data(),
|
||||
dynamicData_.size());
|
||||
|
||||
return MapBuffer(std::move(buffer));
|
||||
}
|
||||
|
||||
MapBufferBuilder::~MapBufferBuilder() {
|
||||
if (dynamicDataValues_ != nullptr) {
|
||||
delete[] dynamicDataValues_;
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace react
|
||||
} // namespace facebook
|
||||
|
|
|
|||
|
|
@ -25,23 +25,11 @@ class MapBufferBuilder {
|
|||
private:
|
||||
Header _header = {ALIGNMENT, 0, 0};
|
||||
|
||||
void ensureDynamicDataSpace(int32_t size);
|
||||
|
||||
void storeKeyValue(Key key, uint8_t *value, int32_t valueSize);
|
||||
void storeKeyValue(Key key, uint8_t const *value, uint32_t valueSize);
|
||||
|
||||
std::vector<Bucket> buckets_{};
|
||||
|
||||
// This array contains data for dynamic values in the MapBuffer.
|
||||
// A dynamic value is a String or another MapBuffer.
|
||||
uint8_t *dynamicDataValues_ = nullptr;
|
||||
|
||||
// Amount of bytes allocated on _dynamicDataValues
|
||||
int32_t dynamicDataSize_ = 0;
|
||||
|
||||
// Relative offset on the _dynamicDataValues array.
|
||||
// This represents the first byte that can be written in _dynamicDataValues
|
||||
// array
|
||||
int32_t dynamicDataOffset_ = 0;
|
||||
std::vector<Byte> dynamicData_{};
|
||||
|
||||
// Minimmum key to store in the MapBuffer (this is used to guarantee
|
||||
// consistency)
|
||||
|
|
@ -50,8 +38,6 @@ class MapBufferBuilder {
|
|||
public:
|
||||
MapBufferBuilder(uint32_t initialSize = INITIAL_BUCKETS_SIZE);
|
||||
|
||||
~MapBufferBuilder();
|
||||
|
||||
static MapBuffer EMPTY();
|
||||
|
||||
void putInt(Key key, int32_t value);
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ namespace react {
|
|||
struct Header {
|
||||
uint16_t alignment; // alignment of serialization
|
||||
uint16_t count; // amount of items in the map
|
||||
int32_t bufferSize; // Amount of bytes used to store the map in memory
|
||||
uint32_t bufferSize; // Amount of bytes used to store the map in memory
|
||||
};
|
||||
|
||||
static_assert(sizeof(Header) == 8, "MapBuffer header size is incorrect.");
|
||||
|
|
@ -48,9 +48,10 @@ constexpr static int32_t DOUBLE_SIZE = sizeof(double);
|
|||
constexpr static int32_t UINT8_SIZE = sizeof(uint8_t);
|
||||
constexpr static int32_t UINT16_SIZE = sizeof(uint16_t);
|
||||
constexpr static int32_t UINT64_SIZE = sizeof(uint64_t);
|
||||
constexpr static int32_t HEADER_ALIGNMENT_OFFSET = 0;
|
||||
constexpr static int32_t HEADER_COUNT_OFFSET = UINT16_SIZE;
|
||||
constexpr static int32_t HEADER_BUFFER_SIZE_OFFSET = UINT16_SIZE * 2;
|
||||
constexpr static int32_t HEADER_ALIGNMENT_OFFSET = offsetof(Header, alignment);
|
||||
constexpr static int32_t HEADER_COUNT_OFFSET = offsetof(Header, count);
|
||||
constexpr static int32_t HEADER_BUFFER_SIZE_OFFSET =
|
||||
offsetof(Header, bufferSize);
|
||||
|
||||
constexpr static int32_t MAX_VALUE_SIZE = UINT64_SIZE;
|
||||
|
||||
|
|
@ -71,10 +72,6 @@ inline int32_t getValueOffset(Key key) {
|
|||
return getKeyOffset(key) + KEY_SIZE;
|
||||
}
|
||||
|
||||
static inline const char *getCstring(const std::string *str) {
|
||||
return str ? str->c_str() : "";
|
||||
}
|
||||
|
||||
inline void
|
||||
checkKeyConsistency(const Header &header, const uint8_t *data, Key key) {
|
||||
#ifdef CHECK_CONSISTENCY
|
||||
|
|
|
|||
|
|
@ -21,7 +21,7 @@ TEST(MapBufferTest, testSimpleIntMap) {
|
|||
|
||||
auto map = builder.build();
|
||||
|
||||
EXPECT_EQ(map.getCount(), 2);
|
||||
EXPECT_EQ(map.count(), 2);
|
||||
EXPECT_EQ(map.getInt(0), 1234);
|
||||
EXPECT_EQ(map.getInt(1), 4321);
|
||||
}
|
||||
|
|
@ -38,7 +38,7 @@ TEST(MapBufferTest, testMapBufferExtension) {
|
|||
|
||||
auto map = buffer.build();
|
||||
|
||||
EXPECT_EQ(map.getCount(), 4);
|
||||
EXPECT_EQ(map.count(), 4);
|
||||
|
||||
EXPECT_EQ(map.getInt(0), 1234);
|
||||
EXPECT_EQ(map.getInt(1), 4321);
|
||||
|
|
@ -54,7 +54,7 @@ TEST(MapBufferTest, testBoolEntries) {
|
|||
|
||||
auto map = buffer.build();
|
||||
|
||||
EXPECT_EQ(map.getCount(), 2);
|
||||
EXPECT_EQ(map.count(), 2);
|
||||
EXPECT_EQ(map.getBool(0), true);
|
||||
EXPECT_EQ(map.getBool(1), false);
|
||||
}
|
||||
|
|
@ -67,7 +67,7 @@ TEST(MapBufferTest, testNullEntries) {
|
|||
|
||||
auto map = buffer.build();
|
||||
|
||||
EXPECT_EQ(map.getCount(), 2);
|
||||
EXPECT_EQ(map.count(), 2);
|
||||
EXPECT_EQ(map.isNull(0), true);
|
||||
EXPECT_EQ(map.isNull(1), false);
|
||||
// TODO T83483191: serialize null values to be distinguishable from '0'
|
||||
|
|
@ -84,7 +84,7 @@ TEST(MapBufferTest, testDoubleEntries) {
|
|||
|
||||
auto map = buffer.build();
|
||||
|
||||
EXPECT_EQ(map.getCount(), 2);
|
||||
EXPECT_EQ(map.count(), 2);
|
||||
|
||||
EXPECT_EQ(map.getDouble(0), 123.4);
|
||||
EXPECT_EQ(map.getDouble(1), 432.1);
|
||||
|
|
@ -134,12 +134,12 @@ TEST(MapBufferTest, testUTFStringEntries) {
|
|||
TEST(MapBufferTest, testEmptyMap) {
|
||||
auto builder = MapBufferBuilder();
|
||||
auto map = builder.build();
|
||||
EXPECT_EQ(map.getCount(), 0);
|
||||
EXPECT_EQ(map.count(), 0);
|
||||
}
|
||||
|
||||
TEST(MapBufferTest, testEmptyMapConstant) {
|
||||
auto map = MapBufferBuilder::EMPTY();
|
||||
EXPECT_EQ(map.getCount(), 0);
|
||||
EXPECT_EQ(map.count(), 0);
|
||||
}
|
||||
|
||||
TEST(MapBufferTest, testMapEntries) {
|
||||
|
|
@ -148,7 +148,7 @@ TEST(MapBufferTest, testMapEntries) {
|
|||
builder.putInt(1, 1234);
|
||||
auto map = builder.build();
|
||||
|
||||
EXPECT_EQ(map.getCount(), 2);
|
||||
EXPECT_EQ(map.count(), 2);
|
||||
EXPECT_EQ(map.getString(0), "This is a test");
|
||||
EXPECT_EQ(map.getInt(1), 1234);
|
||||
|
||||
|
|
@ -157,12 +157,12 @@ TEST(MapBufferTest, testMapEntries) {
|
|||
builder2.putMapBuffer(1, map);
|
||||
auto map2 = builder2.build();
|
||||
|
||||
EXPECT_EQ(map2.getCount(), 2);
|
||||
EXPECT_EQ(map2.count(), 2);
|
||||
EXPECT_EQ(map2.getInt(0), 4321);
|
||||
|
||||
MapBuffer readMap2 = map2.getMapBuffer(1);
|
||||
|
||||
EXPECT_EQ(readMap2.getCount(), 2);
|
||||
EXPECT_EQ(readMap2.count(), 2);
|
||||
EXPECT_EQ(readMap2.getString(0), "This is a test");
|
||||
EXPECT_EQ(readMap2.getInt(1), 1234);
|
||||
}
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче