[PROTON] Refactor GPU profilers #4056
Conversation
Jokeren
commented
Jun 3, 2024
Jokeren
commented
- Extract duplicated code into GPUProfiler.h
- Track finished correlation ids for both cupti and amd profilers
|
Waiting for @CRobeck's feedback |
| } | ||
| cupti::activityFlushAll<true>(CUPTI_ACTIVITY_FLAG_FLUSH_FORCED); | ||
| profiler.correlation.flush( | ||
| /*maxRetries=*/100, /*sleepMs=*/10, |
There was a problem hiding this comment.
Did we do any testing of these maxRetries and sleepMs values? maxRetries is probably fine at 100 but is the 10 ms from a documented latency value from somewhere? Maybe worth a command line argument to set these?
There was a problem hiding this comment.
There isn't a documentation for it. In general increase it may get more valid records in corner cases. Unfortunately we don't support passing configurations through the command line or python function in proton. We would like to keep a very small number of profiling knobs to avoid increasing the learning curve.
I think may could use environment variables to control it though.
I can leave it to you to do a bit investigation and add the support. What do you think?
| std::set<Data *> &dataSet, | ||
| const roctracer_record_t *record) { | ||
| void processActivity(std::mutex &corrIdToExternIdMutex, | ||
| std::map<uint64_t, size_t> &corrIdToExternId, |
There was a problem hiding this comment.
Is this data type inconsistency with above expected? Is it a known difference that cupti uses uint32_t and roctracer uint64_t?
There was a problem hiding this comment.
| (symbol != NULL) | ||
| ? abi::__cxa_demangle(symbol, NULL, &funcnamesize, &status) | ||
| ? abi::__cxa_demangle(symbol, NULL, &funcNameSize, &status) |
There was a problem hiding this comment.
Nit: Maybe move this into a check around abi::__cxa_demangle:
if (const char* name = abi::__cxa_demangle(symbol, NULL, &funcnamesize, &status))
and then just return the unmangled name if unsuccessful.
| @@ -173,10 +146,10 @@ struct CuptiProfiler::CuptiProfilerPimpl { | |||
| static void callbackFn(void *userData, CUpti_CallbackDomain domain, | |||
| CUpti_CallbackId cbId, const void *cbData); | |||
|
|
|||
| const inline static size_t AlignSize = 8; | |||
| const inline static size_t BufferSize = 64 * 1024 * 1024; | |||
| inline const static size_t AlignSize = 8; | |||
There was a problem hiding this comment.
Maybe constexpr instead of/in addition inline to here?
1. Extract duplicated code into GPUProfiler.h 2. Track finished correlation ids for both cupti and amd profilers (cherry picked from commit 328b86d)
Cherry picks for release/3.0.x General: - e8bc45d [BACKEND][AMD] Disable linear layout due to perf regression (#4126) - 9a0a7c2 [AMD] Add basic verification to MFMA encoding (#4117) for RDNA: - 100e2aa [AMD][WMMA] Support dot3d (#3674) - 4a1ea8e [AMD][gfx11] Fix BF16 wmma instr generation (#4135) Proton HIP PRs: - 328b86d [PROTON] Refactor GPU profilers (#4056) - 60613fb [PROTON] Roctracer: convert agent id to gpu id for gpu ops (#4090) - c1776fa [PROTON][AMD] Add Proton HIP GPU Utilization Metrics (#4119) --------- Co-authored-by: Lei Zhang <antiagainst@gmail.com> Co-authored-by: Alexander Efimov <efimov.alexander@gmail.com> Co-authored-by: Ilya V <152324710+joviliast@users.noreply.github.com> Co-authored-by: Keren Zhou <kerenzhou@openai.com> Co-authored-by: mwootton <michael.wootton@amd.com> Co-authored-by: Corbin Robeck <corbin.robeck@amd.com>
