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
