toddfarmer / arrow-migration

0 stars 1 forks source link

[C++] Add BufferAllocator abstract interface #1412

Closed toddfarmer closed 4 years ago

toddfarmer commented 7 years ago

Note: This issue was originally created as ARROW-1470. Please see the migration documentation for further details.

Original Issue Description:

There are some situations (arrow::ipc::SerializeRecordBatch where we pass a MemoryPool* solely to call AllocateBuffer using it. This is not as flexible as it could be, since there are situation where we may wish to allocate from shared memory instead.

So instead:

Func(..., BufferAllocator* allocator, ...) {
  ...
  std::shared_ptr<Buffer> buffer;
  RETURN_NOT_OK(allocator->Allocate(nbytes, &buffer));
  ...
}
toddfarmer commented 7 years ago

Note: Comment by Wes McKinney (wesm): Moving this to 0.8.0. I would like to give this some more thoughtful consideration in a patch than adding it at last minute

toddfarmer commented 6 years ago

Note: Comment by Antoine Pitrou (apitrou): Could you explain why this couldn't be solved by having a shared memory-allocating MemoryPool subclass?

toddfarmer commented 6 years ago

Note: Comment by Wes McKinney (wesm): Yes, these APIs assume that we are allocating CPU memory:

https://github.com/apache/arrow/blob/master/cpp/src/arrow/buffer.cc#L164

That detail is sort of "baked in". A BufferAllocator abstract interface would allow other kinds of buffers to be returned

toddfarmer commented 6 years ago

Note: Comment by Wes McKinney (wesm): I'd like to take a look at this for 0.11

toddfarmer commented 4 years ago

Note: Comment by Antoine Pitrou (apitrou): I believe the functionality is now provided by the Device and MemoryManager abstractions, so closing.