Skip to content

Improvements to stream-ordered memory management - #1177

Merged
maleadt merged 2 commits into
masterfrom
tb/pool_reclaim
Oct 2, 2021
Merged

maleadt merged 2 commits into
masterfrom
tb/pool_reclaim

Conversation

@maleadt

@maleadt maleadt commented Oct 1, 2021

Copy link
Copy Markdown
Member

No description provided.

The logic was also switched around; `reserved_memory`  was intended to compute
the memory that should be kept available, not kept reserved in the pool.
Otherwise the stream-ordered pool doesn't actually release memory.
@codecov

codecov Bot commented Oct 1, 2021 •

Copy link
Copy Markdown

Codecov Report

Merging #1177 (b9704fb) into master (ade0940) will increase coverage by 0.01%.
The diff coverage is 84.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1177      +/-   ##
==========================================
+ Coverage   79.77%   79.79%   +0.01%     
==========================================
  Files         119      119              
  Lines        8239     8234       -5     
==========================================
- Hits         6573     6570       -3     
+ Misses       1666     1664       -2     
Impacted Files Coverage Δ
src/pool.jl 78.60% <84.84%> (+0.77%) ⬆️
lib/cusparse/level3.jl 74.34% <0.00%> (-0.66%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ade0940...b9704fb. Read the comment docs.

@maleadt
maleadt merged commit 2686c9e into master Oct 2, 2021
@maleadt
maleadt deleted the tb/pool_reclaim branch October 2, 2021 12:49
@maleadt maleadt added the cuda libraries Stuff about CUDA library wrappers. label Oct 15, 2021
@danielwe

danielwe commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

This PR broke copying from unified memory to regular device memory, which my code used to do. Would it be a big hassle to resuscitate this functionality, or should I plan on doing things differently in the future? Was this safe to do in past versions?

@vchuravy

vchuravy commented Dec 8, 2021

Copy link
Copy Markdown
Member

Do you have an MWE of what you are doing?

@maleadt

maleadt commented Dec 8, 2021

Copy link
Copy Markdown
Member Author

Please open a new issue with an MWE. CUDA.jl supports and tests copying to and from unified memory, so I doubt it's broken in generality.

@danielwe

danielwe commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

See #1263. I meant to post my original comment to #1176, sorry about the confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda libraries Stuff about CUDA library wrappers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants