Skip to content

Commit

Permalink
Use struct for building mapbuffer buckets
Browse files Browse the repository at this point in the history 
Summary:
Instead of copying integer values into key/value data array, this implementation keeps a structured vector of `Bucket` structs (which is sized to be serialized later).

This approach provides multiple benefits to modify and access buckets (e.g. sort them based on the key, allowing for out of order inserts (D33611758))

Changelog: [Internal]

Reviewed By: javache

Differential Revision: D33601231

fbshipit-source-id: 62ace1374936cb504836d6eae672e909ea404e3f
  • Loading branch information
@pull
Andrei Shikov authored and pull[bot] committed Aug 17, 2022
1 parent 44e9df5 commit 8961513
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 95 deletions.
82 changes: 12 additions & 70 deletions ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,38 +12,18 @@ using namespace facebook::react;
namespace facebook {
namespace react {

// TODO T83483191: Add asserts to check overflowing on additions
MapBufferBuilder::MapBufferBuilder()
: MapBufferBuilder::MapBufferBuilder(INITIAL_KEY_VALUE_SIZE) {}

MapBuffer MapBufferBuilder::EMPTY() {
return MapBufferBuilder().build();
}

MapBufferBuilder::MapBufferBuilder(uint16_t initialSize) {
keyValuesSize_ = initialSize;
keyValues_ = new Byte[keyValuesSize_];
// First Key should be written right after the header.
keyValuesOffset_ = HEADER_SIZE;
MapBufferBuilder::MapBufferBuilder(uint32_t initialSize) {
buckets_.reserve(initialSize);

dynamicDataSize_ = 0;
dynamicDataValues_ = nullptr;
dynamicDataOffset_ = 0;
}

void MapBufferBuilder::ensureKeyValueSpace() {
int32_t oldKeyValuesSize = keyValuesSize_;
react_native_assert(
(keyValuesSize_ < std::numeric_limits<uint16_t>::max() / 2) &&
"Error trying to assign a value beyond the capacity of uint16_t: ");
keyValuesSize_ *= 2;
uint8_t *newKeyValues = new Byte[keyValuesSize_];
uint8_t *oldKeyValues = keyValues_;
memcpy(newKeyValues, keyValues_, oldKeyValuesSize);
keyValues_ = newKeyValues;
delete[] oldKeyValues;
}

void MapBufferBuilder::storeKeyValue(
Key key,
uint8_t *value,
Expand All @@ -58,24 +38,15 @@ void MapBufferBuilder::storeKeyValue(
abort();
}

// TODO T83483191: header.count points to the next index
// TODO T83483191: add test to verify storage of sparse keys
int32_t keyOffset = getKeyOffset(_header.count);
int32_t valueOffset = keyOffset + KEY_SIZE;

int32_t nextKeyValueOffset = keyOffset + BUCKET_SIZE;
if (nextKeyValueOffset >= keyValuesSize_) {
ensureKeyValueSpace();
}
uint64_t data = 0;
auto *dataPtr = reinterpret_cast<uint8_t *>(&data);
memcpy(dataPtr, value, valueSize);

memcpy(keyValues_ + keyOffset, &key, KEY_SIZE);
memcpy(keyValues_ + valueOffset, value, valueSize);
buckets_.emplace_back(key, data);

_header.count++;

minKeyToStore_ = key + 1;
// Move keyValuesOffset_ to the next available [key, value] position
keyValuesOffset_ = std::max(nextKeyValueOffset, keyValuesOffset_);
}

void MapBufferBuilder::putBool(Key key, bool value) {
Expand Down Expand Up @@ -168,56 +139,27 @@ void MapBufferBuilder::putMapBuffer(Key key, MapBuffer const &map) {
}

MapBuffer MapBufferBuilder::build() {
react_native_assert(
(keyValues_ != nullptr) &&
"Error when building mapbuffer with invalid datastructures.");

// Create buffer: [header] + [key, values] + [dynamic data]
int32_t bufferSize = keyValuesOffset_ + dynamicDataOffset_;
auto bucketSize = buckets_.size() * BUCKET_SIZE;
int32_t bufferSize = HEADER_SIZE + bucketSize + dynamicDataOffset_;

_header.bufferSize = bufferSize;

// Copy header at the beginning of "keyValues_"
memcpy(keyValues_, &_header, HEADER_SIZE);

std::vector<uint8_t> buffer(bufferSize);

memcpy(buffer.data(), keyValues_, keyValuesOffset_);
memcpy(buffer.data(), &_header, HEADER_SIZE);
memcpy(buffer.data() + HEADER_SIZE, buckets_.data(), bucketSize);

if (dynamicDataValues_ != nullptr) {
memcpy(
buffer.data() + keyValuesOffset_,
buffer.data() + HEADER_SIZE + bucketSize,
dynamicDataValues_,
dynamicDataOffset_);
}

auto map = MapBuffer(std::move(buffer));

// TODO T83483191: we should invalidate the class once the build() method is
// called.

if (keyValues_ != nullptr) {
delete[] keyValues_;
}
keyValues_ = nullptr;
keyValuesSize_ = 0;
keyValuesOffset_ = 0;

if (dynamicDataValues_ != nullptr) {
delete[] dynamicDataValues_;
dynamicDataValues_ = nullptr;
}
dynamicDataSize_ = 0;
dynamicDataOffset_ = 0;
_header = {ALIGNMENT, 0, 0};

return map;
return MapBuffer(std::move(buffer));
}

MapBufferBuilder::~MapBufferBuilder() {
if (keyValues_ != nullptr) {
delete[] keyValues_;
}
if (dynamicDataValues_ != nullptr) {
delete[] dynamicDataValues_;
}
Expand Down
30 changes: 5 additions & 25 deletions ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,13 @@
#include <react/debug/react_native_assert.h>
#include <react/renderer/mapbuffer/MapBuffer.h>
#include <react/renderer/mapbuffer/primitives.h>
#include <stdlib.h>
#include <cstdlib>

namespace facebook {
namespace react {

// Default initial size for _keyValues array
// 108 = 10 entries = 10*10 + 8 sizeof(header)
constexpr uint16_t INITIAL_KEY_VALUE_SIZE = 108;

// Default initial size for _dynamicDataValues array
constexpr int32_t INITIAL_DYNAMIC_DATA_SIZE = 200;
// Default reserved size for buckets_ vector
constexpr uint16_t INITIAL_BUCKETS_SIZE = 10;

/**
* MapBufferBuilder is a builder class for MapBuffer
Expand All @@ -29,25 +25,11 @@ class MapBufferBuilder {
private:
Header _header = {ALIGNMENT, 0, 0};

void ensureKeyValueSpace();

void ensureDynamicDataSpace(int32_t size);

void storeKeyValue(Key key, uint8_t *value, int32_t valueSize);

// Array of [key,value] map entries:
// - Key is represented in 2 bytes
// - Value is stored into 8 bytes. The 8 bytes of the value will contain the
// actual value for the key or a pointer to the actual value (based on the
// type)
uint8_t *keyValues_ = nullptr;

// Amount of bytes allocated on _keyValues
uint16_t keyValuesSize_ = 0;

// Relative offset on the _keyValues array.
// This represents the first byte that can be written in _keyValues array
int32_t keyValuesOffset_ = 0;
std::vector<Bucket> buckets_{};

// This array contains data for dynamic values in the MapBuffer.
// A dynamic value is a String or another MapBuffer.
Expand All @@ -66,9 +48,7 @@ class MapBufferBuilder {
uint16_t minKeyToStore_ = 0;

public:
MapBufferBuilder();

MapBufferBuilder(uint16_t initialSize);
MapBufferBuilder(uint32_t initialSize = INITIAL_BUCKETS_SIZE);

~MapBufferBuilder();

Expand Down
11 changes: 11 additions & 0 deletions ReactCommon/react/renderer/mapbuffer/primitives.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ struct Header {
int32_t bufferSize; // Amount of bytes used to store the map in memory
};

static_assert(sizeof(Header) == 8, "MapBuffer header size is incorrect.");

struct __attribute__((__packed__)) Bucket {
Key key;
uint64_t data;

Bucket(Key key, uint64_t data) : key(key), data(data){};
};

static_assert(sizeof(Bucket) == 10, "MapBuffer bucket size is incorrect.");

constexpr static int32_t KEY_SIZE = sizeof(Key);
constexpr static int32_t HEADER_SIZE = sizeof(Header);
constexpr static int32_t INT_SIZE = sizeof(int32_t);
Expand Down

0 comments on commit 8961513

Please sign in to comment.