Github user squito commented on a diff in the pull request: https://github.com/apache/spark/pull/22612#discussion_r226521801 --- Diff: core/src/main/scala/org/apache/spark/metrics/ExecutorMetricType.scala --- @@ -95,10 +135,29 @@ private[spark] object ExecutorMetricType { OnHeapUnifiedMemory, OffHeapUnifiedMemory, DirectPoolMemory, - MappedPoolMemory + MappedPoolMemory, + ProcessTreeMetrics + ) + // List of defined metrics + val definedMetrics = IndexedSeq( + "JVMHeapMemory", + "JVMOffHeapMemory", + "OnHeapExecutionMemory", + "OffHeapExecutionMemory", + "OnHeapStorageMemory", + "OffHeapStorageMemory", + "OnHeapUnifiedMemory", + "OffHeapUnifiedMemory", + "DirectPoolMemory", + "MappedPoolMemory", + "ProcessTreeJVMVMemory", + "ProcessTreeJVMRSSMemory", + "ProcessTreePythonVMemory", + "ProcessTreePythonRSSMemory", + "ProcessTreeOtherVMemory", + "ProcessTreeOtherRSSMemory" --- End diff -- I don't love having this list of names defined separately. its seems prone to a simple mistake in ordering. In the memory monitor, I had each "metric getter" define two methods, one for just giving the names of the metrics it would grab (which only gets called once), and another method to actually grab the values https://github.com/squito/spark-memory/blob/master/core/src/main/scala/com/cloudera/spark/MemoryGetter.scala#L6-L9 It also avoids allocating objects by having the metric getter write the value directly into a destination array, at the right offset: https://github.com/squito/spark-memory/blob/master/core/src/main/scala/com/cloudera/spark/MemoryMonitor.scala#L46-L53 https://github.com/squito/spark-memory/blob/master/core/src/main/scala/com/cloudera/spark/MemoryMonitor.scala#L68-L71 that does lose a bit of readability. I know currently the metrics aren't collected very often so this isn't a big deal, but I think we need to start doing it more often, and then this might be important.
--- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org