Skip to content

Conversation

@yaooqinn
Copy link
Member

@yaooqinn yaooqinn commented Sep 19, 2023

What changes were proposed in this pull request?

This PR draws a CPU Flame Graph by Java stack traces for executors and drivers. Currently, the Java stack traces is just a SNAPSHOT, not sampling at a certain frequency for a period. Sampling might be considered an upcoming feature out of the scope of this PR.

fg git

If you are new to flame graphs, there are also some references you can refer to learn about the basic concepts and details.

[1] Flame Graphs
[2] FLIP-165: Operator's Flame Graphs
[3] Java in Flames. mixed-mode flame graphs provide a… | by Netflix Technology Blog
[4] HProf

Pending features

This PR mainly focuses on the UI, independent of the profiling steps. What we might have in the future are:

  • Flame Graph Support For Task Thread Page which SPARK-45151 added
  • Add ProfilingExecutor(max, interval) message to profile whole executor
  • Add ProfileTask(taskId, max, interval) message to profile an certain task
  • Different views for on/off/full CPUs
  • Mixed mode profiling, which might rely upon some ext libs at runtime
  • And so on.

Why are the changes needed?

Performance is always an important design factor in Spark. It is desirable to provide better visibility into the distribution of CPU resources while executing user code alongside the Spark kernel. One of the most visually effective means to do that is Flame Graphs, which visually presents the data gathered by performance profiling tools used by developers for performance tuning their applications.

Does this PR introduce any user-facing change?

yes

How was this patch tested?

locally

Was this patch authored or co-authored using generative AI tooling?

no

@HyukjinKwon
Copy link
Member

Looks cool. cc @mridulm FYI

@zhengruifeng
Copy link
Contributor

awesome!

@mridulm
Copy link
Contributor

mridulm commented Sep 19, 2023

The UI looks nice ! Thanks for working on this @yaooqinn :-)

My main concern is around effectively capturing stack frames without safepoint bias, correlating it to the specific task, and for executor flamegraph which threads to capture. @HyukjinKwon might remember what we had built for Safari, but we did that outside of the scope of Spark due to limitations (which might not be relevant now to be honest).

This has been called out of scope - but details around what we planning to support (in the first cut) would be great to understand.

@yaooqinn
Copy link
Member Author

This PR mainly focuses on the UI, independent of the profiling steps. What we might have in the future are:

  • Flame Graph Support For Task Thread Page, which SPARK-45151 added
  • Add ProfilingExecutor(max, interval) message to profile the whole executor, which returns the same data structure with TriggerThreadDump message
  • Add ProfileTask(taskId, max, interval) message to profile a specific task, which returns the same data structure with TaskThreadDump message
  • Different views for on/off/full CPUs
  • Mixed mode profiling, which might rely upon some ext libs at runtime
  • And so on.

@rednaxelafx
Copy link
Contributor

One important aspect of flame graphs is the semantics of the "width" of the bars. It can be defined to mean anything, e.g. aggregated profiling ticks (i.e. number of samples) or wall clock duration etc.

What's the intended semantics of "width" here for the thread dump snapshot?

@yaooqinn
Copy link
Member Author

yaooqinn commented Sep 19, 2023

Currently, the width represents the number of samples. the CPU time is not gathered yet, see also SPARK-44896

@mridulm
Copy link
Contributor

mridulm commented Sep 19, 2023

This PR mainly focuses on the UI, independent of the profiling steps. What we might have in the future are:

  • Flame Graph Support For Task Thread Page, which SPARK-45151 added

SPARK-45151 added ability to fetch stack dump, not flame graph support - and more importantly, from point of view of flamegraphs and performance analysis, the stack dump returned suffers from safepoint bias.

If this is an initial implementation, which we plan to refine over 4.0 release, that sounds fine to me - at a minimum, users/deployments should have the ability to override how the stack dump - for flamegraphs not SPARK-45151 - is generated.

  • Add ProfilingExecutor(max, interval) message to profile the whole executor, which returns the same data structure with TriggerThreadDump message

