Support constructing pylibcudf Columns and Tables from views into arbitrary objects - #18314
Conversation
|
Since this change removes a copy in contiguous_split, I did a quick benchmark to confirm the expected impact, especially since this API is important for the distributed Polars engine work. Here's my (very simple) benchmark: import pyarrow as pa
import pytest
import pylibcudf as plc
@pytest.fixture
def packed_table():
pa_tbl = pa.table({
"a": range(int(1e8)),
"b": range(int(1e8)),
"c": range(int(1e8)),
})
plc_tbl = plc.interop.from_arrow(pa_tbl)
return plc.contiguous_split.pack(plc_tbl)
def bench_unpack(benchmark, packed_table):
benchmark(plc.contiguous_split.unpack, packed_table)and here are the results: # Before
---------------------------------------------- benchmark: 1 tests ----------------------------------------------
Name (time in ms) Min Max Mean StdDev Median IQR Outliers OPS Rounds Iterations
----------------------------------------------------------------------------------------------------------------
bench_unpack 8.8083 82.7581 53.5293 23.6607 46.2718 37.7695 39;0 18.6814 108 1
----------------------------------------------------------------------------------------------------------------
# After
----------------------------------------------- benchmark: 1 tests ----------------------------------------------
Name (time in us) Min Max Mean StdDev Median IQR Outliers OPS (Kops/s) Rounds Iterations
-----------------------------------------------------------------------------------------------------------------
bench_unpack 2.0800 25.2870 2.2061 0.5804 2.1600 0.0421 319;2200 453.2812 33326 1
-----------------------------------------------------------------------------------------------------------------Note that these are actually being reported in different units, ms on top and us below, so it's about 400x faster. Obviously the results are very dependent on the data size, and in this case I picked a large buffer. If I drop it down to a million instead of a hundred million rows, the "After" timings are largely unchanged as expected (since we are just viewing the buffer), while the "Before" timings look like -------------------------------------------- benchmark: 1 tests --------------------------------------------
Name (time in ms) Min Max Mean StdDev Median IQR Outliers OPS Rounds Iterations
------------------------------------------------------------------------------------------------------------
bench_unpack 1.0384 5.0173 1.1214 0.2195 1.0943 0.0596 12;28 891.7214 670 1
------------------------------------------------------------------------------------------------------------We still see a fairly large gap. For the limiting case of a single-row table, we get ------------------------------------------------- benchmark: 1 tests ------------------------------------------------
Name (time in us) Min Max Mean StdDev Median IQR Outliers OPS (Kops/s) Rounds Iterations
---------------------------------------------------------------------------------------------------------------------
bench_unpack 52.0620 122.4501 55.8294 7.2951 53.4215 1.8415 249;391 17.9117 3040 1
---------------------------------------------------------------------------------------------------------------------so there's still about 30x overhead that we get to save ourselves from avoiding the allocation steps altogether. |
bdice
left a comment
There was a problem hiding this comment.
Approving with one question on pointer types.
|
/merge |
This PR leverages #18084 to rework the Python layer of Arrow interchange. With this change, we can now expose [the Arrow capsule interfaces](https://arrow.apache.org/docs/format/CDataInterface/PyCapsuleInterface.html) for pylibcudf Columns and Tables. This PR also paves the way for exposing the device capsules, which will allow us to provide zero-copy Arrow views into pylibcudf objects. To get everything working, this PR also makes some ancillary changes: - These changes uncovered a number of places where the libcudf arrow interop code was not properly handling the NA type or 0 row columns and tables. Those cases have been fixed. - The code added in #18314 to support constructing pylibcudf Columns from a combination of a libcudf column_view and an arbitrary owner (as opposed to a Column owner) was incomplete. It worked in that PR because we don't actually do anything with Columns produced by the one use case tested there other than store the data then quickly unpack it (this was for packed columns). Using that code path with arrow columns uncovered a much bigger gap. The core issue is that gpumemoryview is constructed assuming that every object that it wraps has a CUDA Array interface. The new factory added in #18314 bypassed that, resulting in a gpumemoryview that was effectively in an invalid state for most operations. To fix this, I replaced the existing approach with a requirement that we wrap the existing owning object in something exposing a CAI before constructing the gpumemoryview. - To validate that these changes did not regress performance, I ran the Python benchmarks. In the process I added an additional `from_arrow` benchmark. - While running the benchmarks, I noticed an issue with our usage of pytest-benchmark due to dependency issues. I added a pinning in our repo for now and upstreamed fixes in conda-forge/conda-forge-repodata-patches-feedstock#990 and conda-forge/pytest-benchmark-feedstock#27. - I also addressed some of the outstanding comments from #18302 Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Bradley Dice (https://github.com/bdice) - Shruti Shivakumar (https://github.com/shrshi) - Matthew Murray (https://github.com/Matt711) - Tianyu Liu (https://github.com/kingcrimsontianyu) URL: #18402
Description
The new constructors are necessary when we need to produce views into data that is not owned by one of the standard libcudf types (column/table). The original motivating example is contiguous_split, so that API is modified in this PR for testing.
Closes #17040
Supersedes #17543
Checklist