Skip to content

Added pinned pool reservation API for java - #13964

Merged
rapids-bot[bot] merged 4 commits into
NVIDIA:branch-23.10from
revans2:jni_pinned_pool_reservation
Sep 1, 2023
Merged

Added pinned pool reservation API for java#13964
rapids-bot[bot] merged 4 commits into
NVIDIA:branch-23.10from
revans2:jni_pinned_pool_reservation

Conversation

@revans2

@revans2 revans2 commented Aug 25, 2023

Copy link
Copy Markdown
Contributor

Description

This adds in an API to java to reserve pinned memory. It is very much like allocating it, but provides a way to guarantee up front that there is enough memory for multiple allocations to succeed.

Checklist

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

@revans2 revans2 added 3 - Ready for Review Ready for review by team Java Affects Java cuDF API. Spark Functionality that helps Spark RAPIDS 4 - Needs cuDF (Java) Reviewer improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 25, 2023
@revans2
revans2 requested a review from a team as a code owner August 25, 2023 16:49
@revans2 revans2 self-assigned this Aug 25, 2023

@gerashegalov gerashegalov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, nits

if (section.size >= bytes && section.size <= alignedSize) {
allocated = section;
section = null;
// Not need for reserveAllocInternal because the original section is already tracked

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Not need for reserveAllocInternal because the original section is already tracked
// No need for reserveAllocInternal because the original section is already tracked

@Override
public synchronized HostMemoryBuffer allocate(long bytes) {
if (section == null || section.size < bytes) {
throw new OutOfMemoryError("Reservation didn't have enough space for " + bytes);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would it be helpful for diagnostics to include the section=null / section.size in the exception message?

@gerashegalov gerashegalov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@revans2

revans2 commented Aug 31, 2023

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit b705c81 into NVIDIA:branch-23.10 Sep 1, 2023
@vyasr vyasr added 4 - Needs Review Waiting for reviewer to review or respond and removed 4 - Needs cuDF (Java) Reviewer labels Feb 23, 2024
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 4 - Needs Review Waiting for reviewer to review or respond improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. non-breaking Non-breaking change Spark Functionality that helps Spark RAPIDS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants