LuciferYang commented on code in PR #13144:
URL: https://github.com/apache/gluten/pull/13144#discussion_r4178606925
##########
gluten-core/src/main/scala/org/apache/gluten/iterator/IteratorsV1.scala:
##########
@@ -115,23 +115,21 @@ object IteratorsV1 {
}
}
+ // Reports each read's duration in nanoseconds. Converting each read to
milliseconds would
+ // truncate any read shorter than one millisecond to zero.
private class ReadTimeAccumulator[A](in: Iterator[A], onAdded: Long => Unit)
extends Iterator[A] {
override def hasNext: Boolean = {
val prev = System.nanoTime()
val out = in.hasNext
- val after = System.nanoTime()
- val duration = TimeUnit.NANOSECONDS.toMillis(after - prev)
- onAdded(duration)
+ onAdded(System.nanoTime() - prev)
Review Comment:
The unit change is intentional. Following the review discussion above, the
accumulator was dropped in favor of reporting nanoseconds and making `selfTime`
an `nsTiming` metric. The PR description now says so; it still described the
earlier accumulator when this review ran.
--
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]
