CUDATreeLearner: free GPU memory in destructor if any allocated #4963
Conversation
denmoroz
requested review from
btrotta,
guolinke,
hzy46,
shiyu1994 and
tongwu-sh
as code owners
|
|
|
Ups, seems like some tests are not passing as they are cancelled by timeout. Trying to restart. |
|
Still getting 🤔 |
|
@jmoralez
Thanks 🙏 |
| @@ -63,6 +63,43 @@ CUDATreeLearner::CUDATreeLearner(const Config* config) | |||
| } | |||
|
|
|||
| CUDATreeLearner::~CUDATreeLearner() { | |||
| #pragma omp parallel for schedule(static, num_gpu_) | |||
There was a problem hiding this comment.
With a static schedule, and num_gpu_ chunk size, I think there will be only 1 thread being used. So I can we simply remove this line?
There was a problem hiding this comment.
@shiyu1994
To be honest I've just copy-pasted code from here which uses omp parallel, so I decided that probably it is done intentionally. My expectations were that cleanup for each GPU device will be executed in parallel for efficiency (if num_gpu_ > 1). Do you think we should actually remove this?
There was a problem hiding this comment.
After experimenting, I find that schedule(static, num_gpu_) will only result in a single thread. With num_gpu_ block size, and with num_gpu_ total iterations. So this would not parallelize the for loop.
I think it is ok to keep the omp parallel in this PR. And we can fix this with another PR. Thank you!
|
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
Fixes memory freeing issue described here: allocated GPU memory is not released after training cycle is complete, which leads to constant memory growth while using multiple train() invocations inside the same process (like hyperparameters search).