-
Notifications
You must be signed in to change notification settings - Fork 37
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.
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
Handle SparkRapidsBuildInfoEvent in GPU event logs #1203
Handle SparkRapidsBuildInfoEvent in GPU event logs #1203
Conversation
Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you update the description to include what the solutions is here, how does user see this information.
core/src/main/scala/org/apache/spark/sql/rapids/tool/util/EventUtils.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @cindyyuanjiang
I believe we want to keep the runtimeReport intact.
-
The information of the RAPIDS plugin is per-app, because it is exctracted from the eventlog. Therefore, we expect that to be in the Profiler output of the individual application
-
We need to maintain backward compatibility. If we profile an older GPU eventlog, we should still build the RAPIDs information. We can use a common object that gets populated either from the eventhandler or from parsing the Spark properties. Eventually the support of the legacy method will be dropped once all customers are using the most recent RAPIDs versions
-
The information needs to be passed to the output and to the autotuner as well.
Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
core/src/main/scala/com/nvidia/spark/rapids/tool/profiling/ProfileOutputWriter.scala
Outdated
Show resolved
Hide resolved
core/src/main/scala/org/apache/spark/sql/rapids/tool/util/EventUtils.scala
Outdated
Show resolved
Hide resolved
core/src/main/scala/org/apache/spark/sql/rapids/tool/EventProcessorBase.scala
Outdated
Show resolved
Hide resolved
Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looks good, it would be nice to have a test to verify that it outputs the file and has valid data when the event is present. Maybe that they are empty when the event isn't present in older ones.
core/src/main/scala/com/nvidia/spark/rapids/SparkRapidsBuildInfoEvent.scala
Show resolved
Hide resolved
core/src/main/scala/org/apache/spark/sql/rapids/tool/ToolUtils.scala
Outdated
Show resolved
Hide resolved
Signed-off-by: cindyyuanjiang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @cindyyuanjiang. LGTME. Similar comments as Tom.
Thanks @tgravescs! I added two test cases to verify |
Signed-off-by: cindyyuanjiang <[email protected]>
9370675
core/src/test/resources/spark-events-profiling/spark_rapids_build_info_eventlog
Outdated
Show resolved
Hide resolved
Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks Cindy!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @cindyyuanjiang
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTME
Thanks @cindyyuanjiang
Fixes #995
Changes
SparkRapidsBuildInfoEvent
added in Spark-RapidsSparkRapidsBuildInfoEvent
into filespark_rapids_build_info.json
underrapids_4_spark_profile/application_xxxxxx/
Testing
SparkRapidsBuildInfoEvent
spark_rapids profiling -v --eventlogs <my_gpu_log> --tools_jar <my_tools_jar>
spark_rapids_build_info.json
SparkRapidsBuildInfoEvent
spark_rapids profiling -v --eventlogs <my_gpu_log> --tools_jar <my_tools_jar>
spark_rapids_build_info.json
Follow up
#1296 to fix the hardcoded
SparkRapidsBuildInfoEvent