[BUG] Pinned memory allocation within Parquet reader can be very slow

[jlowe]

Describe the bug
The libcudf Parquet reader performs pinned memory allocations, and in some environments (e.g.: cloud and other virtual envs) pinned memory allocations can be expensive in practice. Here's a screenshot of an Nsight Systems trace showing the cudaHostAlloc taking 1.5 seconds on the first call. Subsequent calls are relatively cheap, likely because the OS has already spent the cost of rearranging the memory and is reusing the work.

In this case it was so slow I'm not sure the use of pinned memory was overall cost-effective vs. using paged memory directly, probably dependent upon the number of times the application calls the Parquet reader.

This use-case was with the RAPIDS Accelerator for Apache Spark which often pre-allocates a pool of pinned memory up-front when it starts. If the Parquet reader had a way of reusing the pre-allocated pinned memory pool provided by the application then this slow allocation could be avoided.

Steps/Code to reproduce bug
The cost seems to be very much dependent upon the runtime environment. I've seen it most often in cloud-like environments. I suspect it could occur in a bare-metal environment as well if the memory was filled with buffers and page-cache, and the OS needed to rearrange pages to form a physically contiguous chunk.

Expected behavior
Parquet reader should not spend excessive amounts of time allocating memory.

[jrhemstad]

This is a problem that we can solve by RMM providing a pinned memory pool and exposing hooks into libcudf to plug in a pinned pool. This way users (Spark) can plug in their own pinned memory pool, or just use the default pinned memory pool.

[abellina]

Please note that I see a behavior difference in 0.19 as compared to 0.18 regarding this.

In 0.18, I don't see the slow cudaHostAlloc at parquet read time reported here. In 0.19, I also see a cuInit at parquet read time that I do not see in 0.18, followed by that slow cudaHostAlloc. It seems we are paying the initialization cost later than we used to in 0.18.

[abellina]

Ok, I think I may have found the culprit. For libcudf.so, it looks like we are linking the cuda runtime statically. I am confused how this is happening, but If I pass -DCUDA_USE_STATIC_CUDA_RUNTIME=OFF, symbols like cuInit and other cuda symbols, go away. Additionally, the extra cuInit call and parquet slowness goes away.

A "fast" libcudf.so has no cuInit symbols for instance:

$ nm libcudf.so |grep cuInit
$ echo $?
1

A "slow" one has the symbols:

00000000036ec9a8 b _ZN6cudart12__fun_cuInitE
00000000029a2ac0 t _ZN6cudartL15__unimpl_cuInitEj

As far as I understand CUDA_USE_STATIC_CUDA_RUNTIME is specific to FindCUDA and we shouldn't be using it (we are using FindCUDAToolkit right?). But this option is removing symbols for me.

It would be good to know if others can corroborate this. This PR looks to be where this behavior changed: 61091a0.

[abellina]

One place that wants FindCUDA is ArrowCUDA (https://github.com/apache/arrow/blob/apache-arrow-1.0.1/cpp/src/arrow/gpu/CMakeLists.txt#L32). We normally statically link arrow, so we see this issue.

It seems if we set CUDA_USE_STATIC_CUDA_RUNTIME=OFF in CMakeLists.txt that we can still get that static cudart from CUDA_STATIC_RUNTIME, so that seems viable. @jrhemstad any objections? @jlowe @nvdbaranec fyi.

[kkraus14]

As far as I know ArrowCUDA only uses the CUDA driver library (https://github.com/apache/arrow/search?q=%22include+cuda%22), but it looks like their CMake erroneously links to the runtime as well 😕 (https://github.com/apache/arrow/blob/apache-arrow-1.0.1/cpp/src/arrow/gpu/CMakeLists.txt#L39-L56). I'm guessing what's happening is that because we link to Arrow before CUDA in our CMake that we inherit the link options of Arrow?

cc @trxcllnt @robertmaynard

[abellina]

@kkraus14, no this is an issue when we statically link against arrow_cuda, since it is also statically linking against cudart.

This CUDA Libraries line is printed from https://github.com/apache/arrow/blob/apache-arrow-1.0.1/cpp/src/arrow/gpu/CMakeLists.txt#L35. Thanks @jlowe for pointing out this is where the static link actually happens (https://github.com/apache/arrow/blob/apache-arrow-1.0.1/cpp/src/arrow/gpu/CMakeLists.txt#L56)

-DCUDA_USE_STATIC_CUDA_RUNTIME=OFF:

$ PARALLEL_LEVEL=12 cmake -GNinja -DCMAKE_CUDA_ARCHITECTURES="75" -DCMAKE_INSTALL_PREFIX=$CONDA_PREFIX -DARROW_STATIC_LIB=ON -DUSE_NVTX=0 -DRMM_LOGGING_LEVEL=OFF -DBUILD_BENCHMARKS=OFF -DBUILD_TESTS=OFF -DCUDF_USE_ARROW_STATIC=ON -DCUDA_USE_STATIC_CUDA_RUNTIME=OFF ..|grep CUDA\ Libraries
-- CUDA Libraries: /usr/local/cuda/lib64/libcudart.so

-DCUDA_USE_STATIC_CUDA_RUNTIME=ON (default):

$ PARALLEL_LEVEL=12 cmake -GNinja -DCMAKE_CUDA_ARCHITECTURES="75" -DCMAKE_INSTALL_PREFIX=$CONDA_PREFIX -DARROW_STATIC_LIB=ON -DUSE_NVTX=0 -DRMM_LOGGING_LEVEL=OFF -DBUILD_BENCHMARKS=OFF -DBUILD_TESTS=OFF -DCUDF_USE_ARROW_STATIC=ON -DCUDA_USE_STATIC_CUDA_RUNTIME=ON ..|grep CUDA\ Libraries
-- CUDA Libraries: /usr/local/cuda/lib64/libcudart_static.a;Threads::Threads;dl;/usr/lib/x86_64-linux-gnu/librt.so

[robertmaynard]

In 0.19 and above we forgot to specify a value for CMAKE_CUDA_RUNTIME_LIBRARY ( https://cmake.org/cmake/help/latest/variable/CMAKE_CUDA_RUNTIME_LIBRARY.html ) and therefore use the default when compiling CUDA code ( static ). Correcting this will make the cudf CUDA_STATIC_RUNTIME variable work as desired when we don't consider Arrow.

If Arrow when built statically is using the CUDA runtime statically we also have to use the runtime statically as you can't combine both the dynamic and static versions in a program.

[abellina]

@robertmaynard thanks, I just tried that but I am not sure that FindCUDA is using it. Perhaps I missed something:

-DCMAKE_CUDA_RUNTIME_LIBRARY=Shared:

$ PARALLEL_LEVEL=12 cmake -GNinja -DCMAKE_CUDA_ARCHITECTURES="75" -DCMAKE_INSTALL_PREFIX=$CONDA_PREFIX -DARROW_STATIC_LIB=ON -DUSE_NVTX=0 -DRMM_LOGGING_LEVEL=OFF -DBUILD_BENCHMARKS=OFF -DBUILD_TESTS=OFF -DCUDF_USE_ARROW_STATIC=ON -DCMAKE_CUDA_RUNTIME_LIBRARY=Shared ..|grep CUDA\ Libraries
-- CUDA Libraries: /usr/local/cuda/lib64/libcudart_static.a;Threads::Threads;dl;/usr/lib/x86_64-linux-gnu/librt.so

[robertmaynard]

@robertmaynard thanks, I just tried that but I am not sure that FindCUDA is using it. Perhaps I missed something:

-DCMAKE_CUDA_RUNTIME_LIBRARY=Shared:

$ PARALLEL_LEVEL=12 cmake -GNinja -DCMAKE_CUDA_ARCHITECTURES="75" -DCMAKE_INSTALL_PREFIX=$CONDA_PREFIX -DARROW_STATIC_LIB=ON -DUSE_NVTX=0 -DRMM_LOGGING_LEVEL=OFF -DBUILD_BENCHMARKS=OFF -DBUILD_TESTS=OFF -DCUDF_USE_ARROW_STATIC=ON -DCMAKE_CUDA_RUNTIME_LIBRARY=Shared ..|grep CUDA\ Libraries
-- CUDA Libraries: /usr/local/cuda/lib64/libcudart_static.a;Threads::Threads;dl;/usr/lib/x86_64-linux-gnu/librt.so

FindCUDA doesn't use CMAKE_CUDA_RUNTIME_LIBRARY as it predates the existence of CUDA as supported language by Cmake ( via enable_language(CUDA) or project(example LANGUAGES CXX CUDA)

[abellina]

Makes sense @robertmaynard.

So for cudf-0.19, it seems we should work around this + work on changes in arrow (especially if it doesn't need cudart) for future cudf versions.

[kkraus14]

@jlowe @abellina is this resolved?

[abellina]

@kkraus14 I retested my case with the latest nightly builds and cuInit is gone. All I see is a fast cudaHostAlloc (~1ms), and my parquet scan takes ~100ms (instead of 1+ second) so this fixes what I reported for 0.19.

@jlowe shared the original trace with me, and I verified that it had a cuInit before the long cudaHostAlloc:

Closing. Thanks @robertmaynard, @kkraus14, @jrhemstad, @jlowe and @nvdbaranec (who suspected this was static cudart).

[kkraus14]

To further close the loop here, PR has been merged in Arrow where come the 4.0.0 release it will no longer link to libcudart: apache/arrow@5b5c058