From dddeb120d0cf8fc33f7f1a07149221fdb2a29e7a Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Thu, 11 Jul 2024 16:25:12 -0700 Subject: [PATCH] Fix ArrowDeviceArray interface to pass address of event (#16058) the `sync_event` member of `ArrowDeviceArray` needs to be a pointer to a `cudaEvent_t`, currently we're returning the `cudaEvent_t` directly. We need to be passing the address of the event. Thankfully this is a single line change, plus adding a test to confirm. Authors: - Matt Topol (https://github.com/zeroshade) Approvers: - David Wendt (https://github.com/davidwendt) - Mike Wilson (https://github.com/hyperbolic2346) URL: https://github.com/rapidsai/cudf/pull/16058 --- cpp/src/interop/to_arrow_device.cu | 2 +- cpp/tests/interop/to_arrow_device_test.cpp | 26 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/cpp/src/interop/to_arrow_device.cu b/cpp/src/interop/to_arrow_device.cu index ebfd6605977..b9d3a59e647 100644 --- a/cpp/src/interop/to_arrow_device.cu +++ b/cpp/src/interop/to_arrow_device.cu @@ -603,7 +603,7 @@ unique_device_array_t create_device_array(nanoarrow::UniqueArray&& out, }); result->device_id = rmm::get_current_cuda_device().value(); result->device_type = ARROW_DEVICE_CUDA; - result->sync_event = private_data->sync_event; + result->sync_event = &private_data->sync_event; result->array = private_data->parent; // makes a shallow copy result->array.private_data = private_data.release(); result->array.release = &detail::ArrowDeviceArrayRelease; diff --git a/cpp/tests/interop/to_arrow_device_test.cpp b/cpp/tests/interop/to_arrow_device_test.cpp index 860544b8606..8903f09b82b 100644 --- a/cpp/tests/interop/to_arrow_device_test.cpp +++ b/cpp/tests/interop/to_arrow_device_test.cpp @@ -352,11 +352,15 @@ TEST_F(ToArrowDeviceTest, EmptyTable) auto got_arrow_device = cudf::to_arrow_device(table->view()); EXPECT_EQ(rmm::get_current_cuda_device().value(), got_arrow_device->device_id); EXPECT_EQ(ARROW_DEVICE_CUDA, got_arrow_device->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_device->sync_event))); compare_arrays(schema.get(), arr.get(), &got_arrow_device->array); got_arrow_device = cudf::to_arrow_device(std::move(*table)); EXPECT_EQ(rmm::get_current_cuda_device().value(), got_arrow_device->device_id); EXPECT_EQ(ARROW_DEVICE_CUDA, got_arrow_device->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_device->sync_event))); compare_arrays(schema.get(), arr.get(), &got_arrow_device->array); } @@ -386,6 +390,8 @@ TEST_F(ToArrowDeviceTest, DateTimeTable) auto got_arrow_array = cudf::to_arrow_device(input.view()); EXPECT_EQ(rmm::get_current_cuda_device().value(), got_arrow_array->device_id); EXPECT_EQ(ARROW_DEVICE_CUDA, got_arrow_array->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_array->sync_event))); EXPECT_EQ(data.size(), got_arrow_array->array.length); EXPECT_EQ(0, got_arrow_array->array.null_count); @@ -402,6 +408,8 @@ TEST_F(ToArrowDeviceTest, DateTimeTable) got_arrow_array = cudf::to_arrow_device(std::move(input)); EXPECT_EQ(rmm::get_current_cuda_device().value(), got_arrow_array->device_id); EXPECT_EQ(ARROW_DEVICE_CUDA, got_arrow_array->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_array->sync_event))); EXPECT_EQ(data.size(), got_arrow_array->array.length); EXPECT_EQ(0, got_arrow_array->array.null_count); @@ -456,6 +464,8 @@ TYPED_TEST(ToArrowDeviceTestDurationsTest, DurationTable) auto got_arrow_array = cudf::to_arrow_device(input.view()); EXPECT_EQ(rmm::get_current_cuda_device().value(), got_arrow_array->device_id); EXPECT_EQ(ARROW_DEVICE_CUDA, got_arrow_array->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_array->sync_event))); EXPECT_EQ(data.size(), got_arrow_array->array.length); EXPECT_EQ(0, got_arrow_array->array.null_count); @@ -472,6 +482,8 @@ TYPED_TEST(ToArrowDeviceTestDurationsTest, DurationTable) got_arrow_array = cudf::to_arrow_device(std::move(input)); EXPECT_EQ(rmm::get_current_cuda_device().value(), got_arrow_array->device_id); EXPECT_EQ(ARROW_DEVICE_CUDA, got_arrow_array->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_array->sync_event))); EXPECT_EQ(data.size(), got_arrow_array->array.length); EXPECT_EQ(0, got_arrow_array->array.null_count); @@ -538,6 +550,8 @@ TEST_F(ToArrowDeviceTest, NestedList) auto got_arrow_array = cudf::to_arrow_device(input.view()); EXPECT_EQ(rmm::get_current_cuda_device().value(), got_arrow_array->device_id); EXPECT_EQ(ARROW_DEVICE_CUDA, got_arrow_array->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_array->sync_event))); compare_arrays(expected_schema.get(), expected_array.get(), &got_arrow_array->array); got_arrow_array = cudf::to_arrow_device(std::move(input)); @@ -682,11 +696,15 @@ TEST_F(ToArrowDeviceTest, StructColumn) auto got_arrow_array = cudf::to_arrow_device(input.view()); EXPECT_EQ(rmm::get_current_cuda_device().value(), got_arrow_array->device_id); EXPECT_EQ(ARROW_DEVICE_CUDA, got_arrow_array->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_array->sync_event))); compare_arrays(expected_schema.get(), expected_array.get(), &got_arrow_array->array); got_arrow_array = cudf::to_arrow_device(std::move(input)); EXPECT_EQ(rmm::get_current_cuda_device().value(), got_arrow_array->device_id); EXPECT_EQ(ARROW_DEVICE_CUDA, got_arrow_array->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_array->sync_event))); compare_arrays(expected_schema.get(), expected_array.get(), &got_arrow_array->array); } @@ -755,11 +773,15 @@ TEST_F(ToArrowDeviceTest, FixedPoint64Table) auto got_arrow_array = cudf::to_arrow_device(input.view()); ASSERT_EQ(rmm::get_current_cuda_device().value(), got_arrow_array->device_id); ASSERT_EQ(ARROW_DEVICE_CUDA, got_arrow_array->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_array->sync_event))); compare_arrays(expected_schema.get(), expected_array.get(), &got_arrow_array->array); got_arrow_array = cudf::to_arrow_device(std::move(input)); ASSERT_EQ(rmm::get_current_cuda_device().value(), got_arrow_array->device_id); ASSERT_EQ(ARROW_DEVICE_CUDA, got_arrow_array->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_array->sync_event))); compare_arrays(expected_schema.get(), expected_array.get(), &got_arrow_array->array); } } @@ -802,11 +824,15 @@ TEST_F(ToArrowDeviceTest, FixedPoint128Table) auto got_arrow_array = cudf::to_arrow_device(input.view()); EXPECT_EQ(rmm::get_current_cuda_device().value(), got_arrow_array->device_id); EXPECT_EQ(ARROW_DEVICE_CUDA, got_arrow_array->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_array->sync_event))); compare_arrays(expected_schema.get(), expected_array.get(), &got_arrow_array->array); got_arrow_array = cudf::to_arrow_device(std::move(input)); EXPECT_EQ(rmm::get_current_cuda_device().value(), got_arrow_array->device_id); EXPECT_EQ(ARROW_DEVICE_CUDA, got_arrow_array->device_type); + ASSERT_CUDA_SUCCEEDED( + cudaEventSynchronize(*reinterpret_cast(got_arrow_array->sync_event))); compare_arrays(expected_schema.get(), expected_array.get(), &got_arrow_array->array); } }