We can iterate more on this when we have a PR for it.
Thanks for the details.

  • Add ProfileTask(taskId, max, interval) message to profile a specific task, which returns the same data structure with TaskThreadDump message
  • Different views for on/off/full CPUs
  • Mixed mode profiling, which might rely upon some ext libs at runtime
  • And so on.

@mridulm
Copy link
Contributor

mridulm commented Sep 19, 2023

+CC @thejdeep as well.

@rednaxelafx
Copy link
Contributor

Currently, the width represents the number of samples.

In the current implementation in this PR, the width is essentially representing the "number of threads" that's sharing the same bottom portion of the stack, from a single snapshot of the threads dump, right?

@yaooqinn
Copy link
Member Author

@rednaxelafx Yes

@yaooqinn yaooqinn changed the title [WIP][SPARK-45209][CORE][UI] Flame Graph Support For Executor Thread Dump Page [SPARK-45209][CORE][UI] Flame Graph Support For Executor Thread Dump Page Sep 20, 2023
@beliefer
Copy link
Contributor

@yaooqinn Looks beautiful!

@yaooqinn
Copy link
Member Author

Thank you @beliefer

Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

+1, LGTM
Very cool features should be very helpful to frontline maintenance engineers.

@LuciferYang
Copy link
Contributor

Merged into master for Spark 4.0.
Thanks @yaooqinn
Thanks for your review. @HyukjinKwon @zhengruifeng @rednaxelafx @mridulm @beliefer

@cloud-fan
Copy link
Contributor

This is a great feature! can we add a config to turn it on/off? I just worried about any possible bugs and stop the UI from working.

@yaooqinn yaooqinn deleted the SPARK-45209 branch November 6, 2023 08:40
Mrhs121 pushed a commit to Mrhs121/spark that referenced this pull request Jan 2, 2024
…Page

### What changes were proposed in this pull request?

This PR draws a CPU Flame Graph by Java stack traces for executors and drivers. Currently, the Java stack traces is just a SNAPSHOT, not sampling at a certain frequency for a period. Sampling might be considered an upcoming feature out of the scope of this PR.

![fg git](https://github.com/apache/spark/assets/8326978/c3f99a1a-78ee-4adb-be1f-e4afd5f307b7)

If you are new to flame graphs, there are also some references you can refer to learn about the basic concepts and details.

[1] [Flame Graphs](https://www.brendangregg.com/flamegraphs.html)
[2] [FLIP-165: Operator's Flame Graphs](https://cwiki.apache.org/confluence/display/FLINK/FLIP-165%3A+Operator%27s+Flame+Graphs)
[3] [Java in Flames. mixed-mode flame graphs provide a… | by Netflix Technology Blog](https://netflixtechblog.com/java-in-flames-e763b3d32166)
[4] [HProf](https://docs.oracle.com/javase/7/docs/technotes/samples/hprof.html)

#### Pending features

This PR mainly focuses on the UI, independent of the profiling steps. What we might have in the future are:

- Flame Graph Support For Task Thread Page which SPARK-45151 added
- Add `ProfilingExecutor(max, interval)` message to profile whole executor
- Add `ProfileTask(taskId, max, interval)` message to profile an certain task
- Different views for on/off/full CPUs
- Mixed mode profiling, which might rely upon some ext libs at runtime
- And so on.

### Why are the changes needed?

Performance is always an important design factor in Spark. It is desirable to provide better visibility into the distribution of CPU resources while executing user code alongside the Spark kernel. One of the most visually effective means to do that is [Flame Graphs](http://www.brendangregg.com/FlameGraphs/cpuflamegraphs.html), which visually presents the data gathered by performance profiling tools used by developers for performance tuning their applications.

### Does this PR introduce _any_ user-facing change?

yes

### How was this patch tested?

locally
### Was this patch authored or co-authored using generative AI tooling?

no

Closes apache#42988 from yaooqinn/SPARK-45209.

Authored-by: Kent Yao <[email protected]>
Signed-off-by: yangjie01 <[email protected]>

(cherry picked from commit a073bf3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants