You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/03/10 15:44:22 UTC
[GitHub] [arrow-rs] jhorstmann opened a new pull request #1421: Fix possibly unaligned writes in MutableBuffer
jhorstmann opened a new pull request #1421:
URL: https://github.com/apache/arrow-rs/pull/1421
# Which issue does this PR close?
<!---
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #1410.
# Rationale for this change
Changes the `push`, `push_unchecked` and `extend_from_iter` functions so that data is transfered using `copy_nonoverlapping` which works for unaligned destinations. LLVM will most likely transform this again into a single unaligned write. This also makes these methods actually use the `ToByteSlice` trait. In `extend_from_slice`, which this PR did not change, the trait is more used like a marker trait because the whole slice is copied without going through the trait methods. This is probably fine since it's supposed to be an internal trait that is only implemented for `ArrowNativeType`.
All newly added tests were previously failing when running under miri.
--
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] jhorstmann commented on pull request #1421: Fix possibly unaligned writes in MutableBuffer
Posted by GitBox <gi...@apache.org>.
jhorstmann commented on pull request #1421:
URL: https://github.com/apache/arrow-rs/pull/1421#issuecomment-1064926553
In `from_trusted_len_iter` the `MutableBuffer` is created inside the method, so there should be no way for the pointer to become unaligned. Probably still a good idea to adjust it for consistency.
--
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] jhorstmann commented on a change in pull request #1421: Fix possibly unaligned writes in MutableBuffer
Posted by GitBox <gi...@apache.org>.
jhorstmann commented on a change in pull request #1421:
URL: https://github.com/apache/arrow-rs/pull/1421#discussion_r824952619
##########
File path: arrow/src/buffer/mutable.rs
##########
@@ -710,6 +713,46 @@ mod tests {
);
}
+ #[test]
+ fn mutable_extend_from_iter_unaligned_u64() {
+ let mut buf = MutableBuffer::new(16);
+ buf.push(1_u8);
+ buf.extend([1_u64]);
+ dbg!(&buf.as_slice());
+ assert_eq!(9, buf.len());
+ assert_eq!(&[1u8, 1u8, 0, 0, 0, 0, 0, 0, 0], buf.as_slice());
+ }
+
+ #[test]
+ fn mutable_extend_from_slice_unaligned_u64() {
+ let mut buf = MutableBuffer::new(16);
+ buf.extend_from_slice(&[1_u8]);
+ buf.extend_from_slice(&[1_u64]);
+ dbg!(&buf.as_slice());
Review comment:
Thanks for the review! The dbg is now removed.
--
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] sunchao merged pull request #1421: Fix possibly unaligned writes in MutableBuffer
Posted by GitBox <gi...@apache.org>.
sunchao merged pull request #1421:
URL: https://github.com/apache/arrow-rs/pull/1421
--
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] sunchao commented on pull request #1421: Fix possibly unaligned writes in MutableBuffer
Posted by GitBox <gi...@apache.org>.
sunchao commented on pull request #1421:
URL: https://github.com/apache/arrow-rs/pull/1421#issuecomment-1065419887
Merged, thanks!
--
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] sunchao commented on a change in pull request #1421: Fix possibly unaligned writes in MutableBuffer
Posted by GitBox <gi...@apache.org>.
sunchao commented on a change in pull request #1421:
URL: https://github.com/apache/arrow-rs/pull/1421#discussion_r824881391
##########
File path: arrow/src/buffer/mutable.rs
##########
@@ -710,6 +713,46 @@ mod tests {
);
}
+ #[test]
+ fn mutable_extend_from_iter_unaligned_u64() {
+ let mut buf = MutableBuffer::new(16);
+ buf.push(1_u8);
+ buf.extend([1_u64]);
+ dbg!(&buf.as_slice());
+ assert_eq!(9, buf.len());
+ assert_eq!(&[1u8, 1u8, 0, 0, 0, 0, 0, 0, 0], buf.as_slice());
+ }
+
+ #[test]
+ fn mutable_extend_from_slice_unaligned_u64() {
+ let mut buf = MutableBuffer::new(16);
+ buf.extend_from_slice(&[1_u8]);
+ buf.extend_from_slice(&[1_u64]);
+ dbg!(&buf.as_slice());
Review comment:
nit: is this needed?
--
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