Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added pinned pool reservation API for java #13964

Merged

Conversation

revans2
Copy link
Contributor

@revans2 revans2 commented Aug 25, 2023

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
Copy link
Contributor

@gerashegalov gerashegalov left a comment

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
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
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?

Copy link
Contributor

@gerashegalov gerashegalov left a comment

Choose a reason for hiding this comment

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

LGTM

@revans2
Copy link
Contributor Author

revans2 commented Aug 31, 2023

/merge

@rapids-bot rapids-bot bot merged commit b705c81 into rapidsai: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