Skip to content

[SPARK-41709][CORE][SQL][UI] Explicitly define Seq as collection.Seq to avoid toSeq when create ui objects from protobuf objects for Scala 2.13 #39215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
LuciferYang wants to merge 14 commits into from
Closed

[SPARK-41709][CORE][SQL][UI] Explicitly define Seq as collection.Seq to avoid toSeq when create ui objects from protobuf objects for Scala 2.13 #39215

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f9ef3c8
Explicitly define Seq to collection.Seq
LuciferYang Dec 26, 2022
90e6f6f
sql module
LuciferYang Dec 26, 2022
f5865c2
avoid change public apis
LuciferYang Dec 26, 2022
ad179a1
more
LuciferYang Dec 26, 2022
755a01e
Merge branch 'apache:master' into status-pb-toSeq
LuciferYang Dec 26, 2022
02d6e0c
just use collection.Seq
LuciferYang Dec 27, 2022
fb4c23c
add mima excludes
LuciferYang Dec 27, 2022
d3659eb
fix more
LuciferYang Dec 27, 2022
2453322
add more change
LuciferYang Dec 28, 2022
6c46a0a
fix compile
LuciferYang Dec 28, 2022
464ce87
fix 2.13 compile
LuciferYang Dec 28, 2022
9a682a5
add more mima excludes
LuciferYang Dec 28, 2022
43acbcf
Merge branch 'status-pb-toSeq' of github.com:LuciferYang/spark into s…
LuciferYang Dec 28, 2022
5b500d4
fix catalyst compile
LuciferYang Dec 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions core/src/main/scala/org/apache/spark/status/api/v1/api.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ case class ApplicationInfo private[spark](
maxCores: Option[Int],
coresPerExecutor: Option[Int],
memoryPerExecutorMB: Option[Int],
attempts: Seq[ApplicationAttemptInfo])
attempts: collection.Seq[ApplicationAttemptInfo])

