Skip to content

Conversation

@madsbk
Copy link
Member

@madsbk madsbk commented Jan 16, 2026

Add option:

    spill_to_pinned_memory
        Whether RapidsMPF should spill to pinned host memory when available,
        or use regular pageable host memory. Pinned host memory offers higher
        bandwidth and lower transfer performance for device transfers compared to
        regular pageable host memory.

@madsbk madsbk self-assigned this Jan 16, 2026
@madsbk madsbk added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jan 16, 2026
@copy-pr-bot
Copy link

copy-pr-bot bot commented Jan 16, 2026

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.

@github-actions github-actions bot added Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels Jan 16, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python Jan 16, 2026
@madsbk madsbk marked this pull request as ready for review January 18, 2026 19:25
@madsbk madsbk requested a review from a team as a code owner January 18, 2026 19:25
Copy link
Contributor

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only question was whether a boolean will be sufficient, or whether we might need to configure something about the pinned memory resource. But https://github.com/rapidsai/rapidsmpf/blob/331162c5d8eec65352a18596f3d89c2a925db5f0/cpp/include/rapidsmpf/memory/pinned_memory_resource.hpp#L82 mentions that things like a maximum size should be configured outside of the MR, so that shouldn't be needed.

Do we have any opinions on whether this should be the default? Or do we need to collect some benchmark results first?

@madsbk
Copy link
Member Author

madsbk commented Jan 20, 2026

My only question was whether a boolean will be sufficient, or whether we might need to configure something about the pinned memory resource. But https://github.com/rapidsai/rapidsmpf/blob/331162c5d8eec65352a18596f3d89c2a925db5f0/cpp/include/rapidsmpf/memory/pinned_memory_resource.hpp#L82 mentions that things like a maximum size should be configured outside of the MR, so that shouldn't be needed.

Yeah, I think a boolean should suffice for now.

Do we have any opinions on whether this should be the default? Or do we need to collect some benchmark results first?

@pentschev did some runs that showed mixed results, which might be because we don’t reuse the BufferResource between evaluate_pipeline() calls. For now, let’s disable pinned memory by default, then see if we can reuse the buffer resource in a follow-up PR. If that consistently improves performance, we can enable it by default.

@madsbk madsbk added the DO NOT MERGE Hold off on merging; see PR for details label Jan 20, 2026
@madsbk madsbk removed the DO NOT MERGE Hold off on merging; see PR for details label Jan 21, 2026
Co-authored-by: Lawrence Mitchell <[email protected]>
@madsbk
Copy link
Member Author

madsbk commented Jan 21, 2026

/merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cudf-polars Issues specific to cudf-polars improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

4 participants