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

Inability to select pinned memory space #102

Closed
cameronrutherford opened this issue Nov 16, 2020 · 3 comments
Closed

Inability to select pinned memory space #102

cameronrutherford opened this issue Nov 16, 2020 · 3 comments
Assignees
Labels

Comments

@cameronrutherford
Copy link
Collaborator

Selecting "pinned" memory as the memory space for hiop::LinearAlgebraFactory will crash the application. An example of why this is can be seen in the initialization of hiopMatrixRajaDense:

//hiopMatrixRajaDense.cpp
auto& resmgr = umpire::ResourceManager::getInstance();
umpire::Allocator devalloc  = resmgr.getAllocator(mem_space_);
data_dev_ = static_cast<double*>(devalloc.allocate(n_local_*max_rows_*sizeof(double)));
M_dev_    = static_cast<double**>(devalloc.allocate((max_rows_ == 0 ? 1 : max_rows_) * sizeof(double*)));
if(mem_space_ == "DEVICE")
{
  // If memory space is on device, create a host mirror
  umpire::Allocator hostalloc = resmgr.getAllocator("HOST");
  data_host_  = static_cast<double*>(hostalloc.allocate(n_local_*max_rows_*sizeof(double)));
  M_host_     = static_cast<double**>(hostalloc.allocate((max_rows_ == 0 ? 1 : max_rows_) * sizeof(double*)));
  yglob_host_ = static_cast<double*>(hostalloc.allocate(m_local_ * sizeof(double)));
  ya_host_    = static_cast<double*>(hostalloc.allocate(m_local_ * sizeof(double)));
}
else
{
  data_host_ = data_dev_;
  M_host_    = M_dev_;
  // If memory space is not on device, these buffers are allocated in memory space
  yglob_host_ = static_cast<double*>(devalloc.allocate(m_local_ * sizeof(double)));
  ya_host_    = static_cast<double*>(devalloc.allocate(m_local_ * sizeof(double)));
}

With the current code, "pinned" memory will be allocated, but there will be no equivalent memory allocation on the "device" memory space for the "pinned" memory to correspond to. In effect, "pinned" memory is treated in the same way that "um" or "host" memory spaces are treated. Instead, there should be both "pinned" and "device" memory allocated when "pinned" memory is selected, and data should be transferred between the two memory spaces as needed.

@pelesh
Copy link
Collaborator

pelesh commented Nov 17, 2020

Thanks @cameronrutherford for bringing this up. It seems this is a bug. The quick fix is to temporarily remove pinned memory from the list of user options. We haven't implemented pinned memory functionality.

@pelesh
Copy link
Collaborator

pelesh commented Dec 12, 2020

This was resolved in #153 by removing "PINNED" option altogether.

@pelesh pelesh closed this as completed Dec 12, 2020
@cameronrutherford
Copy link
Collaborator Author

Are we going to implement pinned memory in the future?

I assume another issue would probably be better for this once we have finalised the device memory implementation, as pinned memory could improve application performance.

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