Skip to content

Commit

Permalink
Fix ArrowDeviceArray interface to pass address of event (#16058)
Browse files Browse the repository at this point in the history 
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: #16058
  • Loading branch information
@zeroshade
zeroshade authored Jul 11, 2024
1 parent cd2d53b commit dddeb12
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
2 changes: 1 addition & 1 deletion cpp/src/interop/to_arrow_device.cu
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
26 changes: 26 additions & 0 deletions cpp/tests/interop/to_arrow_device_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<cudaEvent_t*>(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<cudaEvent_t*>(got_arrow_device->sync_event)));
compare_arrays(schema.get(), arr.get(), &got_arrow_device->array);
}

Expand Down Expand Up @@ -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<cudaEvent_t*>(got_arrow_array->sync_event)));

EXPECT_EQ(data.size(), got_arrow_array->array.length);
EXPECT_EQ(0, got_arrow_array->array.null_count);
Expand All @@ -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<cudaEvent_t*>(got_arrow_array->sync_event)));

EXPECT_EQ(data.size(), got_arrow_array->array.length);
EXPECT_EQ(0, got_arrow_array->array.null_count);
Expand Down Expand Up @@ -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<cudaEvent_t*>(got_arrow_array->sync_event)));

EXPECT_EQ(data.size(), got_arrow_array->array.length);
EXPECT_EQ(0, got_arrow_array->array.null_count);
Expand All @@ -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<cudaEvent_t*>(got_arrow_array->sync_event)));

EXPECT_EQ(data.size(), got_arrow_array->array.length);
EXPECT_EQ(0, got_arrow_array->array.null_count);
Expand Down Expand Up @@ -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<cudaEvent_t*>(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));
Expand Down Expand Up @@ -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<cudaEvent_t*>(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<cudaEvent_t*>(got_arrow_array->sync_event)));
compare_arrays(expected_schema.get(), expected_array.get(), &got_arrow_array->array);
}

Expand Down Expand Up @@ -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<cudaEvent_t*>(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<cudaEvent_t*>(got_arrow_array->sync_event)));
compare_arrays(expected_schema.get(), expected_array.get(), &got_arrow_array->array);
}
}
Expand Down Expand Up @@ -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<cudaEvent_t*>(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<cudaEvent_t*>(got_arrow_array->sync_event)));
compare_arrays(expected_schema.get(), expected_array.get(), &got_arrow_array->array);
}
}

0 comments on commit dddeb12

Please sign in to comment.