Skip to content

[rocsparse] Fix memory leak of nnzsplit starting_ids in csrmv info#6595

Merged
kliegeois merged 1 commit intoROCm:developfrom
kliegeois:fix/csrmv-nnzsplit-memleak
Apr 21, 2026
Merged

[rocsparse] Fix memory leak of nnzsplit starting_ids in csrmv info#6595
kliegeois merged 1 commit intoROCm:developfrom
kliegeois:fix/csrmv-nnzsplit-memleak

Conversation

@kliegeois
Copy link
Copy Markdown
Contributor

What changed and why

_rocsparse_csrmv_info::clear() frees the adaptive and lrb sub-info device buffers but never frees nnzsplit.starting_ids, the device buffer allocated in rocsparse_csrmv_template_nnzsplit.cpp. This causes a device memory leak every time csrmv info is cleared or destroyed.

The fix adds a destructor and clear() method to _rocsparse_nnzsplit_info, matching the existing pattern used by _rocsparse_adaptive_info and _rocsparse_lrb_info, and calls this->nnzsplit.clear() from _rocsparse_csrmv_info::clear().

starting_block_ids is an interior pointer into the starting_ids allocation (&temp_buffer_j[requiredBlocks + 1]), so only starting_ids needs to be freed.

API changes / backward compatibility

None. This is an internal-only change; no public API is affected.

Performance impact

This is a memory leak fix. No performance regression is expected — the only added cost is freeing a device allocation that was previously leaked.

Testing

The existing csrmv test suite exercises the nnzsplit code path and validates functional correctness. The leak was silent (no crash or wrong result), so no new test case is added. The fix can be verified with a memory sanitizer or memstat-enabled build.

…:clear()

Add clear() and destructor to _rocsparse_nnzsplit_info (matching the
existing pattern for adaptive_info and lrb_info), and call
this->nnzsplit.clear() from _rocsparse_csrmv_info::clear().

The starting_ids buffer allocated via rocsparse_hipMallocAsync in
csrmv_template_nnzsplit was never freed when clear() was called.
starting_block_ids points into the same allocation, so only one
free is needed.
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.47059% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...brary/src/level2/rocsparse_csrmv_nnzsplit_info.cpp 75.00% 0 Missing and 4 partials ⚠️

❌ Your project status has failed because the head coverage (47.50%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6595      +/-   ##
===========================================
+ Coverage    66.79%   66.82%   +0.03%     
===========================================
  Files         2255     2256       +1     
  Lines       325081   324970     -111     
  Branches     41532    41510      -22     
===========================================
+ Hits        217132   217145      +13     
+ Misses       93289    93161     -128     
- Partials     14660    14664       +4     
Flag Coverage Δ *Carryforward flag
hipBLAS 90.65% <ø> (ø) Carriedforward from 8e7e068
hipBLASLt 40.00% <ø> (ø) Carriedforward from 8e7e068
hipCUB 82.21% <ø> (ø) Carriedforward from 8e7e068
hipDNN 82.40% <ø> (ø) Carriedforward from 8e7e068
hipFFT 55.00% <ø> (ø) Carriedforward from 8e7e068
hipRAND 76.12% <ø> (ø) Carriedforward from 8e7e068
hipSPARSE 84.70% <ø> (ø) Carriedforward from 8e7e068
rocBLAS 48.11% <ø> (ø) Carriedforward from 8e7e068
rocFFT 47.50% <ø> (ø) Carriedforward from 8e7e068
rocPRIM 38.96% <ø> (ø) Carriedforward from 8e7e068
rocRAND 57.09% <ø> (ø) Carriedforward from 8e7e068
rocSPARSE 71.65% <76.47%> (+0.12%) ⬆️
rocThrust 91.37% <ø> (ø) Carriedforward from 8e7e068

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...parse/library/src/include/rocsparse_csrmv_info.hpp 100.00% <100.00%> (ø)
...brary/src/level2/rocsparse_csrmv_nnzsplit_info.cpp 75.00% <75.00%> (ø)

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kliegeois kliegeois merged commit 3c45717 into ROCm:develop Apr 21, 2026
19 of 21 checks passed
@kliegeois kliegeois deleted the fix/csrmv-nnzsplit-memleak branch April 21, 2026 12:42
assistant-librarian Bot pushed a commit to ROCm/rocSPARSE that referenced this pull request Apr 21, 2026
[rocsparse] Fix memory leak of nnzsplit starting_ids in csrmv
 info (#6595)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## What changed and why

`_rocsparse_csrmv_info::clear()` frees the `adaptive` and `lrb` sub-info
device buffers but never frees `nnzsplit.starting_ids`, the device
buffer allocated in `rocsparse_csrmv_template_nnzsplit.cpp`. This causes
a device memory leak every time csrmv info is cleared or destroyed.

The fix adds a destructor and `clear()` method to
`_rocsparse_nnzsplit_info`, matching the existing pattern used by
`_rocsparse_adaptive_info` and `_rocsparse_lrb_info`, and calls
`this->nnzsplit.clear()` from `_rocsparse_csrmv_info::clear()`.

`starting_block_ids` is an interior pointer into the `starting_ids`
allocation (`&temp_buffer_j[requiredBlocks + 1]`), so only
`starting_ids` needs to be freed.

## API changes / backward compatibility

None. This is an internal-only change; no public API is affected.

## Performance impact

This is a memory leak fix. No performance regression is expected — the
only added cost is freeing a device allocation that was previously
leaked.

## Testing

The existing csrmv test suite exercises the nnzsplit code path and
validates functional correctness. The leak was silent (no crash or wrong
result), so no new test case is added. The fix can be verified with a
memory sanitizer or memstat-enabled build.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants