Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gdb pretty-printers for rmm types #1088

Merged
merged 9 commits into from
Aug 26, 2022

Conversation

upsj
Copy link
Contributor

@upsj upsj commented Aug 17, 2022

Description

This PR adds a pretty-printer for device_uvector and pulls in Thrust pretty-printers that were added in NVIDIA/thrust#1631. CMake provides a convenience script to load all of the pretty-printers, to resolve the duplication concerns raised in rapidsai/cudf#11499.

Example output:

$ gdb -q gtests/DEVICE_UVECTOR_TEST
Reading symbols from gtests/DEVICE_UVECTOR_TEST...
(gdb) b cudaMalloc
Function "cudaMalloc" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (cudaMalloc) pending.
(gdb) run
Starting program: /home/nfs/tribizel/rapids/rmm/build/cuda-11.5.0/feature__pretty-printers/debug/gtests/DEVICE_UVECTOR_TEST 
warning: Error disabling address space randomization: Operation not permitted
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/x86_64-linux-gnu/libthread_db.so.1".
Running main() from gmock_main.cc
[==========] Running 95 tests from 5 test suites.
[----------] Global test environment set-up.
[----------] 19 tests from TypedUVectorTest/0, where TypeParam = signed char
[ RUN      ] TypedUVectorTest/0.MemoryResource
[New Thread 0x7f741efc1000 (LWP 86147)]

Thread 1 "DEVICE_UVECTOR_" hit Breakpoint 1, 0x00007f7426dddc80 in cudaMalloc () from /home/nfs/tribizel/rapids/compose/etc/conda/cuda_11.5/envs/rapids/lib/libcudart.so.11.0
(gdb) c
Continuing.
[New Thread 0x7f741e7c0000 (LWP 86148)]
[       OK ] TypedUVectorTest/0.MemoryResource (999 ms)
[ RUN      ] TypedUVectorTest/0.ZeroSizeConstructor
[       OK ] TypedUVectorTest/0.ZeroSizeConstructor (0 ms)
[ RUN      ] TypedUVectorTest/0.NonZeroSizeConstructor

Thread 1 "DEVICE_UVECTOR_" hit Breakpoint 1, 0x00007f7426dddc80 in cudaMalloc () from /home/nfs/tribizel/rapids/compose/etc/conda/cuda_11.5/envs/rapids/lib/libcudart.so.11.0
(gdb) finish
Run till exit from #0  0x00007f7426dddc80 in cudaMalloc () from /home/nfs/tribizel/rapids/compose/etc/conda/cuda_11.5/envs/rapids/lib/libcudart.so.11.0
rmm::mr::cuda_memory_resource::do_allocate (bytes=<optimized out>, this=<optimized out>) at /home/nfs/tribizel/rapids/rmm/include/rmm/mr/device/cuda_memory_resource.hpp:70
70          RMM_CUDA_TRY_ALLOC(cudaMalloc(&ptr, bytes));
(gdb) finish
Run till exit from #0  rmm::mr::cuda_memory_resource::do_allocate (bytes=<optimized out>, this=<optimized out>) at /home/nfs/tribizel/rapids/rmm/include/rmm/mr/device/cuda_memory_resource.hpp:70
0x00005587874feef0 in rmm::device_buffer::allocate_async (bytes=12345, this=0x7ffefebb46b0) at /home/nfs/tribizel/rapids/rmm/include/rmm/device_buffer.hpp:418
418         _data     = (bytes > 0) ? memory_resource()->allocate(bytes, stream()) : nullptr;
Value returned is $1 = (void *) 0x7f73ff000000
(gdb) finish
Run till exit from #0  0x00005587874feef0 in rmm::device_buffer::allocate_async (bytes=12345, this=0x7ffefebb46b0) at /home/nfs/tribizel/rapids/rmm/include/rmm/device_buffer.hpp:418
TypedUVectorTest_NonZeroSizeConstructor_Test<signed char>::TestBody (this=<optimized out>) at /home/nfs/tribizel/rapids/rmm/tests/device_uvector_tests.cpp:55
55        EXPECT_EQ(vec.size(), size);
(gdb) print vec
$2 = {_storage = {_data = 0x7f73ff000000, _size = 12345, _capacity = 12345, _stream = {stream_ = 0x0}, _mr = 0x558787561008 <rmm::mr::detail::initial_resource()::mr>}}
(gdb) source load-pretty-printers 
(gdb) print vec
$3 = rmm::device_uvector<signed char> of length 12345, capacity 12345 = {0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 
  0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 
  0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 
  0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 
  0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 
  0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 
  0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000'...}
(gdb) 

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@upsj upsj requested a review from a team as a code owner August 17, 2022 16:18
@github-actions github-actions bot added the CMake label Aug 17, 2022
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some thoughts on Python clarity -- I haven't used GDB pretty printers before so forgive me if I'm not aware of some limitations. Does GDB run the Python interpreter from the user environment, or a recent version like 3.8+?

scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
if char == '>':
depth -= 1
if depth == 0:
return False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to understand the logic here -- if you only care if the < brackets are balanced, you can just count those. What's the "not alias" part mean?

Copy link
Contributor Author

@upsj upsj Aug 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess an example is easiest: std::vector<std::optional<std::pair<int, std::optional<double>>>> should pass, but std::vector<std::optional<std::pair<int, std::optional<double>>>>::some_templated_type_alias<int> should fail. gdb sometimes doesn't resolve type aliases, so the pretty-printer might erroneously be called for such a some_templated_type_alias<int>, and would fail at accessing the expected members.

@upsj
Copy link
Contributor Author

upsj commented Aug 17, 2022

gdb runs in the user environment, so I wouldn't bet on recent Python being available unfortunately

@upsj upsj requested a review from bdice August 17, 2022 18:41
@harrism harrism added this to PR-WIP in v22.10 Release via automation Aug 17, 2022
@harrism harrism added feature request New feature or request non-breaking Non-breaking change labels Aug 17, 2022
@@ -0,0 +1,2 @@
source @Thrust_SOURCE_DIR@/scripts/gdb-pretty-printers.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a .in file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is a general convention, but in the projects I worked in, we use it for template files that will be expanded into an actual file by CMake - config headers, pkg-config files etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CMake convention is <file>.suffix.in for the input to configure_file. ( .cpp.in => .cpp )

v22.10 Release automation moved this from PR-WIP to PR-Reviewer approved Aug 22, 2022
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a handful of Python improvements. The general approach is much clearer now, with load-pretty-printers.in and the like.

scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
scripts/gdb-pretty-printers.py Outdated Show resolved Hide resolved
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @upsj!

@bdice
Copy link
Contributor

bdice commented Aug 26, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit ed43650 into rapidsai:branch-22.10 Aug 26, 2022
v22.10 Release automation moved this from PR-Reviewer approved to Done Aug 26, 2022
@bdice bdice mentioned this pull request Oct 7, 2022
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Oct 11, 2022
This applies rmm's configured pre-commit hooks to format the gdb pretty printer script that was recently added (#1088). I would like to move rmm to use pre-commit for all style checks (as was recently done for cudf) and this is a prerequisite step.

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - Mark Harris (https://github.com/harrism)

URL: #1127
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake feature request New feature or request non-breaking Non-breaking change
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants