paleolimbot commented on code in PR #459:
URL: https://github.com/apache/arrow-nanoarrow/pull/459#discussion_r1597150916
##########
src/nanoarrow/buffer_inline.h:
##########
@@ -468,32 +479,38 @@ static inline void ArrowBitmapMove(struct ArrowBitmap*
src, struct ArrowBitmap*
static inline ArrowErrorCode ArrowBitmapReserve(struct ArrowBitmap* bitmap,
int64_t additional_size_bits) {
int64_t min_capacity_bits = bitmap->size_bits + additional_size_bits;
- if (min_capacity_bits <= (bitmap->buffer.capacity_bytes * 8)) {
+ int64_t min_capacity_bytes = _ArrowBytesForBits(min_capacity_bits);
+ int64_t current_size_bytes = bitmap->buffer.size_bytes;
+ int64_t current_capacity_bytes = bitmap->buffer.capacity_bytes;
+
+ if (min_capacity_bytes <= current_capacity_bytes) {
return NANOARROW_OK;
}
- NANOARROW_RETURN_NOT_OK(
- ArrowBufferReserve(&bitmap->buffer,
_ArrowBytesForBits(additional_size_bits)));
+ int64_t additional_capacity_bytes = min_capacity_bytes - current_size_bytes;
+ NANOARROW_RETURN_NOT_OK(ArrowBufferReserve(&bitmap->buffer,
additional_capacity_bytes));
+ // Zero out the last byte for deterministic output in the common case
Review Comment:
When I first wrote this I copied this behaviour from Arrow C++:
https://github.com/apache/arrow/blob/7aea8bf7a65d679bd71d973b358f997eb3b6c6af/cpp/src/arrow/buffer.cc#L183-L189
...although there is also precedent in Arrow C++ to zero out any additional
allocated memory:
https://github.com/apache/arrow/blob/7aea8bf7a65d679bd71d973b358f997eb3b6c6af/cpp/src/arrow/buffer_builder.h#L418-L423
In general nanoarrow's "reserve" step will never initialize memory since it
assumes that the caller is about to do that. For a bitmap the caller might want
to initialize to `0xff` or `0x00` depending on what they're about to do...the
final byte is just to make it more likely that somebody building a bitmap of a
pre-known length will get deterministic output (without any performance issue).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
