LuciferYang commented on code in PR #13144:
URL: https://github.com/apache/gluten/pull/13144#discussion_r4178593126


##########
gluten-core/src/main/scala/org/apache/gluten/iterator/IteratorsV1.scala:
##########
@@ -115,23 +115,37 @@ object IteratorsV1 {
     }
   }
 
+  // Accumulates nanosecond read durations and reports only whole 
milliseconds, carrying the
+  // sub-millisecond remainder to the next call so reads shorter than a 
millisecond are not
+  // truncated to zero. Package-visible so the carry-over can be tested 
deterministically
+  // without depending on wall-clock timing.
+  private[iterator] class NanosToMillisAccumulator(onAdded: Long => Unit) {

Review Comment:
   There is no production report behind this; it came from reading the code. 
Agreed that a stateful accumulator is too much for this metric, and that 
milliseconds is the wrong unit. Reworked in 309a0255b: the collector now 
reports raw nanoseconds (`collectReadNanos`) and `selfTime` is an `nsTiming` 
metric, so the accumulator is gone and sub-millisecond reads are no longer 
dropped.



-- 
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]

Reply via email to