[chenterry85]

Summary:

Current Issue

AMD trace latency is higher than the actual running program due to high overheads incurred from tracing. This issue mainly surfaces when QPS is high, making the gpu traces unusable for analysis (see https://fburl.com/workplace/r3mffmwg for more context).

The main goal here is to reduce the high overheads incurred from tracing. To achieve this goal, we borrow the idea from Nvidia's CUPTI_PER_THREAD_BUFFER_ENABLED flag.

Context

Start with one worker thread making one GPU call.

merge thread A ──► hipLaunchKernel
│
└─ kineto callback fires, builds a row
│
└──► rows_ one shared vector

That callback runs inline, on your thread, before the HIP call returns. So your worker is doing kineto's bookkeeping on its own time.

Before

Now the real workload. Thirty-two merge threads, all doing this constantly.

merge thread A ──► row ──┐
merge thread B ──► row ──┤
merge thread C ──► row ──┼──► [ one mutex ] ──► rows_
... │
merge thread ...32 ──────┘

One vector means one lock. Thirty-two threads take turns. That queue is the 54 microseconds per launch.

Fix:

The fix gives each thread its own bucket.

merge thread A ──► row ──► buffer A
merge thread B ──► row ──► buffer B no lock anywhere
merge thread C ──► row ──► buffer C
...

A thread only ever touches its own buffer, so there is nothing to contend over.

But the trace writer still expects one list. So the buckets get poured back together once, at the end, after the capture window closes.

buffer A ─┐
buffer B ─┼──► mergeThreadBuffers() ──► rows_ ──► trace written
buffer C ─┘
...
runs once per trace, off the hot path

That is the whole idea. Fast path writes to a private bucket, slow path stitches them together once.

The flag picks which of the two pictures you get.

ROCPROF_PER_THREAD_BUFFER_ENABLED

 false (default) ──► the mutex picture, unchanged from today
 true            ──► the buckets picture

---

How the six files split

Two groups, and they do unrelated jobs.

Getting the flag from a config string down to the logger. Three files, one line or so each.

"ROCPROF_PER_THREAD_BUFFER_ENABLED=true"
│
├─ Config.cpp parse the key into a bool
├─ Config.h hold it, expose a getter
└─ RocmActivityProfiler.cpp read it, hand it to the logger

The two paths themselves. Three more files.

RocprofLogger.h the ThreadTraceBuffer type, the registry, the flag
RocprofLogger.cpp the two branches, plus getThreadBuffer and the merge
RocprofActivityApi.cpp calls the merge before anyone reads the rows

Test Plan:
buck2 build @//mode/opt-amd-gpu fbcode//kineto/libkineto:cupti_api
buck2 build @//mode/opt-amd-gpu fbcode//kineto/libkineto:activity_profiler
buck2 build @//mode/opt-amd-gpu fbcode//kineto/libkineto:config
buck2 test @//mode/opt-amd-gpu fbcode//kineto/libkineto/test:config_test

All green. config_test 13 pass, 0 fail, including two new cases covering the
new key's default and parse, and that the CUPTI key does not enable it.
Negative control: deleting the parse branch drops it to 12 pass, 1 fail.

Per-file compile coverage was verified rather than inferred. A cpp_library
build compiles its own srcs, not its deps' objects, so a green dependent target
proves nothing about a changed file. Injecting static_assert(false) at each
edit site and requiring the named failure confirms all four regions compile under
mode/opt-amd-gpu: RocprofLogger.cpp and RocprofActivityApi.cpp via :cupti_api,
RocmActivityProfiler.cpp via :activity_profiler, Config.cpp via :config.

Contention benchmark, both accumulation strategies, 200k ops per thread:

g++ -std=c++17 -O2 -pthread bench.cpp -o bench && ./bench

 threads      OLD ns/op      NEW ns/op    speedup
       1           19.0           18.6       1.0x
       2           39.4            9.9       4.0x
       3           42.7            9.1       4.7x
       4           62.9            8.4       7.5x
       8          397.0           11.2      35.6x
      16          563.7           36.9      15.3x
      32          844.0           18.1      46.6x

Identical at 1 thread, which is the control that both strategies do the same work
uncontended. The old path's 1/2/3 progression reproduces the 18.7/37.5/56.9 us
shape measured in the production trace, from an independent source.

Correctness harness, 32 threads x 5000 rows x 3 rounds per mode, clearLogs()
between rounds:

g++ -std=c++17 -Wall -Wextra -O1 -pthread stress.cpp -o stress && ./stress

[flag off] rows 160000, thread buffers  0, correlations 160000/160000, totalRows_      0
[flag on ] rows 160000, thread buffers 32, correlations 160000/160000, totalRows_ 160000

Covers both gated sites. Row insertion and external-correlation recording are
each accumulated per thread and merged, with identical totals in both modes.
Asserts merge idempotency, that clearLogs() zeroes rows, correlations and thread
buffers, and that the disabled path leaves totalRows_ at exactly 0, which is the
observable proof the off arm never touches the shared atomic.

Not run on a GPU. No before-and-after capture on hardware.

Re-verified on V15 after the two review fixes (threadBuffersMutex_ taken before
the empty() check in mergeThreadBuffers, and the Config.h member initializer
mirrored to xplat):

cupti_api EXIT=0   activity_profiler EXIT=0   config EXIT=0
config_test: Pass 13. Fail 0.
stress harness: PASS, both modes, 3 rounds x 32 threads x 5000 rows,
                plus the flag-transition scenario.

Reviewed By: sanrise

Differential Revision: D121287631

Pulled By: chenterry85

[pytorch-bot]

Workflows were awaiting approval. CI has now been triggered for the ciflow labels on this PR.

[meta-codesync]

@chenterry85 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D121287631.

[meta-codesync]

@chenterry85 has imported this pull request. If you are a Meta employee, you can view this in D121287631.