Hi, Thanks for the quick answers, and for the clarification wrt to the specs, Antoine.
I believe that no-one had the goal of making an architecture-depent requirement of aligned memory. A plausible explanation is that no-one tried to create a mis-aligned buffer, since, in Rust, they are allocated via the same architecture-depent alignment and thus always pass the assert. I can imagine myself placing that assert to make sure that our allocation was right, and then forgetting to remove it... One simple solution is to continue to allocate buffers with a architecture-depent alignment, as it offers speed benefits, but still allow mis-aligned buffers to be created via raw pointers. This would make it follow the spec: follow the recommendation on alloc, but do not require alignment in general. In practice, this requires removing an "assert!". One question is whether this invariant is being assumed somewhere else in our code. I assume this to be negative, as that is the only code that uses that alignment constant. As an extra data-point, I locally removed the assert and manually changed the alignment of my own architecture, and the tests continue to pass. I encapsulated this on ARROW-10039 <https://issues.apache.org/jira/browse/ARROW-10039> and I plan to follow-up with a PR. Since this is about following the specs, I placed it to target 2.0.0. Feel free to move it around if you think that this requires further discussion / is not worth targeting for the 2.0.0. Best, Jorge On Fri, Sep 18, 2020 at 6:08 PM Antoine Pitrou <anto...@python.org> wrote: > > Le 18/09/2020 à 18:03, Jorge Cardoso Leitão a écrit : > > // panics with "memory not aligned" > > Buffer::from_raw_parts(address as *const u8, size, size) > > > > I get an address such as `4624199872` (i64), but, when converted to > `*const > > u8`, our rust implementation does not consider it to be memory aligned > (at > > least in x86_64) and panics. Has anyone worked in this problem? Any hints > > on what I am doing wrong? > > The Rust implementation should not panic on misaligned buffers. 64-byte > alignment is a recommendation, not a requirement. Buffers allocated by > foreign systems such as Python / NumPy, or simply sliced on an arbitrary > offset, will not necessarily be aligned. > > > AFAIK, our rust implementation requires different alignments, depending > on > > the architecture (see memory.rs > > <https://github.com/apache/arrow/blob/master/rust/arrow/src/memory.rs>). > > I am not a Rust expert, but I remember noticing the PR where this was > proposed and explaining that this was wrong. Again: alignment is a > recommendation, not a requirement. Making it mandatory *and* > platform-specific even sounds a bit perverse... > > Regards > > Antoine. >
