Yicong-Huang commented on code in PR #59246:
URL: https://github.com/apache/spark/pull/59246#discussion_r4192090108
##########
python/pyspark/worker.py:
##########
@@ -128,21 +128,88 @@
)
-def report_metrics(
- outfile, boot, init, finish, execution_duration_ms, memory_bytes_spilled,
disk_bytes_spilled
-):
+class _WorkerTimer:
+ """Measure one named duration and add each interval to the collector's
task total.
+
+ The collector owns the duration dictionary. This scope keeps a reference
to it and a
+ start time for its current with block. Reuse the scope across sequential
batches.
+ """
+
+ def __init__(self, shared_duration_totals_ns: dict[str, int], metric_name:
str) -> None:
+ """Bind one metric name to the collector's shared duration totals."""
+ # Keep the collector's dictionary by reference so updates reach its
task totals.
+ self._shared_duration_totals_ns = shared_duration_totals_ns
+ self._metric_name = metric_name
+ self._block_start_ns: Optional[int] = None
+
+ def __enter__(self) -> None:
+ """Start the clock when Python enters the with block."""
+ # Re-entering this instance would overwrite an unfinished
measurement's start time.
+ if self._block_start_ns is not None:
+ raise RuntimeError("Worker timer is already running")
+ self._block_start_ns = time.perf_counter_ns()
+
+ def __exit__(self, exc_type: Any, exc_value: Any, traceback: Any) -> None:
+ """Accumulate elapsed time when Python leaves the block, including on
an exception."""
+ assert self._block_start_ns is not None
+ elapsed_ns = time.perf_counter_ns() - self._block_start_ns
+ self._block_start_ns = None
+ self._shared_duration_totals_ns[self._metric_name] += elapsed_ns
+ # Python supplies the exception arguments; returning None lets the
error propagate.
Review Comment:
having context manager to measure start and end timer is good, but I've met
this issue before:
consider a code
```
for i in range(10000):
with udf_timer():
udf()
```
vs
```
with udf_timer():
for I in range(10000):
udf()
```
I would expect these two to report noticeably different timings. In the
first case, we take 10,000 separate start/end measurements and sum many small
intervals. The per-measurement overhead and timing error/resolution can
accumulate, whereas the second case measures one continuous interval.
Have we evaluated how large this measurement error can be?
The reason I am asking this is because I see we are using this in handler as
of the first case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
