Skip to content

Commit 1fa3885

Browse files
authored
fix(tensilelite): Reduce unaccounted overhead of ScopedTimer and timing_context (ROCm#6043)
## Motivation Each `ScopedTimer` / `timing_context` call performed string formatting and I/O inline, adding significant overhead that inflated parent timers, especially for bad for high call count but low execution time operations. Additionally, `gpu_kernel_execution` and `cpu_reference_gemm` were placed at the wrong hierarchy level, making the analysis output misleading. Follow-up to ROCm#6259. ## Technical Details Overhead reduction - **Deferred I/O** (`TimingInstrumentation.hpp`, `TimingInstrumentation.py`): Replaced inline `writeLine`/`_timing_logger.info` calls with lightweight buffer appends during measurement. All formatting and I/O now happens in a single `flushTimingBuffer()` call after measurement is complete. - **Overhead calibration** (`TimingInstrumentation.hpp`, `main.cpp`): Added `calibrateTimingOverhead()` which measures per-call `ScopedTimer` overhead at startup (warmup + 100k iterations). The calibrated value is emitted as a `timing_overhead` record and used by `analyze_timing.py` to subtract instrumentation overhead from parent totals. - **Buffer pre-allocation** (`TimingInstrumentation.hpp`): `initTimingBuffer()` reserves 2.5M entries upfront to avoid reallocation stalls during benchmarking. - **Analysis improvements** (`analyze_timing.py`): Added per-call overhead subtraction based on descendant invocation counts, and a new "Timing Overhead" phase group for `timing_overhead` and `flush_timing_buffer`. Fixes - **Hierarchy fixes** (`main.cpp`, `BenchmarkTimer.cpp`, `analyze_timing.py`): - Moved `gpu_kernel_execution` under `benchmark_runs` (was a sibling, incorrectly double-counted). - Moved `cpu_data_init` and `cpu_reference_gemm` under `pre_problem` (where they actually execute). ## Test Plan Run timing instrumentation end-to-end and verify that parent totals are consistent with child sums after overhead adjustment. ## Test Result Timing output shows correct parent-child relationships and overhead-adjusted totals. --------- Signed-off-by: Alex Vasile <48962821+Alex-Vasile@users.noreply.github.com>
1 parent 4068136 commit 1fa3885

8 files changed

Lines changed: 302 additions & 58 deletions

File tree

projects/hipblaslt/tensilelite/Tensile/Common/TimingInstrumentation.py

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,37 @@
3737
_timing_logger.setLevel(logging.INFO)
3838
_timing_logger.propagate = False
3939

40+
# Deferred I/O buffer: list of (category, duration_ms) tuples.
41+
# All formatting and I/O happens in flush_timing_buffer().
42+
# Raw timings are stored without overhead adjustment; the analysis script
43+
# (analyze_timing.py) handles overhead subtraction using the hierarchy and
44+
# the timing_overhead record emitted by the C++ client.
45+
_timing_buffer = []
46+
4047

4148
@contextmanager
4249
def timing_context(category_name):
43-
"""Context manager for timing instrumentation."""
50+
"""Context manager for timing instrumentation.
51+
52+
Records raw wall-clock time with no overhead subtraction. Python-side
53+
overhead (context-manager protocol, time.time_ns, dict lookup) is not
54+
tracked because there are only ~a dozen calls per run — the overhead
55+
is negligible relative to the seconds-scale measurements. C++ overhead
56+
is tracked separately via a calibrated timing_overhead record.
57+
"""
4458
if globalParameters.get("TimingInstrumentation", False):
4559
start = time.time_ns()
4660
try:
4761
yield
4862
finally:
49-
elapsed_ms = (time.time_ns() - start) / 1_000_000
50-
_timing_logger.info(f"TIMING:{category_name}:{elapsed_ms:.3f}")
63+
elapsed_ns = time.time_ns() - start
64+
_timing_buffer.append((category_name, elapsed_ns / 1_000_000))
5165
else:
5266
yield
67+
68+
69+
def flush_timing_buffer():
70+
"""Write all buffered timing records and reset."""
71+
for category, duration_ms in _timing_buffer:
72+
_timing_logger.info(f"TIMING:{category}:{duration_ms:.3f}")
73+
_timing_buffer.clear()

projects/hipblaslt/tensilelite/Tensile/Tensile.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
from Tensile.Common.Capabilities import makeIsaInfoMap
4646
from Tensile.Common.GlobalParameters import globalParameters, assignGlobalParameters, \
4747
restoreDefaultGlobalParameters
48-
from Tensile.Common.TimingInstrumentation import timing_context
48+
from Tensile.Common.TimingInstrumentation import timing_context, flush_timing_buffer
4949
from Tensile.Toolchain.Assembly import AssemblyToolchain, makeAssemblyToolchain
5050
from Tensile.Toolchain.Source import SourceToolchain, makeSourceToolchain
5151
from Tensile.Toolchain.Validators import validateToolchain, ToolchainDefaults
@@ -125,6 +125,7 @@ def executeStepsInConfig(
125125
buildOnly,
126126
solutionPoolFiles,
127127
)
128+
flush_timing_buffer()
128129
print1("")
129130

130131
if buildOnly:
@@ -155,6 +156,7 @@ def executeStepsInConfig(
155156
debugConfig.printIndexAssignmentInfo,
156157
isaInfoMap,
157158
)
159+
flush_timing_buffer()
158160
print1("")
159161
else:
160162
print1("# LibraryLogic already done.")
@@ -178,6 +180,7 @@ def executeStepsInConfig(
178180
deviceId,
179181
gfxName,
180182
)
183+
flush_timing_buffer()
181184
print1("")
182185

183186

@@ -682,6 +685,9 @@ def Tensile(userArgs):
682685
print("Overriding {0}={1}".format(key, value))
683686
globalParameters[key] = value
684687

688+
if "MaxFileName" in globalParameters or "MaxFileName" in config:
689+
printWarning("MaxFileName is no longer configurable, it will be automatically set to 64")
690+
685691
executeStepsInConfig(config, outputPath, asmToolchain, srcToolchain, isaInfoMap, cCompiler, debugConfig, device_id, prob_sol_map, buildOnly, solutionPoolFiles)
686692

687693
def TensileConfigPath(*args):

projects/hipblaslt/tensilelite/Tensile/Tests/unit/characterization/TimingInstrumentation/test_timing_instrumentation_char.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import pytest
1313

1414
from Tensile.Common.GlobalParameters import globalParameters
15-
from Tensile.Common.TimingInstrumentation import timing_context
15+
from Tensile.Common.TimingInstrumentation import flush_timing_buffer, timing_context
1616

1717
pytestmark = pytest.mark.unit
1818

@@ -40,9 +40,10 @@ def test_timing_context_disabled_yields():
4040

4141

4242
def test_timing_context_enabled_times_and_logs():
43-
# Flag on -> the timing path; body runs and a TIMING line is emitted. The
44-
# "tensile.timing" logger has propagate=False, so attach a capture handler
45-
# directly to it (caplog/capsys can't see it).
43+
# Flag on -> the timing path; body runs and a TIMING line is buffered until
44+
# flush_timing_buffer() emits it. The "tensile.timing" logger has
45+
# propagate=False, so attach a capture handler directly to it (caplog/capsys
46+
# can't see it).
4647
import logging
4748

4849
messages = []
@@ -53,13 +54,17 @@ def emit(self, record):
5354

5455
logger = logging.getLogger("tensile.timing")
5556
handler = _Capture()
57+
flush_timing_buffer()
5658
logger.addHandler(handler)
5759
try:
5860
with _timing_flag(True):
5961
ran = []
6062
with timing_context("mycat"):
6163
ran.append(1)
6264
assert ran == [1]
65+
assert not any("TIMING:mycat:" in m for m in messages)
66+
flush_timing_buffer()
6367
finally:
6468
logger.removeHandler(handler)
69+
flush_timing_buffer()
6570
assert any("TIMING:mycat:" in m for m in messages)

projects/hipblaslt/tensilelite/Tensile/Tests/unit/test_analyze_timing.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,6 @@ def test_nested_timing_totals(self, nested_timing_log):
250250
+ timings["code_object_loading"][0]
251251
+ timings["cpu_data_init"][0]
252252
+ timings["cpu_reference_gemm"][0]
253-
+ timings["gpu_kernel_execution"][0]
254253
)
255254
assert cpp_sum <= timings["python_client_execution"][0]
256255

0 commit comments

Comments
 (0)