using ManagedArray::reallocate() on a default-constructed ManagedArray doesn't actually allocate at all #90
Conversation
…st case to ensure realloc on preallocated managed arrays copies over data correctly, also implement a fix for the realloc bug
| { | ||
| chai::ManagedArray<float> array; | ||
| array.reallocate(1); | ||
| float val = array[0]; |
There was a problem hiding this comment.
This causes a segfault in the current implementation, as the ManagedArray doesn't actually allocate anything during the reallocate call.
There was a problem hiding this comment.
changes look reasonable to me.
|
Note I haven't checked for all tests passing on Lassen yet -- since the machine is still down at the moment. |
| pointer_record->m_user_callback(ACTION_ALLOC, ExecutionSpace(space), sizeof(T) * elems); | ||
| void* new_ptr = m_allocators[space]->allocate(sizeof(T)*elems); | ||
| pointer_record->m_user_callback(ACTION_ALLOC, ExecutionSpace(space), sizeof(T) * elems); | ||
| void* new_ptr = m_allocators[space]->allocate(sizeof(T)*elems); |
There was a problem hiding this comment.
looks good, wait for David or Adam to approve as well.
There was a problem hiding this comment.
This will cause reallocate to allocate a pointer in every space - I'm not sure that's what we want.
There was a problem hiding this comment.
@wrtobin @robinson96 - I think this PR introduces an unexpected change in behavior, can you take a look?
There was a problem hiding this comment.
ooh - you're right David.
Bill, the model should preserve lazy allocation of device memory so that reallocations don't inadvertently initialize device memory if they haven't used gpu memory before. At one point, we had an implementation of reallocate that also strode to only do a deep copy of the data on any memory space where the data was active.
There was a problem hiding this comment.
Having reallocate always allocate pointers in all spaces changes the memory usage of reallocated arrays. @robinson96 do you agree?
| pointer_record->m_user_callback(ACTION_ALLOC, ExecutionSpace(space), sizeof(T) * elems); | ||
| void* new_ptr = m_allocators[space]->allocate(sizeof(T)*elems); | ||
| pointer_record->m_user_callback(ACTION_ALLOC, ExecutionSpace(space), sizeof(T) * elems); | ||
| void* new_ptr = m_allocators[space]->allocate(sizeof(T)*elems); |
There was a problem hiding this comment.
This will cause reallocate to allocate a pointer in every space - I'm not sure that's what we want.
No description provided.