feat: Add Meson support in nanoarrow_device #484

WillAyd · 2024-05-23T20:49:24Z

No description provided.

WillAyd · 2024-05-23T20:50:01Z

extensions/nanoarrow_device/src/nanoarrow/meson.build

+
+nanoarrow_dep = dependency('nanoarrow')
+
+apple_dep = dependency('appleframeworks',


I am not able to test the apple or cuda deps unfortunately

I can give this a try tomorrow for both!

WillAyd · 2024-06-11T20:58:11Z

ci/scripts/build-with-meson.sh

@@ -66,7 +66,7 @@ function main() {
    pushd "${SANDBOX_DIR}"

    show_header "Run test suite"
-    meson configure -Dtests=true -Db_coverage=true
+    meson configure -Dtests=true -Db_coverage=true -Dipc=true -Ddevice=true


Missed this on the IPC change. If we want to expand this job in the future would need to accept these options dynamically, but this should work for now

paleolimbot

This looks great!

When I run:

meson configure -Dtests=true -Ddevice=true -Dmetal=true

I get

../src/nanoarrow/nanoarrow_device_metal_test.cc:22:10: fatal error: 'Metal/Metal.hpp' file not found
#include <Metal/Metal.hpp>
         ^~~~~~~~~~~~~~~~~
../src/nanoarrow/nanoarrow_device_metal_test.cc:22:10: note: did not find header 'Metal.hpp' in framework 'Metal' (loaded from '/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks')

Metal C++ is its own thing and is a sort of header-only wrapper around the Objective C (not included directly in the framework):

arrow-nanoarrow/CMakeLists.txt

Lines 389 to 400 in d6eb52b

    
           if(NANOARROW_DEVICE_WITH_METAL) 
        
             if(NOT EXISTS "${CMAKE_BINARY_DIR}/metal-cpp") 
        
               message(STATUS "Fetching metal-cpp") 
        
               file(DOWNLOAD 
        
                    "https://developer.apple.com/metal/cpp/files/metal-cpp_macOS12_iOS15.zip" 
        
                    "${CMAKE_BINARY_DIR}/metal-cpp.zip") 
        
               file(ARCHIVE_EXTRACT 
        
                    INPUT 
        
                    ${CMAKE_BINARY_DIR}/metal-cpp.zip 
        
                    DESTINATION 
        
                    ${CMAKE_BINARY_DIR}) 
        
             endif()

I also tried

meson configure -Dtests=true -Ddevice=true -Dcuda=true

...and get

c++  -o src/nanoarrow/nanoarrow-device-cuda-test src/nanoarrow/nanoarrow-device-cuda-test.p/nanoarrow_device_cuda_test.cc.o src/nanoarrow/nanoarrow-device-cuda-test.p/.._.._subprojects_googletest-1.14.0_googletest_src_gtest-all.cc.o src/nanoarrow/nanoarrow-device-cuda-test.p/.._.._subprojects_googletest-1.14.0_googletest_src_gtest_main.cc.o -Wl,--as-needed -Wl,--no-undefined -Wl,-O1 '-Wl,-rpath,$ORIGIN/' -Wl,-rpath-link,/home/dewey/rscratch/arrow-nanoarrow/builddir/src/nanoarrow -Wl,--start-group src/nanoarrow/libnanoarrow_device.so src/nanoarrow/libnanoarrow.so -pthread /usr/local/cuda/lib64/libcudart_static.a -lrt -lpthread -ldl /usr/local/cuda/lib64/libcublas.so -Wl,--end-group
/usr/bin/ld: src/nanoarrow/nanoarrow-device-cuda-test.p/nanoarrow_device_cuda_test.cc.o: in function `NanoarrowDeviceCuda_DeviceCudaBufferCopy_Test::TestBody()':
nanoarrow_device_cuda_test.cc:(.text+0x1ec2): undefined reference to `ArrowDeviceCuda'
/usr/bin/ld: nanoarrow_device_cuda_test.cc:(.text+0x1f34): undefined reference to `cuDeviceGet'

...and this is maybe because cublas is not the right thing to link to (the thing we're using is called the "driver" API, which is not the "runtime" API). Also possibly because there needs to be a -DNANOARROW_WITH_CUDA compile define to get the ArrowDeviceCuda symbol into nanoarrow_device.so (still workshopping the final version of that).

In the next week or two we'll have a CUDA runner that will help with this 🙂

WillAyd · 2024-06-13T01:22:19Z

Sweet thanks for giving this a shot

Metal C++ is its own thing and is a sort of header-only wrapper around the Objective C (not included directly in the framework):

Possibly, though I would still hope Meson could handle that. I found another project in the WrapDB that uses Metal:

https://github.com/mesonbuild/wrapdb/blob/2d9ed3ad5bed879d8aa010de21ae165f6cf5ce4d/subprojects/packagefiles/imgui/meson.build#L49

Maybe we just need to add required: get_option('metal') to our dependency call?

...and this is maybe because cublas is not the right thing to link to (the thing we're using is called the "driver" API, which is not the "runtime" API). Also possibly because there needs to be a -DNANOARROW_WITH_CUDA compile define to get the ArrowDeviceCuda symbol into nanoarrow_device.so (still workshopping the final version of that).

Ah OK...apologies as I know embarassingly little about CUDA. But in the configuration now I have modules: [cublas] as I just copy-pasted that from the Meson documentation without much thought. Maybe that needs to be modules: [cuda-driver]?

WillAyd · 2024-06-13T01:25:09Z

Ah ignore my comment above about Metal - I confused required: get_option('metal') for something else. The only other difference I see in that case is the AppKit framework inclusion, but that seems unlikely to be the root cause.

I'll ask on the Meson IRC channel tomorrow how to resolve that header; they are usually really responsive to questions like this

WillAyd · 2024-06-13T18:01:45Z

Actually was able to borrow a MacBook to test out the metal stuff. Didn't realize metal-cpp was not installed with the system framework :-)

So the metal stuff should be compiling now. There were also some issues with the source files that needed to be corrected. Note that the metal test fails - I haven't looked into why but things at least compile now:

1/1 nanoarrow-device-metal        FAIL            0.03s   killed by signal 11 SIGSEGV
>>> MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MALLOC_PERTURB_=159 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 /Users/willayd/clones/arrow-nanoarrow/builddir/src/nanoarrow/nanoarrow-device-metal-test
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀  ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
Running main() from /tmp/googletest-20230817-5025-15wtgyq/googletest-1.14.0/googletest/src/gtest_main.cc
[==========] Running 10 tests from 2 test suites.
[----------] Global test environment set-up.
[----------] 6 tests from NanoarrowDeviceMetal
[ RUN      ] NanoarrowDeviceMetal.DefaultDevice
../src/nanoarrow/nanoarrow_device_metal_test.cc:30: Failure
Expected equality of these values:
  ArrowDeviceMetalInitDefaultDevice(device.get(), nullptr)
    Which is: 22
  0

[  FAILED  ] NanoarrowDeviceMetal.DefaultDevice (15 ms)
[ RUN      ] NanoarrowDeviceMetal.DeviceGpuBufferInit

paleolimbot · 2024-06-13T18:32:44Z

Actually was able to borrow a MacBook to test out the metal stuff

Thanks! I can also help here since I have the setup. I wonder if you have an Intel MacBook (which wouldn't have a metal default device). (Metal's default device is also available on the MacOS CI runner if you use macOS-latest, since those are all M1s).

WillAyd · 2024-06-13T18:37:11Z

src/nanoarrow/meson.build

+  endif
+
+  if get_option('cuda')
+    cuda_dep = dependency('cuda', modules: ['cuda_driver'])


Just guessing on the module name. Hoping it matches what CMake has (and what exists in our current CMake config)

https://cmake.org/cmake/help/latest/module/FindCUDAToolkit.html

I tried a number of things here, but ultimately was unable to make the linking work properly. We need libcuda.so, which I couldn't find in the Meson CUDA sources. It's the "driver" library/API, which the cudart would also need to link; however, I think the Meson CUDA stuff is almost 100% designed for nvcc, and we're using regular gcc (i.e., we're not actually using NVIDIA's compiler, we're using a regular compiler and linking to the library).

It's fine to error here for now until somebody with both Meson experience and a graphics card feels like debugging this!

WillAyd · 2024-06-13T18:41:33Z

Yea its an old Intel device - so is the metal-cpp header only required for that?

paleolimbot · 2024-06-13T19:43:56Z

is the metal-cpp header only required for that?

It will build without error because the framework is installed; however, it won't run because there is no GPU on an intel mac (i.e., when you call GetDefaultDevice from Metal it will give you a nullptr). I'll give both of these a try shortly!

paleolimbot · 2024-06-14T13:25:49Z

src/nanoarrow/meson.build

+          link_with: nanoarrow_device_lib,
+          dependencies: [nanoarrow_dep, gtest_dep, metal_cpp_dep],
+      )
+      test('nanoarrow-device-metal', exc)


There also needs to be a compile define here -DNANOARROW_DEVICE_WITH_METAL (I think the error you were seeing might have happened even with this enabled if you don't have a Mac with a GPU; however, I'm seeing it also with the Meson build.

Also see #527 for a few updates to the metal build (+ adding it to CI).

paleolimbot · 2024-06-14T16:09:09Z

src/nanoarrow/meson.build

+  endif
+
+  if get_option('cuda')
+    cuda_dep = dependency('cuda', modules: ['cuda_driver'])


I tried a number of things here, but ultimately was unable to make the linking work properly. We need libcuda.so, which I couldn't find in the Meson CUDA sources. It's the "driver" library/API, which the cudart would also need to link; however, I think the Meson CUDA stuff is almost 100% designed for nvcc, and we're using regular gcc (i.e., we're not actually using NVIDIA's compiler, we're using a regular compiler and linking to the library).

It's fine to error here for now until somebody with both Meson experience and a graphics card feels like debugging this!

paleolimbot · 2024-06-14T16:09:18Z

src/nanoarrow/meson.build

+  if get_option('cuda')
+    cuda_dep = dependency('cuda', modules: ['cuda_driver'])
+    device_deps += cuda_dep
+    device_srcs += 'nanoarrow_device_cuda.cc'


Suggested change

device_srcs += 'nanoarrow_device_cuda.cc'

device_srcs += 'nanoarrow_device_cuda.c'

paleolimbot

Thank you!

WillAyd commented May 23, 2024

View reviewed changes

WillAyd mentioned this pull request Jun 3, 2024

feat: Standalone Python installer using Meson #505

Closed

feat: Meson build system for device

9c4c7ec

WillAyd force-pushed the meson-for-cuda branch from aa556c5 to 9c4c7ec Compare June 11, 2024 20:52

Add compile options to script

5f09083

WillAyd commented Jun 11, 2024

View reviewed changes

Merge branch 'main' into meson-for-cuda

4e7d10e

paleolimbot reviewed Jun 13, 2024

View reviewed changes

Metal support

4d9eb55

WillAyd added 2 commits June 13, 2024 14:18

license fix

a420705

try cuda_driver module

0fa48e8

WillAyd commented Jun 13, 2024

View reviewed changes

Merge remote-tracking branch 'upstream/main' into meson-for-cuda

c39e0c4

paleolimbot reviewed Jun 14, 2024

View reviewed changes

Metal / CUDA feedback

3470c71

paleolimbot approved these changes Jun 17, 2024

View reviewed changes

paleolimbot merged commit 1febb81 into apache:main Jun 17, 2024
33 checks passed

WillAyd deleted the meson-for-cuda branch June 17, 2024 14:16

paleolimbot added this to the nanoarrow 0.6.0 milestone Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add Meson support in nanoarrow_device #484

feat: Add Meson support in nanoarrow_device #484

WillAyd commented May 23, 2024

WillAyd May 23, 2024

paleolimbot May 24, 2024

WillAyd Jun 11, 2024

paleolimbot left a comment

WillAyd commented Jun 13, 2024

WillAyd commented Jun 13, 2024

WillAyd commented Jun 13, 2024

paleolimbot commented Jun 13, 2024

WillAyd Jun 13, 2024

paleolimbot Jun 14, 2024

WillAyd commented Jun 13, 2024

paleolimbot commented Jun 13, 2024

paleolimbot Jun 14, 2024

paleolimbot Jun 14, 2024

paleolimbot Jun 14, 2024

paleolimbot left a comment


		nanoarrow_dep = dependency('nanoarrow')

		apple_dep = dependency('appleframeworks',

	if(NANOARROW_DEVICE_WITH_METAL)
	if(NOT EXISTS "${CMAKE_BINARY_DIR}/metal-cpp")
	message(STATUS "Fetching metal-cpp")
	file(DOWNLOAD
	"https://developer.apple.com/metal/cpp/files/metal-cpp_macOS12_iOS15.zip"
	"${CMAKE_BINARY_DIR}/metal-cpp.zip")
	file(ARCHIVE_EXTRACT
	INPUT
	${CMAKE_BINARY_DIR}/metal-cpp.zip
	DESTINATION
	${CMAKE_BINARY_DIR})
	endif()

	device_srcs += 'nanoarrow_device_cuda.cc'
	device_srcs += 'nanoarrow_device_cuda.c'

feat: Add Meson support in nanoarrow_device #484

feat: Add Meson support in nanoarrow_device #484

Conversation

WillAyd commented May 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paleolimbot left a comment

Choose a reason for hiding this comment

WillAyd commented Jun 13, 2024

WillAyd commented Jun 13, 2024

WillAyd commented Jun 13, 2024

paleolimbot commented Jun 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd commented Jun 13, 2024

paleolimbot commented Jun 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paleolimbot left a comment

Choose a reason for hiding this comment