From ed1837af40d013f0358ca87c08ef442bea9b0c58 Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Thu, 31 Aug 2023 11:05:17 -0400 Subject: [PATCH 01/10] MINOR: fix cuda crossbow build --- cpp/src/arrow/gpu/cuda_context.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/gpu/cuda_context.cc b/cpp/src/arrow/gpu/cuda_context.cc index fb8935319ed1c..90ba0a53c9f8e 100644 --- a/cpp/src/arrow/gpu/cuda_context.cc +++ b/cpp/src/arrow/gpu/cuda_context.cc @@ -323,7 +323,7 @@ Status CudaDevice::Stream::WaitEvent(const Device::SyncEvent& event) { ContextSaver set_temporary(reinterpret_cast(context_.get()->handle())); CU_RETURN_NOT_OK("cuStreamWaitEvent", - cuStreamWaitEvent(value(), cu_event, CU_EVENT_WAIT_DEFAULT)); + cuStreamWaitEvent(value(), cu_event, CU_EVENT_DEFAULT)); return Status::OK(); } From 6626ee691d98860e6abb6fec951e79772d062963 Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Thu, 31 Aug 2023 11:21:56 -0400 Subject: [PATCH 02/10] add a comment explaining the default flag --- cpp/src/arrow/gpu/cuda_context.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/gpu/cuda_context.cc b/cpp/src/arrow/gpu/cuda_context.cc index 90ba0a53c9f8e..4b2c6658223a2 100644 --- a/cpp/src/arrow/gpu/cuda_context.cc +++ b/cpp/src/arrow/gpu/cuda_context.cc @@ -322,8 +322,10 @@ Status CudaDevice::Stream::WaitEvent(const Device::SyncEvent& event) { } ContextSaver set_temporary(reinterpret_cast(context_.get()->handle())); + // we are currently building with CUDA toolkit 11.0.3 which doesn't have enum + // values for the flags yet. The "flags" param *must* be 0 for now. CU_RETURN_NOT_OK("cuStreamWaitEvent", - cuStreamWaitEvent(value(), cu_event, CU_EVENT_DEFAULT)); + cuStreamWaitEvent(value(), cu_event, 0)); return Status::OK(); } From e62f2c6753dcd6acaf2b026a51e3af858c36422e Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Thu, 31 Aug 2023 11:30:31 -0400 Subject: [PATCH 03/10] linting --- cpp/src/arrow/gpu/cuda_context.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cpp/src/arrow/gpu/cuda_context.cc b/cpp/src/arrow/gpu/cuda_context.cc index 4b2c6658223a2..851fb08dcd616 100644 --- a/cpp/src/arrow/gpu/cuda_context.cc +++ b/cpp/src/arrow/gpu/cuda_context.cc @@ -324,8 +324,7 @@ Status CudaDevice::Stream::WaitEvent(const Device::SyncEvent& event) { ContextSaver set_temporary(reinterpret_cast(context_.get()->handle())); // we are currently building with CUDA toolkit 11.0.3 which doesn't have enum // values for the flags yet. The "flags" param *must* be 0 for now. - CU_RETURN_NOT_OK("cuStreamWaitEvent", - cuStreamWaitEvent(value(), cu_event, 0)); + CU_RETURN_NOT_OK("cuStreamWaitEvent", cuStreamWaitEvent(value(), cu_event, 0)); return Status::OK(); } From 65d8df87dcd6ffb778618a036bc8855d3a95679a Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Thu, 31 Aug 2023 12:29:24 -0400 Subject: [PATCH 04/10] remove RecordWithFlags --- cpp/src/arrow/c/bridge_test.cc | 2 +- cpp/src/arrow/device.h | 6 ++---- cpp/src/arrow/gpu/cuda_context.cc | 5 ++--- cpp/src/arrow/gpu/cuda_context.h | 2 +- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/cpp/src/arrow/c/bridge_test.cc b/cpp/src/arrow/c/bridge_test.cc index 2aedccc965c45..bbf5060bae278 100644 --- a/cpp/src/arrow/c/bridge_test.cc +++ b/cpp/src/arrow/c/bridge_test.cc @@ -1222,7 +1222,7 @@ class MyDevice : public Device { virtual ~MySyncEvent() = default; Status Wait() override { return Status::OK(); } - Status Record(const Device::Stream&, const unsigned int) override { + Status Record(const Device::Stream&) override { return Status::OK(); } }; diff --git a/cpp/src/arrow/device.h b/cpp/src/arrow/device.h index 066ca7e32a4fe..a67c05f3d5f2b 100644 --- a/cpp/src/arrow/device.h +++ b/cpp/src/arrow/device.h @@ -164,13 +164,11 @@ class ARROW_EXPORT Device : public std::enable_shared_from_this, void* get_raw() { return sync_event_.get(); } /// @brief Block until sync event is completed. - virtual Status Wait() = 0; - - inline Status Record(const Stream& st) { return Record(st, 0); } + virtual Status Wait() = 0; /// @brief Record the wrapped event on the stream so it triggers /// the event when the stream gets to that point in its queue. - virtual Status Record(const Stream&, const unsigned int flags) = 0; + virtual Status Record(const Stream&) = 0; protected: /// If creating this with a passed in event, the caller must ensure diff --git a/cpp/src/arrow/gpu/cuda_context.cc b/cpp/src/arrow/gpu/cuda_context.cc index 851fb08dcd616..81542d339bd70 100644 --- a/cpp/src/arrow/gpu/cuda_context.cc +++ b/cpp/src/arrow/gpu/cuda_context.cc @@ -340,15 +340,14 @@ Status CudaDevice::SyncEvent::Wait() { return Status::OK(); } -Status CudaDevice::SyncEvent::Record(const Device::Stream& st, const unsigned int flags) { +Status CudaDevice::SyncEvent::Record(const Device::Stream& st) { auto cuda_stream = checked_cast(&st); if (!cuda_stream) { return Status::Invalid("CudaDevice::Event cannot record on non-cuda stream"); } ContextSaver set_temporary(reinterpret_cast(context_.get()->handle())); - CU_RETURN_NOT_OK("cuEventRecordWithFlags", - cuEventRecordWithFlags(value(), cuda_stream->value(), flags)); + CU_RETURN_NOT_OK("cuEventRecord", cuEventRecord(value(), cuda_stream->value())); return Status::OK(); } diff --git a/cpp/src/arrow/gpu/cuda_context.h b/cpp/src/arrow/gpu/cuda_context.h index e4d8482855f6b..56c4f3203524f 100644 --- a/cpp/src/arrow/gpu/cuda_context.h +++ b/cpp/src/arrow/gpu/cuda_context.h @@ -212,7 +212,7 @@ class ARROW_EXPORT CudaDevice : public Device { /// /// Once the stream completes the tasks previously added to it, /// it will trigger the event. - Status Record(const Device::Stream&, const unsigned int) override; + Status Record(const Device::Stream&) override; protected: friend class CudaMemoryManager; From 3a3f3fdcd4af47feddafc06ae13f68d1d6a6bf3f Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Thu, 31 Aug 2023 12:38:10 -0400 Subject: [PATCH 05/10] more lint --- cpp/src/arrow/c/bridge_test.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cpp/src/arrow/c/bridge_test.cc b/cpp/src/arrow/c/bridge_test.cc index bbf5060bae278..bd0e498a9f332 100644 --- a/cpp/src/arrow/c/bridge_test.cc +++ b/cpp/src/arrow/c/bridge_test.cc @@ -1222,9 +1222,7 @@ class MyDevice : public Device { virtual ~MySyncEvent() = default; Status Wait() override { return Status::OK(); } - Status Record(const Device::Stream&) override { - return Status::OK(); - } + Status Record(const Device::Stream&) override { return Status::OK(); } }; protected: From 7be0ea1a90768d3d42331e8a3037369b369bd72b Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Thu, 31 Aug 2023 13:16:48 -0400 Subject: [PATCH 06/10] more linting whitespace --- cpp/src/arrow/device.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/device.h b/cpp/src/arrow/device.h index a67c05f3d5f2b..efb0a5ab400a1 100644 --- a/cpp/src/arrow/device.h +++ b/cpp/src/arrow/device.h @@ -164,7 +164,7 @@ class ARROW_EXPORT Device : public std::enable_shared_from_this, void* get_raw() { return sync_event_.get(); } /// @brief Block until sync event is completed. - virtual Status Wait() = 0; + virtual Status Wait() = 0; /// @brief Record the wrapped event on the stream so it triggers /// the event when the stream gets to that point in its queue. From 4e6835b46f17f32d33d0099a731cc90104a556e7 Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Thu, 31 Aug 2023 17:03:27 -0400 Subject: [PATCH 07/10] try adding CMAKE_PREFIX_PATH --- ci/scripts/python_build.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/scripts/python_build.sh b/ci/scripts/python_build.sh index b5b5b75b9679a..16b2edb50a95e 100755 --- a/ci/scripts/python_build.sh +++ b/ci/scripts/python_build.sh @@ -70,6 +70,7 @@ export PYARROW_WITH_SUBSTRAIT=${ARROW_SUBSTRAIT:-OFF} export PYARROW_PARALLEL=${n_jobs} export LD_LIBRARY_PATH=${ARROW_HOME}/lib:${LD_LIBRARY_PATH} +export CMAKE_PREFIX_PATH=${ARROW_HOME}:${CMAKE_PREFIX_PATH} pushd ${source_dir} # - Cannot call setup.py as it may install in the wrong directory From 78ccfb9e7f6743a1b4b75d346e6aca54033f0bbf Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Fri, 1 Sep 2023 12:32:30 -0400 Subject: [PATCH 08/10] trying cuda include cmake --- cpp/src/arrow/gpu/ArrowCUDAConfig.cmake.in | 4 ++++ cpp/src/arrow/gpu/CMakeLists.txt | 8 ++++++-- cpp/src/arrow/gpu/arrow-cuda.pc.in | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/gpu/ArrowCUDAConfig.cmake.in b/cpp/src/arrow/gpu/ArrowCUDAConfig.cmake.in index bb36abf241193..626c536a08d5e 100644 --- a/cpp/src/arrow/gpu/ArrowCUDAConfig.cmake.in +++ b/cpp/src/arrow/gpu/ArrowCUDAConfig.cmake.in @@ -30,6 +30,10 @@ include(CMakeFindDependencyMacro) find_dependency(Arrow) if(CMAKE_VERSION VERSION_LESS 3.17) find_package(CUDA REQUIRED) + add_library(ArrowCUDA::cuda_driver SHARED IMPORTED) + set_target_properties(ArrowCUDA::cuda_driver + PROPERTIES IMPORTED_LOCATION "${CUDA_CUDA_LIBRARY}" + INTERFACE_INCLUDE_DIRECTORIES "${CUDA_INCLUDE_DIRS}") else() find_package(CUDAToolkit REQUIRED) endif() diff --git a/cpp/src/arrow/gpu/CMakeLists.txt b/cpp/src/arrow/gpu/CMakeLists.txt index 2556f30f0d5e6..79e82abc69c45 100644 --- a/cpp/src/arrow/gpu/CMakeLists.txt +++ b/cpp/src/arrow/gpu/CMakeLists.txt @@ -32,8 +32,11 @@ endif() set(ARROW_CUDA_LINK_LIBS arrow::flatbuffers) if(CMAKE_VERSION VERSION_LESS 3.17) find_package(CUDA REQUIRED) - set(ARROW_CUDA_SHARED_LINK_LIBS ${CUDA_CUDA_LIBRARY}) - include_directories(SYSTEM ${CUDA_INCLUDE_DIRS}) + add_library(ArrowCUDA::cuda_driver SHARED IMPORTED) + set_target_properties(ArrowCUDA::cuda_driver + PROPERTIES IMPORTED_LOCATION "${CUDA_CUDA_LIBRARY}" + INTERFACE_INCLUDE_DIRECTORIES "${CUDA_INCLUDE_DIRS}") + set(ARROW_CUDA_SHARED_LINK_LIBS ArrowCUDA::cuda_driver) else() # find_package(CUDA) is deprecated, and for newer CUDA, it doesn't # recognize that the CUDA driver library is in the "stubs" dir, but @@ -61,6 +64,7 @@ add_arrow_lib(arrow_cuda ${ARROW_CUDA_SHARED_LINK_LIBS} SHARED_INSTALL_INTERFACE_LIBS Arrow::arrow_shared + ${ARROW_CUDA_SHARED_LINK_LIBS} # Static arrow_cuda must also link against CUDA shared libs STATIC_LINK_LIBS ${ARROW_CUDA_LINK_LIBS} diff --git a/cpp/src/arrow/gpu/arrow-cuda.pc.in b/cpp/src/arrow/gpu/arrow-cuda.pc.in index 84c8c5889856e..173d7d91ef9b5 100644 --- a/cpp/src/arrow/gpu/arrow-cuda.pc.in +++ b/cpp/src/arrow/gpu/arrow-cuda.pc.in @@ -22,6 +22,6 @@ libdir=@ARROW_PKG_CONFIG_LIBDIR@ Name: Apache Arrow CUDA Description: CUDA integration library for Apache Arrow Version: @ARROW_VERSION@ -Requires: arrow +Requires: arrow cuda Libs: -L${libdir} -larrow_cuda Cflags: -I${includedir} From 89408bfa175464c9700a1b4f5f31c0c57458e75c Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Fri, 1 Sep 2023 13:02:04 -0400 Subject: [PATCH 09/10] lint cmake --- cpp/src/arrow/gpu/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/gpu/CMakeLists.txt b/cpp/src/arrow/gpu/CMakeLists.txt index 79e82abc69c45..a5b176793e495 100644 --- a/cpp/src/arrow/gpu/CMakeLists.txt +++ b/cpp/src/arrow/gpu/CMakeLists.txt @@ -33,7 +33,7 @@ set(ARROW_CUDA_LINK_LIBS arrow::flatbuffers) if(CMAKE_VERSION VERSION_LESS 3.17) find_package(CUDA REQUIRED) add_library(ArrowCUDA::cuda_driver SHARED IMPORTED) - set_target_properties(ArrowCUDA::cuda_driver + set_target_properties(ArrowCUDA::cuda_driver PROPERTIES IMPORTED_LOCATION "${CUDA_CUDA_LIBRARY}" INTERFACE_INCLUDE_DIRECTORIES "${CUDA_INCLUDE_DIRS}") set(ARROW_CUDA_SHARED_LINK_LIBS ArrowCUDA::cuda_driver) From f204bab47afd9ec6128b46c7a4373ba3f7f6b27f Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Tue, 5 Sep 2023 10:28:48 -0400 Subject: [PATCH 10/10] remove cmake prefix path from python build --- ci/scripts/python_build.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/ci/scripts/python_build.sh b/ci/scripts/python_build.sh index 16b2edb50a95e..b5b5b75b9679a 100755 --- a/ci/scripts/python_build.sh +++ b/ci/scripts/python_build.sh @@ -70,7 +70,6 @@ export PYARROW_WITH_SUBSTRAIT=${ARROW_SUBSTRAIT:-OFF} export PYARROW_PARALLEL=${n_jobs} export LD_LIBRARY_PATH=${ARROW_HOME}/lib:${LD_LIBRARY_PATH} -export CMAKE_PREFIX_PATH=${ARROW_HOME}:${CMAKE_PREFIX_PATH} pushd ${source_dir} # - Cannot call setup.py as it may install in the wrong directory