Skip to content
This repository has been archived by the owner on Mar 12, 2021. It is now read-only.

Allocate workspace repeatedly right before calling into CUDNN. #517

Merged
merged 11 commits into from
Nov 27, 2019

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented Nov 26, 2019

This to deal with GC runs affecting CUDNN workspace size heuristics.

cc @MikeInnes

This to deal with GC runs affecting CUDNN workspace size heuristics.
@MikeInnes
Copy link
Collaborator

One other possibility is to have a CuArrays equivalent of GC.enable to just disallow memory recovery while doing these allocations.

One concern with using anonymous functions like this is that it could end up producing a lot of untyped slots, but hopefully it's simple enough to avoid that.

@maleadt
Copy link
Member Author

maleadt commented Nov 26, 2019

One other possibility is to have a CuArrays equivalent of GC.enable to just disallow memory recovery while doing these allocations.

It's only going to actually free stuff when it's really needed though (to prevent OOM), so we can't disable that. And disabling the Julia GC won't do anything, because those finalizers only put stuff back in the pool, and don't actually free (for now).

One concern with using anonymous functions like this is that it could end up producing a lot of untyped slots, but hopefully it's simple enough to avoid that.

The code looks good, fully typed and all closures inlined. Pretty bloaty still, but not significantly worse with these extra closures.

@appleparan
Copy link

Code looks okay but it seems just CI system failure. please rerun?

@appleparan
Copy link

appleparan commented Nov 26, 2019

Suddenly, I feel dangerous to have possibility of infinite loop. How about change while to for loop and break in @workspace?

@MikeInnes
Copy link
Collaborator

It could only be infinite if the required workspace increases forever. Not vouching for CUDNN but at any rate, the memory allocation will error out eventually.

@maleadt
Copy link
Member Author

maleadt commented Nov 27, 2019

Did some more work on @workspace, it has some additional features now. @MikeInnes, your fear was right and use of closures leads to badly inferred code due to the capturing issue. In this case, I think we can safely deconstruct the closure into a let block though.

@maleadt maleadt marked this pull request as ready for review November 27, 2019 10:51
@maleadt
Copy link
Member Author

maleadt commented Nov 27, 2019

This should be good to go if CI passes. Might also be the time for some feedback on the @workspace and @argout macros (cc @kshyatt, you've wrapped a lot of these), but those improvements can land in another PR since this isn't a public API.

@maleadt maleadt merged commit ea3d93b into master Nov 27, 2019
@bors bors bot deleted the tb/workspace branch November 27, 2019 13:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants