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 2020/11/07 21:35:42 UTC

[GitHub] [arrow] vertexclique opened a new pull request #8609: ARROW-10513: [Rust]: ARMv7 compilation

vertexclique opened a new pull request #8609:
URL: https://github.com/apache/arrow/pull/8609


   todos:
   - [x] gnu abi hf
   - [ ] gnu abi
   - [x] musl abi hf
   - [ ] musl abi
   - [ ] start ci for armv7
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] vertexclique commented on a change in pull request #8609: ARROW-10513: [Rust]: ARMv7 compilation

Posted by GitBox <gi...@apache.org>.
vertexclique commented on a change in pull request #8609:
URL: https://github.com/apache/arrow/pull/8609#discussion_r520177140



##########
File path: rust/arrow/src/array/union.rs
##########
@@ -698,7 +699,11 @@ mod tests {
             4 * 8 * 4 * mem::size_of::<i32>(),
             union.get_buffer_memory_size()
         );
-        let internals_of_union_array = (8 + 72) + (union.boxed_fields.len() * 144); // Arc<ArrayData> & Vec<ArrayRef> combined.
+        let tagged_pointer_size = POINTER_WIDTH / 4 + POINTER_WIDTH;
+        let internals_of_union_array = tagged_pointer_size

Review comment:
       Please do. Calculated nestedness and pointer table from my mind. Just tried to write a little bit more clearly which part of the pointers accounts for which part of the arrays with this pr.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] vertexclique commented on pull request #8609: ARROW-10513: [Rust]: ARMv7 compilation

Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #8609:
URL: https://github.com/apache/arrow/pull/8609#issuecomment-731752132


   @nevi-me enabled ci, let's see how it will go.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] vertexclique commented on pull request #8609: ARROW-10513: [Rust]: ARMv7 compilation

Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #8609:
URL: https://github.com/apache/arrow/pull/8609#issuecomment-759442736


   @alamb Thanks for reaching out! I don't have time to work on these PRs. Closing.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] jhorstmann commented on a change in pull request #8609: ARROW-10513: [Rust]: ARMv7 compilation

Posted by GitBox <gi...@apache.org>.
jhorstmann commented on a change in pull request #8609:
URL: https://github.com/apache/arrow/pull/8609#discussion_r523458622



##########
File path: rust/arrow/src/memory.rs
##########
@@ -22,6 +22,14 @@ use std::alloc::Layout;
 use std::mem::align_of;
 use std::ptr::NonNull;
 
+/// Target pointer width of the targeted platform
+#[cfg(target_pointer_width = "64")]
+pub(crate) const POINTER_WIDTH: usize = 64;

Review comment:
       If this is in bits, how is it related to the alignment check below? It also makes the usage in the size calculations a little confusing since that needs to convert to bytes.
   
   Just an idea, could this be defined without the `cfg` attributes, something like `std::mem::size_of::<*const u8>()`?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] vertexclique commented on a change in pull request #8609: ARROW-10513: [Rust]: ARMv7 compilation

Posted by GitBox <gi...@apache.org>.
vertexclique commented on a change in pull request #8609:
URL: https://github.com/apache/arrow/pull/8609#discussion_r523467239



##########
File path: rust/arrow/src/memory.rs
##########
@@ -22,6 +22,14 @@ use std::alloc::Layout;
 use std::mem::align_of;
 use std::ptr::NonNull;
 
+/// Target pointer width of the targeted platform
+#[cfg(target_pointer_width = "64")]
+pub(crate) const POINTER_WIDTH: usize = 64;

Review comment:
       I don't know what you are referring, there is nothing wrong with the implementation. 
   std::mem::size_of::<*const u8>() is not for target pointer size and doesn't give that.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] vertexclique commented on pull request #8609: ARROW-10513: [Rust]: ARMv7 compilation

Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #8609:
URL: https://github.com/apache/arrow/pull/8609#issuecomment-730071003


   Yup, we can do that. I will remove all bit slice iterator implementation from this PR and make it only to work on arm LE. Then we can use it.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] nevi-me commented on a change in pull request #8609: ARROW-10513: [Rust]: ARMv7 compilation

Posted by GitBox <gi...@apache.org>.
nevi-me commented on a change in pull request #8609:
URL: https://github.com/apache/arrow/pull/8609#discussion_r520092924



##########
File path: rust/arrow/src/array/union.rs
##########
@@ -698,7 +699,11 @@ mod tests {
             4 * 8 * 4 * mem::size_of::<i32>(),
             union.get_buffer_memory_size()
         );
-        let internals_of_union_array = (8 + 72) + (union.boxed_fields.len() * 144); // Arc<ArrayData> & Vec<ArrayRef> combined.
+        let tagged_pointer_size = POINTER_WIDTH / 4 + POINTER_WIDTH;
+        let internals_of_union_array = tagged_pointer_size

Review comment:
       I struggled to follow the arithmetic here, but perhaps I need to look at the spec again




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] github-actions[bot] commented on pull request #8609: ARROW-10513: [Rust]: ARMv7 compilation

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8609:
URL: https://github.com/apache/arrow/pull/8609#issuecomment-723499160


   https://issues.apache.org/jira/browse/ARROW-10513


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] alamb commented on pull request #8609: ARROW-10513: [Rust]: ARMv7 compilation

Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #8609:
URL: https://github.com/apache/arrow/pull/8609#issuecomment-759438487


   @vertexclique  --  Given the imminent Arrow 3.0 release, I am trying to clean up older Rust PRs and see if the authors have plans to move them forward. 
   
   Do you plan on working on this PR in the near future? If not, should we close this PR until there is time to make progress? Thanks again for your contributions so far. 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] nevi-me commented on pull request #8609: ARROW-10513: [Rust]: ARMv7 compilation

Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #8609:
URL: https://github.com/apache/arrow/pull/8609#issuecomment-730068705


   @vertexclique would we be able to use https://crates.io/crates/cross for the armv7 CI?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] vertexclique closed pull request #8609: ARROW-10513: [Rust]: ARMv7 compilation

Posted by GitBox <gi...@apache.org>.
vertexclique closed pull request #8609:
URL: https://github.com/apache/arrow/pull/8609


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org