Skip to content

Commit

Permalink
Avoid ByteBuffer copies when reading nested MapBuffers (#44436)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #44436

The backing buffer behind `ReadableMapBuffer` is effectively immutable, so we can make reads of nested MapBuffers work on an inline view of the same buffer. This book-keeping is kept within ReadableMapBuffer (we can not user `ByteBuffer.wrap()` because the fbjni produces ByteBuffer is not array backed).

The main downside I can think of is that the whole buffer is kept in memory until all children buffers leave, but current use-cases don't involve long-term storage of MapBuffer children, so this is probably a better tradeoff.

Changelog: [Internal]

Reviewed By: javache

Differential Revision: D57020759

fbshipit-source-id: d2f5a76561fa4a4219fe5022ba62cc96f56ce022
  • Loading branch information
NickGerleman authored and facebook-github-bot committed May 7, 2024
1 parent 29c3bc0 commit 9da6546
Showing 1 changed file with 14 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public class ReadableMapBuffer : MapBuffer {

// Byte data of the mapBuffer
private val buffer: ByteBuffer
// Offset to the start of the MapBuffer
private val offsetToMapBuffer: Int
// Amount of items serialized on the ByteBuffer
override var count: Int = 0
private set
Expand All @@ -40,12 +42,21 @@ public class ReadableMapBuffer : MapBuffer {
private constructor(hybridData: HybridData) {
mHybridData = hybridData
buffer = importByteBuffer()
offsetToMapBuffer = 0
readHeader()
}

private constructor(buffer: ByteBuffer) {
mHybridData = null
this.buffer = buffer
offsetToMapBuffer = 0
readHeader()
}

private constructor(buffer: ByteBuffer, offset: Int) {
mHybridData = null
this.buffer = buffer.duplicate().apply { position(offset) }
offsetToMapBuffer = offset
readHeader()
}

Expand Down Expand Up @@ -135,12 +146,7 @@ public class ReadableMapBuffer : MapBuffer {

private fun readMapBufferValue(position: Int): ReadableMapBuffer {
val offset = offsetForDynamicData + buffer.getInt(position)
val sizeMapBuffer = buffer.getInt(offset)
val newBuffer = ByteArray(sizeMapBuffer)
val bufferOffset = offset + Int.SIZE_BYTES
buffer.position(bufferOffset)
buffer[newBuffer, 0, sizeMapBuffer]
return ReadableMapBuffer(ByteBuffer.wrap(newBuffer))
return ReadableMapBuffer(buffer, offset + Int.SIZE_BYTES)
}

private fun readMapBufferListValue(position: Int): List<ReadableMapBuffer> {
Expand All @@ -151,18 +157,15 @@ public class ReadableMapBuffer : MapBuffer {
var curLen = 0
while (curLen < sizeMapBufferList) {
val sizeMapBuffer = buffer.getInt(offset + curLen)
val newMapBuffer = ByteArray(sizeMapBuffer)
curLen = curLen + Int.SIZE_BYTES
buffer.position(offset + curLen)
buffer[newMapBuffer, 0, sizeMapBuffer]
readMapBufferList.add(ReadableMapBuffer(ByteBuffer.wrap(newMapBuffer)))
readMapBufferList.add(ReadableMapBuffer(buffer, offset + curLen))
curLen = curLen + sizeMapBuffer
}
return readMapBufferList
}

private fun getKeyOffsetForBucketIndex(bucketIndex: Int): Int {
return HEADER_SIZE + BUCKET_SIZE * bucketIndex
return offsetToMapBuffer + HEADER_SIZE + BUCKET_SIZE * bucketIndex
}

override fun contains(key: Int): Boolean {
Expand Down

0 comments on commit 9da6546

Please sign in to comment.