arrow icon indicating copy to clipboard operation
arrow copied to clipboard

ARROW-17836: [C++] Allow specifying alignment of buffers

Open save-buffer opened this issue 2 years ago • 7 comments

save-buffer avatar Sep 24 '22 02:09 save-buffer

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

github-actions[bot] avatar Sep 24 '22 02:09 github-actions[bot]

:warning: Ticket has no components in JIRA, make sure you assign one.

github-actions[bot] avatar Sep 24 '22 02:09 github-actions[bot]

:warning: Ticket has not been started in JIRA, please click 'Start Progress'.

github-actions[bot] avatar Sep 24 '22 02:09 github-actions[bot]

Seems like the only CI issue is

`actual$x`:   "2018-10-07T19:04:05" NA
[24011](https://github.com/apache/arrow/actions/runs/3147572706/jobs/5117455486#step:11:24012)
`expected$x`: "2018-10-07 19:04:05" NA
[24012](https://github.com/apache/arrow/actions/runs/3147572706/jobs/5117455486#step:11:24013)

I see this issue on some other PRs and also I don't touch this test at all in my code, so I think this is fine.

save-buffer avatar Sep 29 '22 19:09 save-buffer

Rebasing, the R errors are gone but the scanner test seems to be failing:

terminate called after throwing an instance of 'std::system_error'
[263](https://github.com/apache/arrow/actions/runs/3176058214/jobs/5174896904#step:11:264)
  what():  Resource temporarily unavailable
[264](https://github.com/apache/arrow/actions/runs/3176058214/jobs/5174896904#step:11:265)
terminate called recursively
[265](https://github.com/apache/arrow/actions/runs/3176058214/jobs/5174896904#step:11:266)

Again, probably not me. I'd guess related to Weston's scanner refactor.

save-buffer avatar Oct 03 '22 18:10 save-buffer

@westonpace ^^

pitrou avatar Oct 03 '22 19:10 pitrou

Also opened https://issues.apache.org/jira/browse/ARROW-17927 for a similar issue on scanner tests.

pitrou avatar Oct 04 '22 10:10 pitrou

@save-buffer I added some tests for the random array generator and it turns out that list arrays may not end up aligned due to ListArray::FromArrays. That can be for a later PR though. What do you think?

pitrou avatar Nov 24 '22 16:11 pitrou

@github-actions crossbow submit -g cpp

pitrou avatar Nov 24 '22 16:11 pitrou

Revision: 3948cb21109ce715fd83fb762f63833805f8e009

Submitted crossbow builds: ursacomputing/crossbow @ actions-0ee166c579

Task Status
test-alpine-linux-cpp Github Actions
test-build-cpp-fuzz Github Actions
test-conda-cpp Github Actions
test-conda-cpp-valgrind Azure
test-cuda-cpp Github Actions
test-debian-10-cpp-amd64 Github Actions
test-debian-10-cpp-i386 Github Actions
test-debian-11-cpp-amd64 Github Actions
test-debian-11-cpp-i386 Github Actions
test-fedora-35-cpp Github Actions
test-ubuntu-18.04-cpp Github Actions
test-ubuntu-18.04-cpp-release Github Actions
test-ubuntu-18.04-cpp-static Github Actions
test-ubuntu-20.04-cpp Github Actions
test-ubuntu-20.04-cpp-20 Github Actions
test-ubuntu-20.04-cpp-bundled Github Actions
test-ubuntu-20.04-cpp-thread-sanitizer Github Actions
test-ubuntu-22.04-cpp Github Actions

github-actions[bot] avatar Nov 24 '22 16:11 github-actions[bot]

Thanks for adding those, yes I agree this PR is big enough for now. Adding support for Lists the issue is that the buffers passed in may not be aligned, right? This may be more convenient to do after we have a utility that will reallocate/copy to a specific alignment if needed (I think Sanjiban is working on something like that?)

save-buffer avatar Nov 25 '22 07:11 save-buffer

Thanks for adding those, yes I agree this PR is big enough for now. Adding support for Lists the issue is that the buffers passed in may not be aligned, right? This may be more convenient to do after we have a utility that will reallocate/copy to a specific alignment if needed (I think Sanjiban is working on something like that?)

Yes, I already have that utility ready. I am waiting for this PR to get merged and I will then make a PR for ensuring alignment.

sanjibansg avatar Nov 25 '22 08:11 sanjibansg

Adding support for Lists the issue is that the buffers passed in may not be aligned, right?

No, it's just that random generation for lists calls ListArray::FromArrays, which can allocate new buffers in some cases, and that method doesn't take the alignment parameter yet.

pitrou avatar Nov 25 '22 08:11 pitrou

Benchmark runs are scheduled for baseline = 63f013cdb36d05f6f96a145aff3c6232470f2d02 and contender = 2078af7c710d688c14313b9486b99c981550a7b7. 2078af7c710d688c14313b9486b99c981550a7b7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. Conbench compare runs links: [Finished :arrow_down:0.0% :arrow_up:0.0%] ec2-t3-xlarge-us-east-2 [Finished :arrow_down:0.84% :arrow_up:0.57%] test-mac-arm [Finished :arrow_down:0.82% :arrow_up:0.0%] ursa-i9-9960x [Finished :arrow_down:1.12% :arrow_up:0.98%] ursa-thinkcentre-m75q Buildkite builds: [Finished] 2078af7c ec2-t3-xlarge-us-east-2 [Finished] 2078af7c test-mac-arm [Finished] 2078af7c ursa-i9-9960x [Finished] 2078af7c ursa-thinkcentre-m75q [Finished] 63f013cd ec2-t3-xlarge-us-east-2 [Finished] 63f013cd test-mac-arm [Finished] 63f013cd ursa-i9-9960x [Finished] 63f013cd ursa-thinkcentre-m75q Supported benchmarks: ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True test-mac-arm: Supported benchmark langs: C++, Python, R ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

ursabot avatar Nov 26 '22 18:11 ursabot