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

SyncedMemory::set_cpu_data and set_gpu_data #5822

Closed
dongfangduoshou123 opened this issue Aug 4, 2017 · 2 comments
Closed

SyncedMemory::set_cpu_data and set_gpu_data #5822

dongfangduoshou123 opened this issue Aug 4, 2017 · 2 comments
Labels

Comments

@dongfangduoshou123
Copy link

dongfangduoshou123 commented Aug 4, 2017

void SyncedMemory::set_cpu_data(void* data) {
  check_device();
  CHECK(data);
  if (own_cpu_data_) {
    CaffeFreeHost(cpu_ptr_, cpu_malloc_use_cuda_);
  }
  cpu_ptr_ = data;
  head_ = HEAD_AT_CPU;
  own_cpu_data_ = false;
}
void SyncedMemory::set_gpu_data(void* data) {
  check_device();
#ifndef CPU_ONLY
  CHECK(data);
  if (own_gpu_data_) {
    CUDA_CHECK(cudaFree(gpu_ptr_));
  }
  gpu_ptr_ = data;
  head_ = HEAD_AT_GPU;
  own_gpu_data_ = false;
#else
  NO_GPU;
#endif
}

if before call set_cpu_data(set_gpu_data), the head_ = SYNCED which means both gpu memory and cpu memory has mallced, so after call set_cpu_data(set_gpu_data), from the definition we could see the original mallced cpu(gpu) memory is set free, and assign a new data point, but the original gpu(cpu) memory has not be free, is this will cause memory leak?

in addition, l set_cpu_data(set_gpu_data) just transmit the pointer, we don't know the new memory block size, is this a problem?

@zhewang95
Copy link

zhewang95 commented Dec 30, 2017

I think set_cpu_data just means set the host(cpu side) memory, It has no need to deal with the device(gpu side) memory. The device memory will not be wasted because to_gpu(defined in syncedmem.cpp) can memcopy the data from cpu_ptr_ to gpu_ptr_.

@Noiredd Noiredd changed the title SyncedMemory::set_cpu_data and set_gpu_data SyncedMemory::set_cpu_data and set_gpu_data Feb 5, 2018
@Noiredd
Copy link
Member

Noiredd commented Feb 5, 2018

There will be no memory leak - SyncedMemory frees its pointers upon its own destruction. When you feed it a new pointer (to memory you allocated manually), it takes ownership of it - it makes no difference whether the memory has been allocated by the object itself or by you. But when you replace a pointer with your own (set_&pu_data), it has to free the existing memory to avoid the leak.

@Noiredd Noiredd closed this as completed Feb 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants