You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "viirya (via GitHub)" <gi...@apache.org> on 2023/06/17 16:19:46 UTC
[GitHub] [arrow-rs] viirya opened a new issue, #4431: Buffer from external memory doesn't match ScalarBuffer alignment requirement
viirya opened a new issue, #4431:
URL: https://github.com/apache/arrow-rs/issues/4431
**Describe the bug**
<!--
A clear and concise description of what the bug is.
-->
Recently some memory alignment error was found while passing arrays from Java arrow to Rust arrow. The error comes from `From<Buffer>` for `ScalarBuffer<T>`. I inspected the error further and am wondering if the alignment requirement doesn't make sense (at least for working with external allocated memory).
The idea of alignment check in `From<Buffer>` is making sure that the buffer pointer is aligned with scalar type `T`. The existing test shows that:
```rust
fn test_unaligned() {
let expected = [0_i32, 1, 2];
let buffer = Buffer::from_iter(expected.iter().cloned());
let buffer = buffer.slice(1);
ScalarBuffer::<i32>::new(buffer, 0, 2);
}
```
So the buffer slice is not aligned anymore with i32, so converting it to i32 ScalarBuffer panics with alignment error. But the assumption here is the memory region of the buffer is allocated with type. In Java Arrow, it allocates buffer simply with required number of bytes without type information, it's basically a contiguous memory region with `n` bytes. So obviously it doesn't necessarily have alignment with some type `T` there. When we try to pass array backed by the buffer to Rust arrow, the alignment check in `From<Buffer>` will fail occasionally (sometimes it passes the check if the allocated buffer happens to align with `T`).
**To Reproduce**
<!--
Steps to reproduce the behavior:
-->
**Expected behavior**
<!--
A clear and concise description of what you expected to happen.
-->
**Additional context**
<!--
Add any other context about the problem here.
-->
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on issue #4431: Buffer from external memory doesn't match ScalarBuffer alignment requirement
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #4431:
URL: https://github.com/apache/arrow-rs/issues/4431#issuecomment-1733478635
Closed by https://github.com/apache/arrow-rs/pull/4681
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on issue #4431: Buffer from external memory doesn't match ScalarBuffer alignment requirement
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on issue #4431:
URL: https://github.com/apache/arrow-rs/issues/4431#issuecomment-1595798681
I made some change to skip alignment check in `From<Buffer>` and let `Deref` panic when trying to deref a `ScalarBuffer` as `[T]`. However, currently a `ScalarBuffer` is assumed to be access as a slice using slice APIs, e.g., getting offset through `OffsetBuffer`. And we already expose these scalar buffers as slice, e.g. `GenericByteArray.value_offsets`. To get rid of these slice assumption seems some breaking changes.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold closed issue #4431: Buffer from external memory doesn't match ScalarBuffer alignment requirement
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold closed issue #4431: Buffer from external memory doesn't match ScalarBuffer alignment requirement
URL: https://github.com/apache/arrow-rs/issues/4431
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on issue #4431: Buffer from external memory doesn't match ScalarBuffer alignment requirement
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on issue #4431:
URL: https://github.com/apache/arrow-rs/issues/4431#issuecomment-1598472732
This is a fairly hard-coded assumption that boils down to Rust's fairly strict alignment rules, there isn't really a way to circumvent this. FWIW the arrow specification explicitly calls out that unaligned buffers are problematic, the Java implementation should probably be "fixed", as this will also cause issues for other implementations
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] viirya commented on issue #4431: Buffer from external memory doesn't match ScalarBuffer alignment requirement
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on issue #4431:
URL: https://github.com/apache/arrow-rs/issues/4431#issuecomment-1599650435
Yea, I think I agree with you that this is Rust's alignment rules so seems no good way to avoid it.
What I am not sure about is Arrow spec. I found there is also similar issue on C++ https://github.com/apache/arrow/issues/33336. And it points out that the spec does not require alignment but recommends it.
As this seems not possible to avoid in Rust, I will look into Java Arrow to see if it could be fixed there.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org