Copilot commented on code in PR #13116:
URL: https://github.com/apache/gluten/pull/13116#discussion_r4091597356
##########
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxMetricsApi.scala:
##########
@@ -554,6 +554,22 @@ class VeloxMetricsApi extends MetricsApi with Logging {
override def genSortTransformerMetricsUpdater(metrics: Map[String,
SQLMetric]): MetricsUpdater =
new SortMetricsUpdater(metrics)
+ override def genTopNTransformerMetrics(sparkContext: SparkContext):
Map[String, SQLMetric] =
+ Map(
+ "numOutputRows" -> SQLMetrics.createMetric(sparkContext, "number of
output rows"),
+ "outputVectors" -> SQLMetrics.createMetric(sparkContext, "number of
output vectors"),
+ "outputBytes" -> SQLMetrics.createSizeMetric(sparkContext, "number of
output bytes"),
+ "wallNanos" -> SQLMetrics.createNanoTimingMetric(sparkContext, "time of
top-n"),
+ "cpuCount" -> SQLMetrics.createMetric(sparkContext, "cpu wall time
count"),
+ "peakMemoryBytes" -> SQLMetrics.createSizeMetric(sparkContext, "peak
memory bytes"),
+ "numMemoryAllocations" -> SQLMetrics.createMetric(
+ sparkContext,
+ "number of memory allocations")
Review Comment:
The new TopN metric set omits `loadLazyVectorTime`, although
`LimitMetricsUpdater`—which this implementation is intended to mirror—reports
it and native metrics parsing populates it on the final operator suite. Queries
that load lazy vectors during TopN will therefore still lose that native
metric; add the metric to this map and update `TopNMetricsUpdater` to consume
it.
##########
backends-bolt/src/main/scala/org/apache/gluten/backendsapi/bolt/BoltMetricsApi.scala:
##########
@@ -543,6 +543,22 @@ class BoltMetricsApi extends MetricsApi with Logging {
override def genSortTransformerMetricsUpdater(metrics: Map[String,
SQLMetric]): MetricsUpdater =
new SortMetricsUpdater(metrics)
+ override def genTopNTransformerMetrics(sparkContext: SparkContext):
Map[String, SQLMetric] =
+ Map(
+ "numOutputRows" -> SQLMetrics.createMetric(sparkContext, "number of
output rows"),
+ "outputVectors" -> SQLMetrics.createMetric(sparkContext, "number of
output vectors"),
+ "outputBytes" -> SQLMetrics.createSizeMetric(sparkContext, "number of
output bytes"),
+ "wallNanos" -> SQLMetrics.createNanoTimingMetric(sparkContext, "time of
top-n"),
+ "cpuCount" -> SQLMetrics.createMetric(sparkContext, "cpu wall time
count"),
+ "peakMemoryBytes" -> SQLMetrics.createSizeMetric(sparkContext, "peak
memory bytes"),
+ "numMemoryAllocations" -> SQLMetrics.createMetric(
+ sparkContext,
+ "number of memory allocations")
Review Comment:
The new TopN metric set omits `loadLazyVectorTime`, although
`LimitMetricsUpdater`—which this implementation is intended to mirror—reports
it and native metrics parsing populates it on the final operator suite. Queries
that load lazy vectors during TopN will therefore still lose that native
metric; add the metric to this map and update `TopNMetricsUpdater` to consume
it.
--
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]
