diff --git a/ReactCommon/react/renderer/mapbuffer/MapBuffer.cpp b/ReactCommon/react/renderer/mapbuffer/MapBuffer.cpp index ce7319f3ddb..74b55070227 100644 --- a/ReactCommon/react/renderer/mapbuffer/MapBuffer.cpp +++ b/ReactCommon/react/renderer/mapbuffer/MapBuffer.cpp @@ -12,6 +12,14 @@ using namespace facebook::react; namespace facebook { namespace react { +static inline int32_t bucketOffset(int32_t index) { + return HEADER_SIZE + BUCKET_SIZE * index; +} + +static inline int32_t valueOffset(int32_t bucketIndex) { + return bucketOffset(bucketIndex) + offsetof(Bucket, data); +} + // TODO T83483191: Extend MapBuffer C++ implementation to support basic random // access MapBuffer::MapBuffer(std::vector data) : bytes_(std::move(data)) { @@ -25,13 +33,33 @@ MapBuffer::MapBuffer(std::vector data) : bytes_(std::move(data)) { } } +uint32_t MapBuffer::getKeyBucket(Key key) const { + uint32_t lo = 0; + uint32_t hi = count_ - 1; + while (lo <= hi) { + uint32_t mid = (lo + hi) >> 1; + + Key midVal = + *reinterpret_cast(bytes_.data() + bucketOffset(mid)); + + if (midVal < key) { + lo = mid + 1; + } else if (midVal > key) { + hi = mid - 1; + } else { + return mid; + } + } + + return -1; +} + int32_t MapBuffer::getInt(Key key) const { - int32_t value = 0; - memcpy( - reinterpret_cast(&value), - bytes_.data() + getValueOffset(key), - INT_SIZE); - return value; + auto bucketIndex = getKeyBucket(key); + react_native_assert(bucketIndex != -1 && "Key not found in MapBuffer"); + + return *reinterpret_cast( + bytes_.data() + valueOffset(bucketIndex)); } bool MapBuffer::getBool(Key key) const { @@ -39,41 +67,31 @@ bool MapBuffer::getBool(Key key) const { } double MapBuffer::getDouble(Key key) const { - // TODO T83483191: extract this code into a "template method" and reuse it for - // other types - double value = 0; - memcpy( - reinterpret_cast(&value), - bytes_.data() + getValueOffset(key), - DOUBLE_SIZE); - return value; + auto bucketIndex = getKeyBucket(key); + react_native_assert(bucketIndex != -1 && "Key not found in MapBuffer"); + + return *reinterpret_cast( + bytes_.data() + valueOffset(bucketIndex)); + ; } int32_t MapBuffer::getDynamicDataOffset() const { - // The begininig of dynamic data can be calculated as the offset of the next + // The start of dynamic data can be calculated as the offset of the next // key in the map - return getKeyOffset(count_); + return bucketOffset(count_); } std::string MapBuffer::getString(Key key) const { // TODO T83483191:Add checks to verify that offsets are under the boundaries // of the map buffer int32_t dynamicDataOffset = getDynamicDataOffset(); - int32_t stringLength = 0; int32_t offset = getInt(key); - memcpy( - reinterpret_cast(&stringLength), - bytes_.data() + dynamicDataOffset + offset, - INT_SIZE); + int32_t stringLength = *reinterpret_cast( + bytes_.data() + dynamicDataOffset + offset); + uint8_t const *stringPtr = + bytes_.data() + dynamicDataOffset + offset + INT_SIZE; - char *value = new char[stringLength]; - - memcpy( - reinterpret_cast(value), - bytes_.data() + dynamicDataOffset + offset + INT_SIZE, - stringLength); - - return std::string(value, 0, stringLength); + return std::string(stringPtr, stringPtr + stringLength); } MapBuffer MapBuffer::getMapBuffer(Key key) const { @@ -81,12 +99,9 @@ MapBuffer MapBuffer::getMapBuffer(Key key) const { // of the map buffer int32_t dynamicDataOffset = getDynamicDataOffset(); - int32_t mapBufferLength = 0; int32_t offset = getInt(key); - memcpy( - reinterpret_cast(&mapBufferLength), - bytes_.data() + dynamicDataOffset + offset, - INT_SIZE); + int32_t mapBufferLength = *reinterpret_cast( + bytes_.data() + dynamicDataOffset + offset); std::vector value(mapBufferLength); diff --git a/ReactCommon/react/renderer/mapbuffer/MapBuffer.h b/ReactCommon/react/renderer/mapbuffer/MapBuffer.h index e89e1302c11..2afcee04251 100644 --- a/ReactCommon/react/renderer/mapbuffer/MapBuffer.h +++ b/ReactCommon/react/renderer/mapbuffer/MapBuffer.h @@ -45,6 +45,8 @@ class MapBuffer { // returns the relative offset of the first byte of dynamic data int32_t getDynamicDataOffset() const; + uint32_t getKeyBucket(Key key) const; + public: explicit MapBuffer(std::vector data); diff --git a/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.cpp b/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.cpp index 2e305b70576..89de1e34128 100644 --- a/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.cpp +++ b/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.cpp @@ -6,6 +6,7 @@ */ #include "MapBufferBuilder.h" +#include using namespace facebook::react; @@ -24,10 +25,6 @@ void MapBufferBuilder::storeKeyValue( Key key, uint8_t const *value, uint32_t valueSize) { - if (key < minKeyToStore_) { - LOG(ERROR) << "Error: key out of order - key: " << key; - abort(); - } if (valueSize > MAX_VALUE_SIZE) { LOG(ERROR) << "Error: size of value must be <= MAX_VALUE_SIZE. ValueSize: " << valueSize; @@ -42,7 +39,10 @@ void MapBufferBuilder::storeKeyValue( header_.count++; - minKeyToStore_ = key + 1; + if (lastKey_ > key) { + needsSort_ = true; + } + lastKey_ = key; } void MapBufferBuilder::putBool(Key key, bool value) { @@ -67,10 +67,8 @@ void MapBufferBuilder::putString(Key key, std::string const &value) { int32_t strSize = value.size(); const char *strData = value.data(); - 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); + auto offset = dynamicData_.size(); dynamicData_.resize(offset + INT_SIZE + strSize, 0); memcpy(dynamicData_.data() + offset, &strSize, INT_SIZE); memcpy(dynamicData_.data() + offset + INT_SIZE, strData, strSize); @@ -94,6 +92,10 @@ void MapBufferBuilder::putMapBuffer(Key key, MapBuffer const &map) { putInt(key, offset); } +static inline bool compareBuckets(Bucket const &a, Bucket const &b) { + return a.key < b.key; +} + MapBuffer MapBufferBuilder::build() { // Create buffer: [header] + [key, values] + [dynamic data] auto bucketSize = buckets_.size() * BUCKET_SIZE; @@ -101,6 +103,12 @@ MapBuffer MapBufferBuilder::build() { header_.bufferSize = bufferSize; + if (needsSort_) { + std::sort(buckets_.begin(), buckets_.end(), compareBuckets); + } + + // TODO(T83483191): add pass to check for duplicates + std::vector buffer(bufferSize); memcpy(buffer.data(), &header_, HEADER_SIZE); memcpy(buffer.data() + HEADER_SIZE, buckets_.data(), bucketSize); diff --git a/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.h b/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.h index 3d443679147..df40992aeb1 100644 --- a/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.h +++ b/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.h @@ -22,19 +22,6 @@ constexpr uint16_t INITIAL_BUCKETS_SIZE = 10; * MapBufferBuilder is a builder class for MapBuffer */ class MapBufferBuilder { - private: - Header header_ = {ALIGNMENT, 0, 0}; - - void storeKeyValue(Key key, uint8_t const *value, uint32_t valueSize); - - std::vector buckets_{}; - - std::vector dynamicData_{}; - - // Minimmum key to store in the MapBuffer (this is used to guarantee - // consistency) - uint16_t minKeyToStore_ = 0; - public: MapBufferBuilder(uint32_t initialSize = INITIAL_BUCKETS_SIZE); @@ -52,8 +39,20 @@ class MapBufferBuilder { void putMapBuffer(Key key, MapBuffer const &map); - // TODO T83483191: This should return MapBuffer! MapBuffer build(); + + private: + Header header_ = {ALIGNMENT, 0, 0}; + + std::vector buckets_{}; + + std::vector dynamicData_{}; + + uint16_t lastKey_{0}; + + bool needsSort_{false}; + + void storeKeyValue(Key key, uint8_t const *value, uint32_t valueSize); }; } // namespace react diff --git a/ReactCommon/react/renderer/mapbuffer/primitives.h b/ReactCommon/react/renderer/mapbuffer/primitives.h index 0676ac043ed..f5722a820f9 100644 --- a/ReactCommon/react/renderer/mapbuffer/primitives.h +++ b/ReactCommon/react/renderer/mapbuffer/primitives.h @@ -56,21 +56,7 @@ constexpr static int32_t HEADER_BUFFER_SIZE_OFFSET = constexpr static int32_t MAX_VALUE_SIZE = UINT64_SIZE; // 10 bytes : 2 key + 8 value -constexpr static int32_t BUCKET_SIZE = KEY_SIZE + UINT64_SIZE; - -/** - * Returns the offset of the key received by parameter. - */ -inline int32_t getKeyOffset(Key key) { - return HEADER_SIZE + BUCKET_SIZE * key; -} - -/** - * Returns the offset of the value associated to the key received by parameter. - */ -inline int32_t getValueOffset(Key key) { - return getKeyOffset(key) + KEY_SIZE; -} +constexpr static int32_t BUCKET_SIZE = sizeof(Bucket); inline void checkKeyConsistency(const Header &header, const uint8_t *data, Key key) { diff --git a/ReactCommon/react/renderer/mapbuffer/tests/MapBufferTest.cpp b/ReactCommon/react/renderer/mapbuffer/tests/MapBufferTest.cpp index ff81b608050..886c86d9c60 100644 --- a/ReactCommon/react/renderer/mapbuffer/tests/MapBufferTest.cpp +++ b/ReactCommon/react/renderer/mapbuffer/tests/MapBufferTest.cpp @@ -166,3 +166,20 @@ TEST(MapBufferTest, testMapEntries) { EXPECT_EQ(readMap2.getString(0), "This is a test"); EXPECT_EQ(readMap2.getInt(1), 1234); } + +TEST(MapBufferTest, testMapRandomAccess) { + auto builder = MapBufferBuilder(); + builder.putInt(1234, 4321); + builder.putString(0, "This is a test"); + builder.putDouble(8, 908.1); + builder.putNull(2); + builder.putString(65535, "Let's count: 的, 一, 是"); + auto map = builder.build(); + + EXPECT_EQ(map.count(), 5); + EXPECT_EQ(map.getString(0), "This is a test"); + EXPECT_EQ(map.getDouble(8), 908.1); + EXPECT_EQ(map.isNull(2), true); + EXPECT_EQ(map.getInt(1234), 4321); + EXPECT_EQ(map.getString(65535), "Let's count: 的, 一, 是"); +}