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

[FEA] cuDF Parquet reader should use pinned memory to copy data from sysmem to GPU #6376

Closed
chirayuG-nvidia opened this issue Sep 30, 2020 · 6 comments
Labels
cuIO cuIO issue feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. Performance Performance related issue

Comments

@chirayuG-nvidia
Copy link

Is your feature request related to a problem? Please describe.
NvTabular is building asynchronous dataloader for accelerating tabular dataloading for DL framework like PyTorch and TensorFlow. The primary function of this async dataloader is prepare training data by reading from Parquet input files and preparing input tensors for training. Currently Parquet reader in cuDF uses pageable memory to copy parquet input to GPU for decompression. HostToDeviceMemcpy from pageable memory leads to a lock in CUDA context that prevents from submitting other CUDA API calls from the framework training thread. Nsight profiler image at the end illustrate the problem.

Describe the solution you'd like
Memmapped Parquet input files can be first staged on a pinned system memory before issuing H2D memcpy, this will have additional overhead of a CPU memcpy but it will be good to implement and understand the performance improvements (or regressions) from this approach.

Additional context
image

@jperez999 @benfred @EvenOldridge to help further define NVT priority for this.

@chirayuG-nvidia chirayuG-nvidia added Needs Triage Need team to review and classify feature request New feature or request labels Sep 30, 2020
@harrism harrism added cuIO cuIO issue Performance Performance related issue libcudf Affects libcudf (C++/CUDA) code. and removed Needs Triage Need team to review and classify labels Oct 1, 2020
@github-actions
Copy link

This issue has been marked rotten due to no recent activity in the past 90d. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed.

@jperez999
Copy link
Contributor

jperez999 commented Feb 18, 2021 via email

@devavret
Copy link
Contributor

We are waiting on having a pool of pinned memory in RMM.

@EvenOldridge
Copy link

@JohnZed @rjzamora Is this something we can look into?

@vuule
Copy link
Contributor

vuule commented Jan 9, 2023

Once we switch to kvikIO for device reads, data from the input file will be transfered via pinned buffers in kvikIO's pool. I assume this will address the issue since that's the bulk of data copied onto the GPU.
Expecting to start using kvikIO in next release, will update this issue once we do.

@GregoryKimball
Copy link
Contributor

I believe this is closed by #12574. Further improvements in data copying should be scoped within kvikIO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuIO cuIO issue feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. Performance Performance related issue
Projects
None yet
Development

No branches or pull requests

7 participants