diff --git a/ReactAndroid/src/main/java/com/facebook/react/common/mapbuffer/ReadableMapBuffer.java b/ReactAndroid/src/main/java/com/facebook/react/common/mapbuffer/ReadableMapBuffer.java index 4d633aae998..cbcaf8169a4 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/common/mapbuffer/ReadableMapBuffer.java +++ b/ReactAndroid/src/main/java/com/facebook/react/common/mapbuffer/ReadableMapBuffer.java @@ -25,17 +25,32 @@ public class ReadableMapBuffer implements Iterable( bytes_.data() + valueOffset(bucketIndex)); - ; } int32_t MapBuffer::getDynamicDataOffset() const { @@ -113,10 +112,6 @@ MapBuffer MapBuffer::getMapBuffer(Key key) const { return MapBuffer(std::move(value)); } -bool MapBuffer::isNull(Key key) const { - return getInt(key) == NULL_VALUE; -} - uint32_t MapBuffer::size() const { return bytes_.size(); } diff --git a/ReactCommon/react/renderer/mapbuffer/MapBuffer.h b/ReactCommon/react/renderer/mapbuffer/MapBuffer.h index 2afcee04251..45e415c0a23 100644 --- a/ReactCommon/react/renderer/mapbuffer/MapBuffer.h +++ b/ReactCommon/react/renderer/mapbuffer/MapBuffer.h @@ -18,11 +18,11 @@ namespace react { class ReadableMapBuffer; /** - * MapBuffer is an optimized map format for transferring data like props between - * C++ and other platforms The implementation of this map is optimized to: + * MapBuffer is an optimized sparse array format for transferring props-like + * between C++ and other VMs. The implementation of this map is optimized to: * - be compact to optimize space when sparse (sparse is the common case). * - be accessible through JNI with zero/minimal copying via ByteBuffer. - * - be Have excellent C++ single-write and many-read performance by maximizing + * - Have excellent C++ single-write and many-read performance by maximizing * CPU cache performance through compactness, data locality, and fixed offsets * where possible. * - be optimized for iteration and intersection against other maps, but with @@ -33,21 +33,20 @@ class ReadableMapBuffer; * - have minimal APK size and build time impact. */ class MapBuffer { - friend ReadableMapBuffer; - - private: - // Buffer and its size - std::vector const bytes_; - - // amount of items in the MapBuffer - uint16_t count_ = 0; - - // returns the relative offset of the first byte of dynamic data - int32_t getDynamicDataOffset() const; - - uint32_t getKeyBucket(Key key) const; - public: + /** + * Data types available for serialization in MapBuffer + * Keep in sync with `DataType` enum in `ReadableMapBuffer.java`, which + * expects the same values after reading them through JNI. + */ + enum DataType : uint16_t { + Boolean = 0, + Int = 1, + Double = 2, + String = 3, + Map = 4, + }; + explicit MapBuffer(std::vector data); MapBuffer(MapBuffer const &buffer) = delete; @@ -67,13 +66,25 @@ class MapBuffer { // TODO T83483191: review this declaration MapBuffer getMapBuffer(Key key) const; - bool isNull(Key key) const; - uint32_t size() const; uint8_t const *data() const; uint16_t count() const; + + private: + // Buffer and its size + std::vector const bytes_; + + // amount of items in the MapBuffer + uint16_t count_ = 0; + + // returns the relative offset of the first byte of dynamic data + int32_t getDynamicDataOffset() const; + + uint32_t getKeyBucket(Key key) const; + + friend ReadableMapBuffer; }; } // namespace react diff --git a/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.cpp b/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.cpp index 89de1e34128..8438686bc87 100644 --- a/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.cpp +++ b/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.cpp @@ -23,7 +23,8 @@ MapBufferBuilder::MapBufferBuilder(uint32_t initialSize) { void MapBufferBuilder::storeKeyValue( Key key, - uint8_t const *value, + MapBuffer::DataType type, + Byte const *value, uint32_t valueSize) { if (valueSize > MAX_VALUE_SIZE) { LOG(ERROR) << "Error: size of value must be <= MAX_VALUE_SIZE. ValueSize: " @@ -35,7 +36,7 @@ void MapBufferBuilder::storeKeyValue( auto *dataPtr = reinterpret_cast(&data); memcpy(dataPtr, value, valueSize); - buckets_.emplace_back(key, data); + buckets_.emplace_back(key, static_cast(type), data); header_.count++; @@ -46,21 +47,28 @@ void MapBufferBuilder::storeKeyValue( } void MapBufferBuilder::putBool(Key key, bool value) { - putInt(key, (int)value); + int intValue = (int)value; + storeKeyValue( + key, + MapBuffer::DataType::Boolean, + reinterpret_cast(&intValue), + INT_SIZE); } void MapBufferBuilder::putDouble(Key key, double value) { - auto const *bytePointer = reinterpret_cast(&value); - storeKeyValue(key, bytePointer, DOUBLE_SIZE); -} - -void MapBufferBuilder::putNull(Key key) { - putInt(key, NULL_VALUE); + storeKeyValue( + key, + MapBuffer::DataType::Double, + reinterpret_cast(&value), + DOUBLE_SIZE); } void MapBufferBuilder::putInt(Key key, int32_t value) { - auto const *bytePointer = reinterpret_cast(&(value)); - storeKeyValue(key, bytePointer, INT_SIZE); + storeKeyValue( + key, + MapBuffer::DataType::Int, + reinterpret_cast(&value), + INT_SIZE); } void MapBufferBuilder::putString(Key key, std::string const &value) { @@ -74,7 +82,11 @@ void MapBufferBuilder::putString(Key key, std::string const &value) { memcpy(dynamicData_.data() + offset + INT_SIZE, strData, strSize); // Store Key and pointer to the string - putInt(key, offset); + storeKeyValue( + key, + MapBuffer::DataType::String, + reinterpret_cast(&offset), + INT_SIZE); } void MapBufferBuilder::putMapBuffer(Key key, MapBuffer const &map) { @@ -89,7 +101,11 @@ void MapBufferBuilder::putMapBuffer(Key key, MapBuffer const &map) { memcpy(dynamicData_.data() + offset + INT_SIZE, map.data(), mapBufferSize); // Store Key and pointer to the string - putInt(key, offset); + storeKeyValue( + key, + MapBuffer::DataType::Map, + reinterpret_cast(&offset), + INT_SIZE); } static inline bool compareBuckets(Bucket const &a, Bucket const &b) { diff --git a/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.h b/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.h index df40992aeb1..27a48d2bf81 100644 --- a/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.h +++ b/ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.h @@ -16,7 +16,7 @@ namespace facebook { namespace react { // Default reserved size for buckets_ vector -constexpr uint16_t INITIAL_BUCKETS_SIZE = 10; +constexpr uint32_t INITIAL_BUCKETS_SIZE = 10; /** * MapBufferBuilder is a builder class for MapBuffer @@ -33,8 +33,6 @@ class MapBufferBuilder { void putDouble(Key key, double value); - void putNull(Key key); - void putString(Key key, std::string const &value); void putMapBuffer(Key key, MapBuffer const &map); @@ -52,7 +50,11 @@ class MapBufferBuilder { bool needsSort_{false}; - void storeKeyValue(Key key, uint8_t const *value, uint32_t valueSize); + void storeKeyValue( + Key key, + MapBuffer::DataType type, + Byte const *value, + uint32_t valueSize); }; } // namespace react diff --git a/ReactCommon/react/renderer/mapbuffer/primitives.h b/ReactCommon/react/renderer/mapbuffer/primitives.h index f5722a820f9..58d4b2012d3 100644 --- a/ReactCommon/react/renderer/mapbuffer/primitives.h +++ b/ReactCommon/react/renderer/mapbuffer/primitives.h @@ -34,12 +34,14 @@ static_assert(sizeof(Header) == 8, "MapBuffer header size is incorrect."); struct __attribute__((__packed__)) Bucket { Key key; + uint16_t type; uint64_t data; - Bucket(Key key, uint64_t data) : key(key), data(data){}; + Bucket(Key key, uint16_t type, uint64_t data) + : key(key), type(type), data(data) {} }; -static_assert(sizeof(Bucket) == 10, "MapBuffer bucket size is incorrect."); +static_assert(sizeof(Bucket) == 12, "MapBuffer bucket size is incorrect."); constexpr static int32_t KEY_SIZE = sizeof(Key); constexpr static int32_t HEADER_SIZE = sizeof(Header); @@ -55,7 +57,6 @@ 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 = sizeof(Bucket); inline void diff --git a/ReactCommon/react/renderer/mapbuffer/tests/MapBufferTest.cpp b/ReactCommon/react/renderer/mapbuffer/tests/MapBufferTest.cpp index 886c86d9c60..f0d5c737541 100644 --- a/ReactCommon/react/renderer/mapbuffer/tests/MapBufferTest.cpp +++ b/ReactCommon/react/renderer/mapbuffer/tests/MapBufferTest.cpp @@ -59,23 +59,6 @@ TEST(MapBufferTest, testBoolEntries) { EXPECT_EQ(map.getBool(1), false); } -TEST(MapBufferTest, testNullEntries) { - auto buffer = MapBufferBuilder(); - - buffer.putNull(0); - buffer.putInt(1, 1234); - - auto map = buffer.build(); - - 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' - // values - // EXPECT_EQ(map.isNull(1), false); - // EXPECT_EQ(map.getBool(1), false); -} - TEST(MapBufferTest, testDoubleEntries) { auto buffer = MapBufferBuilder(); @@ -172,14 +155,12 @@ TEST(MapBufferTest, testMapRandomAccess) { 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.count(), 4); 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: 的, 一, 是"); }