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

Add alignment to cuda_malloc_async_memory_resource. #4923

Merged
merged 2 commits into from
Jun 27, 2023

Conversation

mzient
Copy link
Contributor

@mzient mzient commented Jun 23, 2023

Category:

Bug fix
New feature

Description:

cudaMallocAsync allocates memory aligned to a multiple of 256 bytes, however, DALI's memory_resource interface can be used to request overaligned memory. Before this PR, DALI would neither return properly aligned memory nor raise an error, should cudaMallocAsync return insufficiently aligned pointer.
This PR adds overalignment support by requesting larger blocks for allocations with alignment >256B and aligning the pointer accoringly. The mapping aligned->original pointer is kept (in a global state) and upon deletion, the pointer being deleted is looked up in the map and, if found, replaced with the original pointer.

Additionally, a tiny performance bug is fixed in AsyncPool tests.

Additional information:

Affected modules and functionalities:

cuda_malloc_async_memory_resource
AsyncPool tests.

Key points relevant for the review:

Tests:

  • Existing tests apply
  • New tests added Test adjusted
    • Python tests
    • GTests
    • Benchmark
    • Other
  • N/A

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
    • Doxygen
    • RST
    • Jupyter
    • Other
  • N/A

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: N/A

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [8730704]: BUILD STARTED

@JanuszL JanuszL self-assigned this Jun 23, 2023

class aligned_alloc_helper {
public:
static constexpr size_t kCudaMallocAlignment = 256;
Copy link
Contributor

@JanuszL JanuszL Jun 23, 2023

Choose a reason for hiding this comment

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

Is it given once and for all or may change depending on the version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's given - I asked about it on CUDA channel and all functions that allocate global memory return data aligned to at least 256B.

@mzient mzient force-pushed the cudaMallocAsync_align branch from 51b93c5 to f99fdc5 Compare June 23, 2023 15:13
Signed-off-by: Michał Zientkiewicz <[email protected]>
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [8731034]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [8731034]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [8731034]: BUILD PASSED

@mzient mzient merged commit bd48d14 into NVIDIA:main Jun 27, 2023
@JanuszL JanuszL mentioned this pull request Sep 6, 2023
JanuszL pushed a commit to JanuszL/DALI that referenced this pull request Oct 13, 2023
* Overallocate and align allocations with alignment > 256B
* Store a mapping from aligned to original addresses in a global map
* Update tests

Signed-off-by: Michał Zientkiewicz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants