Optimize MapBuffer representation (#57361)

Summary:
Pull Request resolved: #57361

Broadens the former header-shrink change into a single representation-optimization commit for MapBuffer. It bundles three layout optimizations that were previously split: (1) the header is reduced to a single 2-byte `count` field; (2) multi-byte values are read via `memcpy` so unaligned access is well-defined on all platforms; (3) every dynamic-data entry (`String`, `Map`, `MapBufferList`, `IntBuffer`, `DoubleBuffer`) packs its `[offset][byteLength]` into the bucket's 8-byte value instead of writing an in-band length prefix into the dynamic data section.

Net effect: 4 fewer bytes per dynamic entry, one fewer indirection on read (the length is already in the bucket), and every dynamic entry becomes self-delimiting from its bucket alone. No public API change — only the internal serialized representation.

Changelog: [Internal]

Reviewed By: lenaic, zeyap

Differential Revision: D109848478

Author: javache

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/common/mapbuffer/ReadableMapBuffer.kt

@@ -51,13 +51,11 @@ private constructor(
       ReadableMapBuffer(buffer.duplicate().apply { position(offset) }, offset)
 
   private fun readHeader() {
-    // byte order
-    val storedAlignment = buffer.short
-    if (storedAlignment.toInt() != ALIGNMENT) {
-      buffer.order(ByteOrder.LITTLE_ENDIAN)
-    }
-    // count
-    count = readUnsignedShort(buffer.position()).toInt()
+    // The C++ writer always serializes in little-endian byte order. ByteBuffer
+    // defaults to big-endian and duplicate() resets the order, so set it
+    // explicitly on every instance, including nested clones.
+    buffer.order(ByteOrder.LITTLE_ENDIAN)
+    count = readUnsignedShort(offsetToMapBuffer).toInt()
   }
 
   /**
@@ -122,26 +120,27 @@ private constructor(
     return readIntValue(bufferPosition) == 1
   }
 
+  // Dynamic-data entries store [offset][byteLength] in the bucket's 8-byte
+  // value: getInt(bufferPosition) is the offset, getInt(bufferPosition + 4) is
+  // the byte length. The dynamic data section itself carries no length prefix.
   private fun readStringValue(bufferPosition: Int): String {
     val offset = offsetForDynamicData + buffer.getInt(bufferPosition)
-    val sizeOfString = buffer.getInt(offset)
+    val sizeOfString = buffer.getInt(bufferPosition + Int.SIZE_BYTES)
     val result = ByteArray(sizeOfString)
-    val stringOffset = offset + Int.SIZE_BYTES
-    buffer.position(stringOffset)
-    buffer[result, 0, sizeOfString]
+    buffer.position(offset)
+    buffer.get(result, 0, sizeOfString)
     return String(result)
   }
 
   private fun readMapBufferValue(position: Int): ReadableMapBuffer {
     val offset = offsetForDynamicData + buffer.getInt(position)
-    return cloneWithOffset(offset + Int.SIZE_BYTES)
+    return cloneWithOffset(offset)
   }
 
   private fun readMapBufferListValue(position: Int): List<ReadableMapBuffer> {
     val readMapBufferList = arrayListOf<ReadableMapBuffer>()
-    var offset = offsetForDynamicData + buffer.getInt(position)
-    val sizeMapBufferList = buffer.getInt(offset)
-    offset += Int.SIZE_BYTES
+    val offset = offsetForDynamicData + buffer.getInt(position)
+    val sizeMapBufferList = buffer.getInt(position + Int.SIZE_BYTES)
     var curLen = 0
     while (curLen < sizeMapBufferList) {
       val sizeMapBuffer = buffer.getInt(offset + curLen)
@@ -153,16 +152,16 @@ private constructor(
   }
 
   private fun readIntBufferValue(bufferPosition: Int): IntArray {
-    var offset = offsetForDynamicData + buffer.getInt(bufferPosition)
-    val count = buffer.getInt(offset)
-    offset += Int.SIZE_BYTES
+    val offset = offsetForDynamicData + buffer.getInt(bufferPosition)
+    val byteLength = buffer.getInt(bufferPosition + Int.SIZE_BYTES)
+    val count = byteLength / Int.SIZE_BYTES
     return IntArray(count) { i -> buffer.getInt(offset + i * Int.SIZE_BYTES) }
   }
 
   private fun readDoubleBufferValue(bufferPosition: Int): DoubleArray {
-    var offset = offsetForDynamicData + buffer.getInt(bufferPosition)
-    val count = buffer.getInt(offset)
-    offset += Int.SIZE_BYTES
+    val offset = offsetForDynamicData + buffer.getInt(bufferPosition)
+    val byteLength = buffer.getInt(bufferPosition + Int.SIZE_BYTES)
+    val count = byteLength / Double.SIZE_BYTES
     return DoubleArray(count) { i -> buffer.getDouble(offset + i * Double.SIZE_BYTES) }
   }
 
@@ -355,13 +354,10 @@ private constructor(
   }
 
   public companion object {
-    // Value used to verify if the data is serialized with LittleEndian order.
-    private const val ALIGNMENT = 0xFE
-
-    // 8 bytes = 2 (alignment) + 2 (count) + 4 (size)
-    private const val HEADER_SIZE = 8
+    // 2 bytes = 2 (count)
+    private const val HEADER_SIZE = 2
 
-    // 10 bytes = 2 (key) + 2 (type) + 8 (value)
+    // 12 bytes = 2 (key) + 2 (type) + 8 (value)
     private const val BUCKET_SIZE = 12
 
     // 2 bytes = 2 (key)

packages/react-native/ReactCommon/react/renderer/mapbuffer/MapBuffer.cpp

@@ -8,8 +8,43 @@
 #include "MapBuffer.h"
 #include <react/renderer/mapbuffer/MapBufferBuilder.h>
 
+#include <algorithm>
+#include <cstring>
+
 namespace facebook::react {
 
+namespace {
+// Reads a value of type T from a (possibly unaligned) offset in the buffer.
+// MapBuffer's packed layout places multi-byte values at offsets that are not
+// naturally aligned for their type (e.g. an 8-byte value at a 2-byte boundary),
+// so dereferencing a reinterpret_cast pointer there is undefined behavior and
+// can fault on 32-bit ARM. memcpy compiles to a single unaligned load on
+// arm64/x86 and to alignment-safe loads on armv7.
+template <typename T>
+inline T readUnaligned(const uint8_t* data, int32_t offset) {
+  T value;
+  std::memcpy(&value, data + offset, sizeof(T));
+  return value;
+}
+
+// Debug-asserts on OOB (catches corrupt buffers early in dev) AND clamps in
+// release so a corrupt bucket length can never drive an OOB memcpy read.
+// react_native_assert is compiled out in release, so the runtime cost outside
+// dev is the single min() call.
+inline int32_t
+clampToBufferBounds(int32_t offset, int32_t byteLength, size_t bufferSize) {
+  react_native_assert(offset >= 0 && byteLength >= 0);
+  react_native_assert(
+      static_cast<size_t>(offset) + static_cast<size_t>(byteLength) <=
+      bufferSize);
+  size_t maxLength = bufferSize > static_cast<size_t>(offset)
+      ? bufferSize - static_cast<size_t>(offset)
+      : 0;
+  return static_cast<int32_t>(
+      std::min(static_cast<size_t>(std::max(byteLength, 0)), maxLength));
+}
+} // namespace
+
 static inline int32_t bucketOffset(int32_t index) {
   return sizeof(MapBuffer::Header) + sizeof(MapBuffer::Bucket) * index;
 }
@@ -18,16 +53,20 @@ static inline int32_t valueOffset(int32_t bucketIndex) {
   return bucketOffset(bucketIndex) + offsetof(MapBuffer::Bucket, data);
 }
 
+// Dynamic-data entries pack [offset (low 32 bits)][byteLength (high 32 bits)]
+// into the bucket's 8-byte value, so the payload in the dynamic data section
+// carries no in-band length prefix. This returns the position of the high
+// 32 bits (the length).
+static inline int32_t lengthOffset(int32_t bucketIndex) {
+  return valueOffset(bucketIndex) + static_cast<int32_t>(sizeof(int32_t));
+}
+
 // TODO T83483191: Extend MapBuffer C++ implementation to support basic random
 // access
 MapBuffer::MapBuffer(std::vector<uint8_t> data) : bytes_(std::move(data)) {
-  auto header = reinterpret_cast<const Header*>(bytes_.data());
-  count_ = header->count;
-
-  if (header->bufferSize != bytes_.size()) {
-    LOG(ERROR) << "Error: Data size does not match, expected "
-               << header->bufferSize << " found: " << bytes_.size();
-    abort();
+  if (bytes_.size() >= sizeof(Header)) {
+    auto header = reinterpret_cast<const Header*>(bytes_.data());
+    count_ = header->count;
   }
 }
 
@@ -37,8 +76,7 @@ int32_t MapBuffer::getKeyBucket(Key key) const {
   while (lo <= hi) {
     int32_t mid = (lo + hi) >> 1;
 
-    Key midVal =
-        *reinterpret_cast<const Key*>(bytes_.data() + bucketOffset(mid));
+    Key midVal = readUnaligned<Key>(bytes_.data(), bucketOffset(mid));
 
     if (midVal < key) {
       lo = mid + 1;
@@ -53,8 +91,7 @@ int32_t MapBuffer::getKeyBucket(Key key) const {
 }
 
 inline int32_t MapBuffer::getIntAtBucket(int32_t bucketIndex) const {
-  return *reinterpret_cast<const int32_t*>(
-      bytes_.data() + valueOffset(bucketIndex));
+  return readUnaligned<int32_t>(bytes_.data(), valueOffset(bucketIndex));
 }
 
 int32_t MapBuffer::getInt(Key key) const {
@@ -74,8 +111,7 @@ int64_t MapBuffer::getLong(Key key) const {
     return 0;
   }
 
-  return *reinterpret_cast<const int64_t*>(
-      bytes_.data() + valueOffset(bucketIndex));
+  return readUnaligned<int64_t>(bytes_.data(), valueOffset(bucketIndex));
 }
 
 bool MapBuffer::getBool(Key key) const {
@@ -89,8 +125,7 @@ double MapBuffer::getDouble(Key key) const {
     return 0;
   }
 
-  return *reinterpret_cast<const double*>(
-      bytes_.data() + valueOffset(bucketIndex));
+  return readUnaligned<double>(bytes_.data(), valueOffset(bucketIndex));
 }
 
 int32_t MapBuffer::getDynamicDataOffset() const {
@@ -107,9 +142,10 @@ std::string MapBuffer::getString(Key key) const {
   }
 
   int32_t offset = getDynamicDataOffset() + getIntAtBucket(bucketIndex);
-  int32_t stringLength =
-      *reinterpret_cast<const int32_t*>(bytes_.data() + offset);
-  const uint8_t* stringPtr = bytes_.data() + offset + sizeof(int);
+  auto stringLength =
+      readUnaligned<int32_t>(bytes_.data(), lengthOffset(bucketIndex));
+  stringLength = clampToBufferBounds(offset, stringLength, bytes_.size());
+  const uint8_t* stringPtr = bytes_.data() + offset;
 
   return {stringPtr, stringPtr + stringLength};
 }
@@ -122,17 +158,13 @@ MapBuffer MapBuffer::getMapBuffer(Key key) const {
   }
 
   int32_t offset = getDynamicDataOffset() + getIntAtBucket(bucketIndex);
-  int32_t mapBufferLength =
-      *reinterpret_cast<const int32_t*>(bytes_.data() + offset);
-  size_t maxLength = bytes_.size() - offset - sizeof(int32_t);
-  if (mapBufferLength > maxLength) {
-    mapBufferLength = maxLength;
-  }
+  auto mapBufferLength =
+      readUnaligned<int32_t>(bytes_.data(), lengthOffset(bucketIndex));
+  mapBufferLength = clampToBufferBounds(offset, mapBufferLength, bytes_.size());
 
   std::vector<uint8_t> value(mapBufferLength);
 
-  memcpy(
-      value.data(), bytes_.data() + offset + sizeof(int32_t), mapBufferLength);
+  memcpy(value.data(), bytes_.data() + offset, mapBufferLength);
 
   return MapBuffer(std::move(value));
 }
@@ -146,19 +178,27 @@ std::vector<MapBuffer> MapBuffer::getMapBufferList(MapBuffer::Key key) const {
 
   std::vector<MapBuffer> mapBufferList;
   int32_t offset = getDynamicDataOffset() + getIntAtBucket(bucketIndex);
-  int32_t mapBufferListLength =
-      *reinterpret_cast<const int32_t*>(bytes_.data() + offset);
-  offset = offset + sizeof(uint32_t);
+  auto mapBufferListLength =
+      readUnaligned<int32_t>(bytes_.data(), lengthOffset(bucketIndex));
+  mapBufferListLength =
+      clampToBufferBounds(offset, mapBufferListLength, bytes_.size());
 
   int32_t curLen = 0;
   while (curLen < mapBufferListLength) {
-    int32_t mapBufferLength =
-        *reinterpret_cast<const int32_t*>(bytes_.data() + offset + curLen);
-    curLen = curLen + sizeof(uint32_t);
+    if (curLen + sizeof(int32_t) > mapBufferListLength) {
+      break;
+    }
+
+    auto mapBufferLength =
+        readUnaligned<int32_t>(bytes_.data(), offset + curLen);
+    curLen += sizeof(int32_t);
+
+    mapBufferLength =
+        clampToBufferBounds(offset + curLen, mapBufferLength, bytes_.size());
     std::vector<uint8_t> value(mapBufferLength);
     memcpy(value.data(), bytes_.data() + offset + curLen, mapBufferLength);
     mapBufferList.emplace_back(std::move(value));
-    curLen = curLen + mapBufferLength;
+    curLen += mapBufferLength;
   }
   return mapBufferList;
 }
@@ -171,13 +211,18 @@ std::vector<int32_t> MapBuffer::getIntBuffer(MapBuffer::Key key) const {
   }
 
   int32_t offset = getDynamicDataOffset() + getIntAtBucket(bucketIndex);
-  int32_t count = *reinterpret_cast<const int32_t*>(bytes_.data() + offset);
+  auto byteLength =
+      readUnaligned<int32_t>(bytes_.data(), lengthOffset(bucketIndex));
+  byteLength = clampToBufferBounds(offset, byteLength, bytes_.size());
+  int32_t count = byteLength / static_cast<int32_t>(sizeof(int32_t));
 
   std::vector<int32_t> result(count);
   if (count > 0) {
+    // Copy only whole elements: a clamped byteLength may not be a multiple of
+    // sizeof(int32_t), and result holds exactly count elements.
     memcpy(
         result.data(),
-        bytes_.data() + offset + sizeof(int32_t),
+        bytes_.data() + offset,
         static_cast<size_t>(count) * sizeof(int32_t));
   }
   return result;
@@ -191,13 +236,18 @@ std::vector<double> MapBuffer::getDoubleBuffer(MapBuffer::Key key) const {
   }
 
   int32_t offset = getDynamicDataOffset() + getIntAtBucket(bucketIndex);
-  int32_t count = *reinterpret_cast<const int32_t*>(bytes_.data() + offset);
+  auto byteLength =
+      readUnaligned<int32_t>(bytes_.data(), lengthOffset(bucketIndex));
+  byteLength = clampToBufferBounds(offset, byteLength, bytes_.size());
+  int32_t count = byteLength / static_cast<int32_t>(sizeof(double));
 
   std::vector<double> result(count);
   if (count > 0) {
+    // Copy only whole elements: a clamped byteLength may not be a multiple of
+    // sizeof(double), and result holds exactly count elements.
     memcpy(
         result.data(),
-        bytes_.data() + offset + sizeof(int32_t),
+        bytes_.data() + offset,
         static_cast<size_t>(count) * sizeof(double));
   }
   return result;

packages/react-native/ReactCommon/react/renderer/mapbuffer/MapBuffer.h

@@ -40,11 +40,11 @@ class JReadableMapBuffer;
  *
  * MapBuffer data is stored in a continuous chunk of memory (bytes_ field below) with the following layout:
  *
- * ┌─────────────────────Header──────────────────────┐
- * │                    10 bytes                     │
- * ├─Alignment─┬─Item count─┬──────Buffer size───────┤
- * │  2 bytes  │  2 bytes   │        4 bytes         │
- * └───────────┴────────────┴────────────────────────┘
+ * ┌──────Header──────┐
+ * │      2 bytes     │
+ * ├────Item count────┤
+ * │      2 bytes     │
+ * └──────────────────┘
  * ┌────────────────────────────────────────────────────────────────────────────────────────┐
  * │                           Buckets (one per item in the map)                            │
  * │                                                                                        │
@@ -69,14 +69,8 @@ class MapBuffer {
  public:
   using Key = uint16_t;
 
-  // The first value in the buffer, used to check correct encoding/endianness on
-  // JVM side.
-  constexpr static uint16_t HEADER_ALIGNMENT = 0xFE;
-
   struct Header {
-    uint16_t alignment = HEADER_ALIGNMENT; // alignment of serialization
     uint16_t count; // amount of items in the map
-    uint32_t bufferSize; // Amount of bytes used to store the map in memory
   };
 
 #pragma pack(push, 1)
@@ -89,7 +83,7 @@ class MapBuffer {
   };
 #pragma pack(pop)
 
-  static_assert(sizeof(Header) == 8, "MapBuffer header size is incorrect.");
+  static_assert(sizeof(Header) == 2, "MapBuffer header size is incorrect.");
   static_assert(sizeof(Bucket) == 12, "MapBuffer bucket size is incorrect.");
 
   /**
@@ -105,15 +99,17 @@ class MapBuffer {
     String = 3,
     Map = 4,
     Long = 5,
-    // Homogeneous, length-prefixed arrays stored contiguously in the dynamic
+    // Homogeneous arrays of raw elements stored contiguously in the dynamic
     // data section. Unlike Map, they carry no per-element key/type overhead, so
-    // a batch of N values costs ~N*elementSize bytes plus a single 4-byte count
-    // prefix instead of N*12-byte buckets. The bucket value is the offset of the
-    // array within the dynamic data section.
+    // a batch of N values costs ~N*elementSize bytes instead of N*12-byte
+    // buckets. The bucket value packs [offset][byteLength]; the element count is
+    // recovered as byteLength / elementSize.
     IntBuffer = 6,
     DoubleBuffer = 7,
-    // A homogeneous, ordered array of nested MapBuffers. Distinct from `Map` so
-    // that a list of MapBuffers is self-describing (a single Map and a list are
+    // A homogeneous, ordered array of nested MapBuffers. The bucket value packs
+    // [offset][byteLength] for the whole list region; within it each child stays
+    // framed as [int32 childSize][child bytes]. Distinct from `Map` so that a
+    // list of MapBuffers is self-describing (a single Map and a list are
     // byte-distinct in payload but previously shared the `Map` type tag).
     MapBufferList = 8,
   };