-
Notifications
You must be signed in to change notification settings - Fork 72
[rocprofiler-sdk] rename librocprofv3-attach.so #1342
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
[rocprofiler-sdk] rename librocprofv3-attach.so #1342
Conversation
7059074 to
f929080
Compare
779a0ae to
1515bc5
Compare
f929080 to
e01b466
Compare
itrowbri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with the component sections of the code, but the changes seem simple enough. Will we need to add some tests outside of the tests/rocprofv3 section if we formalize this change given that attachment won't be limited to rocprofv3 anymore?
|
Please rename the library hardcoded in rocprofiler-compute at https://github.com/ROCm/rocm-systems/blob/develop/projects/rocprofiler-compute/src/rocprof_compute_profile/profiler_rocprofiler_sdk.py |
1515bc5 to
0914631
Compare
e01b466 to
4faac6b
Compare
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/details/filesystem.hpp
Show resolved
Hide resolved
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/rocattach.cpp
Show resolved
Hide resolved
0914631 to
89626a2
Compare
89626a2 to
7ef5c91
Compare
4b0ca98 to
e55bc1a
Compare
7ef5c91 to
fc03ac7
Compare
e55bc1a to
cd1a7c7
Compare
92434df to
2d55220
Compare
bwelton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the typo, may fix the tests as well.
projects/rocprofiler-sdk/cmake/rocprofiler_config_packaging.cmake
Outdated
Show resolved
Hide resolved
2d55220 to
87e4dd9
Compare
projects/rocprofiler-sdk/source/docs/api-reference/process_attachment.rst
Outdated
Show resolved
Hide resolved
projects/rocprofiler-sdk/source/lib/rocprofiler-sdk-rocattach/ptrace_session.cpp
Show resolved
Hide resolved
f6cd3a8 to
31de924
Compare
1c24012 to
13a5cdb
Compare
Comments have either been addressed or moved to #1653
- Renames library to librocprofiler-sdk-rocattach - ROCAttach library will be formalized and documented in future commit
- Rename rocprofv3-attach.py to rocprof-attach.py - Use common filesystem.hpp in rocattach
13a5cdb to
3c31971
Compare
|
psdb passing rocprofiler test -- override |
* attach: rename librocprofv3-attach - Renames library to librocprofiler-sdk-rocattach - ROCAttach library will be formalized and documented in future commit * Address review comments - Rename rocprofv3-attach.py to rocprof-attach.py - Use common filesystem.hpp in rocattach * Fix component name typo * Doc fixup [rocm-systems] ROCm/rocm-systems#1342 (commit 1271813)
* attach: rename librocprofv3-attach - Renames library to librocprofiler-sdk-rocattach - ROCAttach library will be formalized and documented in future commit * Address review comments - Rename rocprofv3-attach.py to rocprof-attach.py - Use common filesystem.hpp in rocattach * Fix component name typo * Doc fixup [rocm-systems] ROCm/rocm-systems#1342 (commit 1271813)
Motivation
In anticipation of formalizing an API for custom tools to attach with, we will rename librocprofv3-attach.so to librocprofiler-sdk-rocattach.so. This name was chosen because the library is technically not part of rocprofv3. We also anticipate formalizing and publishing a public API with this library, which we are calling ROCAttach. This API will be used by custom tools who wish to create their own attachment scenarios, so they do not need to learn the inner workings of ptrace or the protocol used to communicate with rocprofiler-sdk in the running process.
Technical Details
This PR is staged over a documentation change that I expect to be merged first.
I've kept just the rename separate for easier review and gitfu. Another review and documentation step for this API is to follow.
Test Plan
Use existing CI
Test Result
First result - failed due to incorrect naming of .so file, fixed this
Second result - some existing failures, need to fix before validating this change is ok
Submission Checklist