Skip to content

Conversation

davidwendt
Copy link
Contributor

Description

Although a simple thrust::gather should be enough for gathering/filtering an ArrowStringView/ArrowBinaryView array, in reality it is impractical to share a data buffer between two columns in libcudf. This PR illustrates how a cudf::gather would actually function if ArrowStringView were supported by libcudf. Sharing data in column_views is expected but a libcudf gather (all other libcudf APIs) would return a new column with newly owned (non-shared) data. At best, the data-buffer could be simply copied but it seems impractical in general to not compact the buffer and remove any unused data characters from it. Also, the compaction could help coalesce accessing adjacent row data in down stream calls.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@davidwendt davidwendt self-assigned this Sep 10, 2025
@davidwendt davidwendt added 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 10, 2025
Copy link

copy-pr-bot bot commented Sep 10, 2025

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@davidwendt
Copy link
Contributor Author

The benchmark results show the ArrowStringView to be much faster for smaller strings but slower for longer strings

# sv_gather

## [0] NVIDIA RTX A6000

| num_rows | width | map_rows |  Ref Time |   Cmp Time |       Diff |   %Diff |
|----------|-------|----------|-----------|------------|------------|---------|
|   100000 |   6   |   10000  | 40.842 us |  27.291 us | -13.552 us | -33.18% |
|  1000000 |   6   |   10000  | 41.514 us |  27.536 us | -13.979 us | -33.67% |
| 10000000 |   6   |   10000  | 40.727 us |  27.569 us | -13.158 us | -32.31% |
|   100000 |  12   |   10000  | 41.814 us |  27.229 us | -14.585 us | -34.88% |
|  1000000 |  12   |   10000  | 41.960 us |  27.433 us | -14.528 us | -34.62% |
| 10000000 |  12   |   10000  | 42.806 us |  27.558 us | -15.248 us | -35.62% |
|   100000 |  24   |   10000  | 43.795 us |  36.752 us |  -7.043 us | -16.08% |
|  1000000 |  24   |   10000  | 44.003 us |  36.845 us |  -7.158 us | -16.27% |
| 10000000 |  24   |   10000  | 44.322 us |  36.892 us |  -7.430 us | -16.76% |
|   100000 |  48   |   10000  | 43.472 us |  48.486 us |   5.014 us |  11.53% |
|  1000000 |  48   |   10000  | 42.753 us |  48.509 us |   5.756 us |  13.46% |
| 10000000 |  48   |   10000  | 43.585 us |  48.198 us |   4.613 us |  10.58% |
|   100000 |  64   |   10000  | 42.204 us |  57.034 us |  14.830 us |  35.14% |
|  1000000 |  64   |   10000  | 42.848 us |  57.452 us |  14.604 us |  34.08% |
| 10000000 |  64   |   10000  | 42.886 us |  57.744 us |  14.858 us |  34.65% |
|   100000 |   6   |  100000  | 50.383 us |  30.328 us | -20.054 us | -39.80% |
|  1000000 |   6   |  100000  | 57.507 us |  31.619 us | -25.888 us | -45.02% |
| 10000000 |   6   |  100000  | 58.984 us |  33.023 us | -25.961 us | -44.01% |
|   100000 |  12   |  100000  | 58.175 us |  30.456 us | -27.719 us | -47.65% |
|  1000000 |  12   |  100000  | 60.724 us |  31.635 us | -29.089 us | -47.90% |
| 10000000 |  12   |  100000  | 63.304 us |  33.077 us | -30.227 us | -47.75% |
|   100000 |  24   |  100000  | 66.585 us |  71.662 us |   5.077 us |   7.63% |
|  1000000 |  24   |  100000  | 74.208 us |  72.078 us |  -2.130 us |  -2.87% |
| 10000000 |  24   |  100000  | 75.377 us |  72.247 us |  -3.129 us |  -4.15% |
|   100000 |  48   |  100000  | 81.797 us | 131.860 us |  50.062 us |  61.20% |
|  1000000 |  48   |  100000  | 88.715 us | 130.627 us |  41.911 us |  47.24% |
| 10000000 |  48   |  100000  | 92.774 us | 141.948 us |  49.175 us |  53.01% |
|   100000 |  64   |  100000  | 79.967 us | 173.504 us |  93.537 us | 116.97% |
|  1000000 |  64   |  100000  | 85.521 us | 174.620 us |  89.099 us | 104.18% |
| 10000000 |  64   |  100000  | 87.608 us | 175.502 us |  87.894 us | 100.33% |

@davidwendt
Copy link
Contributor Author

/ok to test

@davidwendt davidwendt marked this pull request as ready for review September 17, 2025 18:49
@davidwendt davidwendt requested a review from a team as a code owner September 17, 2025 18:49
@davidwendt davidwendt added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Sep 18, 2025
@davidwendt davidwendt changed the base branch from branch-25.10 to branch-25.12 September 26, 2025 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant