Skip to content

ARROW-1850: [C++] Use void* / const void* for buffers in file APIs #1363

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.

Sign up for GitHub

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

Closed
wesm wants to merge 2 commits into from
Closed

ARROW-1850: [C++] Use void* / const void* for buffers in file APIs #1363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d5cf2d
Use void* / const void* for buffers in file APIs
wesm Nov 26, 2017
af5a348
Update glib, arrow-gpu for API changes
wesm Nov 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions c_glib/arrow-glib/input-stream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ namespace garrow {

arrow::Status Read(int64_t n_bytes,
int64_t *n_read_bytes,
uint8_t *out) override {
void *out) override {
GError *error = NULL;
*n_read_bytes = g_input_stream_read(input_stream_,
out,
Expand All @@ -437,7 +437,7 @@ namespace garrow {
}

arrow::Status ReadAt(int64_t position, int64_t n_bytes,
int64_t *n_read_bytes, uint8_t* out) override {
int64_t *n_read_bytes, void* out) override {
return arrow::io::RandomAccessFile::ReadAt(
position, n_bytes, n_read_bytes, out);
}
Expand Down
4 changes: 2 additions & 2 deletions c_glib/arrow-glib/output-stream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ garrow_output_stream_file_interface_init(GArrowFileInterface *iface)
iface->get_raw = garrow_output_stream_get_raw_file_interface;
}

static std::shared_ptr<arrow::io::Writeable>
static std::shared_ptr<arrow::io::Writable>
garrow_output_stream_get_raw_writeable_interface(GArrowWriteable *writeable)
{
auto output_stream = GARROW_OUTPUT_STREAM(writeable);
Expand Down Expand Up @@ -325,7 +325,7 @@ namespace garrow {
return arrow::Status::OK();
}

arrow::Status Write(const uint8_t *data,
arrow::Status Write(const void *data,
int64_t n_bytes) override {
GError *error = NULL;
gsize n_written_bytes;
Expand Down
2 changes: 1 addition & 1 deletion c_glib/arrow-glib/writeable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ garrow_writeable_flush(GArrowWriteable *writeable,

G_END_DECLS

std::shared_ptr<arrow::io::Writeable>
std::shared_ptr<arrow::io::Writable>
garrow_writeable_get_raw(GArrowWriteable *writeable)
{
auto *iface = GARROW_WRITEABLE_GET_IFACE(writeable);
Expand Down
6 changes: 3 additions & 3 deletions c_glib/arrow-glib/writeable.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@
/**
* GArrowWriteableInterface:
*
* It wraps `arrow::io::Writeable`.
* It wraps `arrow::io::Writable`.
*/
struct _GArrowWriteableInterface
{
GTypeInterface parent_iface;

std::shared_ptr<arrow::io::Writeable> (*get_raw)(GArrowWriteable *file);
std::shared_ptr<arrow::io::Writable> (*get_raw)(GArrowWriteable *file);
};

std::shared_ptr<arrow::io::Writeable> garrow_writeable_get_raw(GArrowWriteable *writeable);
std::shared_ptr<arrow::io::Writable> garrow_writeable_get_raw(GArrowWriteable *writeable);
2 changes: 1 addition & 1 deletion cpp/src/arrow/gpu/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ configure_file(cuda_version.h.in
@ONLY)

install(FILES
"${CMAKE_CURRENT_SOURCE_DIR}/cuda_version.h"
"${CMAKE_CURRENT_BINARY_DIR}/cuda_version.h"
DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/arrow/gpu")

install(FILES
Expand Down
23 changes: 11 additions & 12 deletions cpp/src/arrow/gpu/cuda_context.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,28 +69,27 @@ class CudaContext::CudaContextImpl {
return Status::OK();
}

Status CopyHostToDevice(uint8_t* dst, const uint8_t* src, int64_t nbytes) {
Status CopyHostToDevice(void* dst, const void* src, int64_t nbytes) {
CU_RETURN_NOT_OK(cuCtxSetCurrent(context_));
CU_RETURN_NOT_OK(cuMemcpyHtoD(reinterpret_cast<CUdeviceptr>(dst),
reinterpret_cast<const void*>(src),
CU_RETURN_NOT_OK(cuMemcpyHtoD(reinterpret_cast<CUdeviceptr>(dst), src,
static_cast<size_t>(nbytes)));
return Status::OK();
}

Status CopyDeviceToHost(uint8_t* dst, const uint8_t* src, int64_t nbytes) {
Status CopyDeviceToHost(void* dst, const void* src, int64_t nbytes) {
CU_RETURN_NOT_OK(cuCtxSetCurrent(context_));
CU_RETURN_NOT_OK(cuMemcpyDtoH(dst, reinterpret_cast<const CUdeviceptr>(src),
static_cast<size_t>(nbytes)));
return Status::OK();
}

Status Free(uint8_t* device_ptr, int64_t nbytes) {
Status Free(void* device_ptr, int64_t nbytes) {
CU_RETURN_NOT_OK(cuMemFree(reinterpret_cast<CUdeviceptr>(device_ptr)));
bytes_allocated_ -= nbytes;
return Status::OK();
}

Status ExportIpcBuffer(uint8_t* data, std::unique_ptr<CudaIpcMemHandle>* handle) {
Status ExportIpcBuffer(void* data, std::unique_ptr<CudaIpcMemHandle>* handle) {
CU_RETURN_NOT_OK(cuCtxSetCurrent(context_));
CUipcMemHandle cu_handle;
CU_RETURN_NOT_OK(cuIpcGetMemHandle(&cu_handle, reinterpret_cast<CUdeviceptr>(data)));
Expand Down Expand Up @@ -145,7 +144,7 @@ class CudaDeviceManager::CudaDeviceManagerImpl {
return Status::OK();
}

Status FreeHost(uint8_t* data, int64_t nbytes) {
Status FreeHost(void* data, int64_t nbytes) {
CU_RETURN_NOT_OK(cuMemFreeHost(data));
host_bytes_allocated_ -= nbytes;
return Status::OK();
Expand Down Expand Up @@ -221,7 +220,7 @@ Status CudaDeviceManager::AllocateHost(int64_t nbytes,
return Status::OK();
}

Status CudaDeviceManager::FreeHost(uint8_t* data, int64_t nbytes) {
Status CudaDeviceManager::FreeHost(void* data, int64_t nbytes) {
return impl_->FreeHost(data, nbytes);
}

Expand All @@ -241,22 +240,22 @@ Status CudaContext::Allocate(int64_t nbytes, std::shared_ptr<CudaBuffer>* out) {
return Status::OK();
}

Status CudaContext::ExportIpcBuffer(uint8_t* data,
Status CudaContext::ExportIpcBuffer(void* data,
std::unique_ptr<CudaIpcMemHandle>* handle) {
return impl_->ExportIpcBuffer(data, handle);
}

Status CudaContext::CopyHostToDevice(uint8_t* dst, const uint8_t* src, int64_t nbytes) {
Status CudaContext::CopyHostToDevice(void* dst, const void* src, int64_t nbytes) {
return impl_->CopyHostToDevice(dst, src, nbytes);
}

Status CudaContext::CopyDeviceToHost(uint8_t* dst, const uint8_t* src, int64_t nbytes) {
Status CudaContext::CopyDeviceToHost(void* dst, const void* src, int64_t nbytes) {
return impl_->CopyDeviceToHost(dst, src, nbytes);
}

Status CudaContext::Close() { return impl_->Close(); }

Status CudaContext::Free(uint8_t* device_ptr, int64_t nbytes) {
Status CudaContext::Free(void* device_ptr, int64_t nbytes) {
return impl_->Free(device_ptr, nbytes);
}

Expand Down
10 changes: 5 additions & 5 deletions cpp/src/arrow/gpu/cuda_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class ARROW_EXPORT CudaDeviceManager {

Status AllocateHost(int64_t nbytes, std::shared_ptr<CudaHostBuffer>* buffer);

Status FreeHost(uint8_t* data, int64_t nbytes);
Status FreeHost(void* data, int64_t nbytes);

int num_devices() const;

Expand Down Expand Up @@ -88,10 +88,10 @@ class ARROW_EXPORT CudaContext : public std::enable_shared_from_this<CudaContext
private:
CudaContext();

Status ExportIpcBuffer(uint8_t* data, std::unique_ptr<CudaIpcMemHandle>* handle);
Status CopyHostToDevice(uint8_t* dst, const uint8_t* src, int64_t nbytes);
Status CopyDeviceToHost(uint8_t* dst, const uint8_t* src, int64_t nbytes);
Status Free(uint8_t* device_ptr, int64_t nbytes);
Status ExportIpcBuffer(void* data, std::unique_ptr<CudaIpcMemHandle>* handle);
Status CopyHostToDevice(void* dst, const void* src, int64_t nbytes);
Status CopyDeviceToHost(void* dst, const void* src, int64_t nbytes);
Status Free(void* device_ptr, int64_t nbytes);

class CudaContextImpl;
std::unique_ptr<CudaContextImpl> impl_;
Expand Down
14 changes: 7 additions & 7 deletions cpp/src/arrow/gpu/cuda_memory.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ CudaBuffer::CudaBuffer(const std::shared_ptr<CudaBuffer>& parent, const int64_t
is_ipc_(false) {}

Status CudaBuffer::CopyToHost(const int64_t position, const int64_t nbytes,
uint8_t* out) const {
void* out) const {
return context_->CopyDeviceToHost(out, data_ + position, nbytes);
}

Status CudaBuffer::CopyFromHost(const int64_t position, const uint8_t* data,
Status CudaBuffer::CopyFromHost(const int64_t position, const void* data,
int64_t nbytes) {
DCHECK_LE(nbytes, size_ - position) << "Copy would overflow buffer";
return context_->CopyHostToDevice(mutable_data_ + position, data, nbytes);
Expand Down Expand Up @@ -134,7 +134,7 @@ CudaBufferReader::CudaBufferReader(const std::shared_ptr<CudaBuffer>& buffer)

CudaBufferReader::~CudaBufferReader() {}

Status CudaBufferReader::Read(int64_t nbytes, int64_t* bytes_read, uint8_t* buffer) {
Status CudaBufferReader::Read(int64_t nbytes, int64_t* bytes_read, void* buffer) {
nbytes = std::min(nbytes, size_ - position_);
*bytes_read = nbytes;
RETURN_NOT_OK(context_->CopyDeviceToHost(buffer, data_ + position_, nbytes));
Expand Down Expand Up @@ -190,7 +190,7 @@ class CudaBufferWriter::CudaBufferWriterImpl {
return Status::OK();
}

Status Write(const uint8_t* data, int64_t nbytes) {
Status Write(const void* data, int64_t nbytes) {
if (nbytes == 0) {
return Status::OK();
}
Expand All @@ -214,7 +214,7 @@ class CudaBufferWriter::CudaBufferWriterImpl {
return Status::OK();
}

Status WriteAt(int64_t position, const uint8_t* data, int64_t nbytes) {
Status WriteAt(int64_t position, const void* data, int64_t nbytes) {
std::lock_guard<std::mutex> guard(lock_);
RETURN_NOT_OK(Seek(position));
return Write(data, nbytes);
Expand Down Expand Up @@ -269,11 +269,11 @@ Status CudaBufferWriter::Seek(int64_t position) {

Status CudaBufferWriter::Tell(int64_t* position) const { return impl_->Tell(position); }

Status CudaBufferWriter::Write(const uint8_t* data, int64_t nbytes) {
Status CudaBufferWriter::Write(const void* data, int64_t nbytes) {
return impl_->Write(data, nbytes);
}

Status CudaBufferWriter::WriteAt(int64_t position, const uint8_t* data, int64_t nbytes) {
Status CudaBufferWriter::WriteAt(int64_t position, const void* data, int64_t nbytes) {
return impl_->WriteAt(position, data, nbytes);
}

Expand Down
10 changes: 5 additions & 5 deletions cpp/src/arrow/gpu/cuda_memory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ class ARROW_EXPORT CudaBuffer : public Buffer {
/// \brief Copy memory from GPU device to CPU host
/// \param[out] out a pre-allocated output buffer
/// \return Status
Status CopyToHost(const int64_t position, const int64_t nbytes, uint8_t* out) const;
Status CopyToHost(const int64_t position, const int64_t nbytes, void* out) const;

/// \brief Copy memory to device at position
/// \param[in] position start position to copy bytes
/// \param[in] data the host data to copy
/// \param[in] nbytes number of bytes to copy
/// \return Status
Status CopyFromHost(const int64_t position, const uint8_t* data, int64_t nbytes);
Status CopyFromHost(const int64_t position, const void* data, int64_t nbytes);

/// \brief Expose this device buffer as IPC memory which can be used in other processes
/// \param[out] handle the exported IPC handle
Expand Down Expand Up @@ -130,7 +130,7 @@ class ARROW_EXPORT CudaBufferReader : public io::BufferReader {
/// \param[in] nbytes number of bytes to read
/// \param[out] bytes_read actual number of bytes read
/// \param[out] buffer pre-allocated memory to write into
Status Read(int64_t nbytes, int64_t* bytes_read, uint8_t* buffer) override;
Status Read(int64_t nbytes, int64_t* bytes_read, void* buffer) override;

/// \brief Zero-copy read from device memory
/// \param[in] nbytes number of bytes to read
Expand Down Expand Up @@ -158,9 +158,9 @@ class ARROW_EXPORT CudaBufferWriter : public io::WriteableFile {

Status Seek(int64_t position) override;

Status Write(const uint8_t* data, int64_t nbytes) override;
Status Write(const void* data, int64_t nbytes) override;

Status WriteAt(int64_t position, const uint8_t* data, int64_t nbytes) override;
Status WriteAt(int64_t position, const void* data, int64_t nbytes) override;

Status Tell(int64_t* position) const override;

Expand Down
26 changes: 13 additions & 13 deletions cpp/src/arrow/io/file.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,11 +394,11 @@ class OSFile {
return Status::OK();
}

Status Read(int64_t nbytes, int64_t* bytes_read, uint8_t* out) {
return FileRead(fd_, out, nbytes, bytes_read);
Status Read(int64_t nbytes, int64_t* bytes_read, void* out) {
return FileRead(fd_, reinterpret_cast<uint8_t*>(out), nbytes, bytes_read);
}

Status ReadAt(int64_t position, int64_t nbytes, int64_t* bytes_read, uint8_t* out) {
Status ReadAt(int64_t position, int64_t nbytes, int64_t* bytes_read, void* out) {
std::lock_guard<std::mutex> guard(lock_);
RETURN_NOT_OK(Seek(position));
return Read(nbytes, bytes_read, out);
Expand All @@ -413,12 +413,12 @@ class OSFile {

Status Tell(int64_t* pos) const { return FileTell(fd_, pos); }

Status Write(const uint8_t* data, int64_t length) {
Status Write(const void* data, int64_t length) {
std::lock_guard<std::mutex> guard(lock_);
if (length < 0) {
return Status::IOError("Length must be non-negative");
}
return FileWrite(fd_, data, length);
return FileWrite(fd_, reinterpret_cast<const uint8_t*>(data), length);
}

int fd() const { return fd_; }
Expand Down Expand Up @@ -504,13 +504,13 @@ Status ReadableFile::Close() { return impl_->Close(); }

Status ReadableFile::Tell(int64_t* pos) const { return impl_->Tell(pos); }

Status ReadableFile::Read(int64_t nbytes, int64_t* bytes_read, uint8_t* out) {
Status ReadableFile::Read(int64_t nbytes, int64_t* bytes_read, void* out) {
std::lock_guard<std::mutex> guard(impl_->lock());
return impl_->Read(nbytes, bytes_read, out);
}

Status ReadableFile::ReadAt(int64_t position, int64_t nbytes, int64_t* bytes_read,
uint8_t* out) {
void* out) {
return impl_->ReadAt(position, nbytes, bytes_read, out);
}

Expand Down Expand Up @@ -570,7 +570,7 @@ Status FileOutputStream::Close() { return impl_->Close(); }

Status FileOutputStream::Tell(int64_t* pos) const { return impl_->Tell(pos); }

Status FileOutputStream::Write(const uint8_t* data, int64_t length) {
Status FileOutputStream::Write(const void* data, int64_t length) {
return impl_->Write(data, length);
}

Expand Down Expand Up @@ -710,7 +710,7 @@ Status MemoryMappedFile::Close() {
return Status::OK();
}

Status MemoryMappedFile::Read(int64_t nbytes, int64_t* bytes_read, uint8_t* out) {
Status MemoryMappedFile::Read(int64_t nbytes, int64_t* bytes_read, void* out) {
nbytes = std::max<int64_t>(
0, std::min(nbytes, memory_map_->size() - memory_map_->position()));
if (nbytes > 0) {
Expand All @@ -735,7 +735,7 @@ Status MemoryMappedFile::Read(int64_t nbytes, std::shared_ptr<Buffer>* out) {
}

Status MemoryMappedFile::ReadAt(int64_t position, int64_t nbytes, int64_t* bytes_read,
uint8_t* out) {
void* out) {
std::lock_guard<std::mutex> guard(memory_map_->lock());
RETURN_NOT_OK(Seek(position));
return Read(nbytes, bytes_read, out);
Expand All @@ -750,7 +750,7 @@ Status MemoryMappedFile::ReadAt(int64_t position, int64_t nbytes,

bool MemoryMappedFile::supports_zero_copy() const { return true; }

Status MemoryMappedFile::WriteAt(int64_t position, const uint8_t* data, int64_t nbytes) {
Status MemoryMappedFile::WriteAt(int64_t position, const void* data, int64_t nbytes) {
std::lock_guard<std::mutex> guard(memory_map_->lock());

if (!memory_map_->opened() || !memory_map_->writable()) {
Expand All @@ -761,7 +761,7 @@ Status MemoryMappedFile::WriteAt(int64_t position, const uint8_t* data, int64_t
return WriteInternal(data, nbytes);
}

Status MemoryMappedFile::Write(const uint8_t* data, int64_t nbytes) {
Status MemoryMappedFile::Write(const void* data, int64_t nbytes) {
std::lock_guard<std::mutex> guard(memory_map_->lock());

if (!memory_map_->opened() || !memory_map_->writable()) {
Expand All @@ -773,7 +773,7 @@ Status MemoryMappedFile::Write(const uint8_t* data, int64_t nbytes) {
return WriteInternal(data, nbytes);
}

Status MemoryMappedFile::WriteInternal(const uint8_t* data, int64_t nbytes) {
Status MemoryMappedFile::WriteInternal(const void* data, int64_t nbytes) {
memcpy(memory_map_->head(), data, static_cast<size_t>(nbytes));
memory_map_->advance(nbytes);
return Status::OK();
Expand Down
Loading