@JsonIgnoreProperties(
value = Array("startTimeEpoch", "endTimeEpoch", "lastUpdatedEpoch"),
Expand Down Expand Up @@ -220,7 +220,7 @@ class RDDStorageInfo private[spark](
val storageLevel: String,
val memoryUsed: Long,
val diskUsed: Long,
val dataDistribution: Option[Seq[RDDDataDistribution]],
val dataDistribution: Option[collection.Seq[RDDDataDistribution]],
val partitions: Option[Seq[RDDPartitionInfo]])

class RDDDataDistribution private[spark](
Expand Down Expand Up @@ -461,7 +461,7 @@ class ApplicationEnvironmentInfo private[spark] (
val systemProperties: Seq[(String, String)],
val metricsProperties: Seq[(String, String)],
val classpathEntries: Seq[(String, String)],
val resourceProfiles: Seq[ResourceProfileInfo])
val resourceProfiles: collection.Seq[ResourceProfileInfo])
Comment thread
LuciferYang marked this conversation as resolved.

class RuntimeInfo private[spark](
val javaVersion: String,
Expand Down
2 changes: 1 addition & 1 deletion .../scala/org/apache/spark/status/protobuf/ApplicationEnvironmentInfoWrapperSerializer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class ApplicationEnvironmentInfoWrapperSerializer extends ProtobufSerDe {
metricsProperties = info.getMetricsPropertiesList.asScala.map(pairSSToTuple).toSeq,
classpathEntries = info.getClasspathEntriesList.asScala.map(pairSSToTuple).toSeq,
resourceProfiles =
info.getResourceProfilesList.asScala.map(deserializeResourceProfileInfo).toSeq
info.getResourceProfilesList.asScala.map(deserializeResourceProfileInfo)
)
}

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/org/apache/spark/status/protobuf/ApplicationInfoWrapperSerializer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class ApplicationInfoWrapperSerializer extends ProtobufSerDe {
val maxCores = getOptional(info.hasMaxCores, info.getMaxCores)
val coresPerExecutor = getOptional(info.hasCoresPerExecutor, info.getCoresPerExecutor)
val memoryPerExecutorMB = getOptional(info.hasMemoryPerExecutorMb, info.getMemoryPerExecutorMb)
val attempts = info.getAttemptsList.asScala.map(deserializeApplicationAttemptInfo).toSeq
val attempts = info.getAttemptsList.asScala.map(deserializeApplicationAttemptInfo)
ApplicationInfo(
id = info.getId,
name = info.getName,
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/org/apache/spark/status/protobuf/RDDStorageInfoWrapperSerializer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class RDDStorageInfoWrapperSerializer extends ProtobufSerDe {
if (info.getDataDistributionList.isEmpty) {
None
} else {
Some(info.getDataDistributionList.asScala.map(deserializeRDDDataDistribution).toSeq)
Some(info.getDataDistributionList.asScala.map(deserializeRDDDataDistribution))
},
partitions =
Some(info.getPartitionsList.asScala.map(deserializeRDDPartitionInfo).toSeq)
Expand Down
11 changes: 10 additions & 1 deletion project/MimaExcludes.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,16 @@ object MimaExcludes {
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.storage.BlockManagerMessages#RegisterBlockManager.copy"),
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.storage.BlockManagerMessages#RegisterBlockManager.this"),
ProblemFilters.exclude[MissingTypesProblem]("org.apache.spark.storage.BlockManagerMessages$RegisterBlockManager$"),
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.storage.BlockManagerMessages#RegisterBlockManager.apply")
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.storage.BlockManagerMessages#RegisterBlockManager.apply"),

// [SPARK-41709][CORE][SQL][UI] Explicitly define Seq as collection.Seq to avoid toSeq when create ui objects from protobuf objects for Scala 2.13
ProblemFilters.exclude[IncompatibleResultTypeProblem]("org.apache.spark.status.api.v1.ApplicationEnvironmentInfo.resourceProfiles"),

@LuciferYang LuciferYang Dec 27, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add MimaExcludes for Scala 2.13, should we close this one

@LuciferYang LuciferYang Dec 27, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is acceptable, I will make more changes, otherwise I will close this pr

@srowen srowen Dec 27, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think those are all internal methods right? people wouldn't call these classes in code

@LuciferYang LuciferYang Dec 28, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the mima filters to be added are all internal apis

ProblemFilters.exclude[IncompatibleMethTypeProblem]("org.apache.spark.status.api.v1.ApplicationInfo.apply"),
ProblemFilters.exclude[IncompatibleResultTypeProblem]("org.apache.spark.status.api.v1.ApplicationInfo.attempts"),
ProblemFilters.exclude[IncompatibleMethTypeProblem]("org.apache.spark.status.api.v1.ApplicationInfo.copy"),
ProblemFilters.exclude[IncompatibleResultTypeProblem]("org.apache.spark.status.api.v1.ApplicationInfo.copy$default$7"),
ProblemFilters.exclude[IncompatibleMethTypeProblem]("org.apache.spark.status.api.v1.ApplicationInfo.this"),
ProblemFilters.exclude[IncompatibleMethTypeProblem]("org.apache.spark.status.api.v1.ApplicationInfo.apply")
)

// Defulat exclude rules
Expand Down
2 changes: 1 addition & 1 deletion sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ private class LiveExecutionData(val executionId: Long) extends LiveEntity {
var details: String = null
var physicalPlanDescription: String = null
var modifiedConfigs: Map[String, String] = _
var metrics = Seq[SQLPlanMetric]()
var metrics = collection.Seq[SQLPlanMetric]()
var submissionTime = -1L
var completionTime: Option[Date] = None
var errorMessage: Option[String] = None
Expand Down
2 changes: 1 addition & 1 deletion sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusStore.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class SQLExecutionUIData(
val details: String,
val physicalPlanDescription: String,
val modifiedConfigs: Map[String, String],
val metrics: Seq[SQLPlanMetric],
val metrics: collection.Seq[SQLPlanMetric],
val submissionTime: Long,
val completionTime: Option[Date],
val errorMessage: Option[String],
Expand Down
2 changes: 1 addition & 1 deletion ...re/src/main/scala/org/apache/spark/status/protobuf/sql/SQLExecutionUIDataSerializer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class SQLExecutionUIDataSerializer extends ProtobufSerDe {
getOptional(ui.hasCompletionTime, () => new Date(ui.getCompletionTime))
val errorMessage = getOptional(ui.hasErrorMessage, () => ui.getErrorMessage)
val metrics =
ui.getMetricsList.asScala.map(m => SQLPlanMetricSerializer.deserialize(m)).toSeq
ui.getMetricsList.asScala.map(m => SQLPlanMetricSerializer.deserialize(m))

@LuciferYang LuciferYang Dec 27, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no toSeq, one operation is about 10ns. After adding toSeq, the delay will increase linearly with the length of the original data:

  • 240ns when input length is 10
  • 1740ns when input length is 100
  • 16600ns when input length is 1000

val jobs = ui.getJobsMap.asScala.map {
case (jobId, status) => jobId.toInt -> JobExecutionStatus.valueOf(status.toString)
}.toMap
Expand Down