Roofline: single-click filtering for kernels, memory levels, and bandwidth peaks#950
Open
dhingora-amd wants to merge 4 commits into
Open
Roofline: single-click filtering for kernels, memory levels, and bandwidth peaks#950dhingora-amd wants to merge 4 commits into
dhingora-amd wants to merge 4 commits into
Conversation
Add a "Kernel intensity" dropdown (All / HBM / L2 / L1 / LDS) to the roofline Options menu so kernel intensity dots can be limited to a single memory level. This declutters the plot versus showing every level's dot per kernel, making it easier to see which roof limits the kernels. The filter is applied consistently to the plotted dots, the legend/options list, and hover hit-testing. Defaults to All (unchanged behavior). Co-authored-by: Cursor <cursoragent@cursor.com>
Add single-click kernel isolation to the workload (all-kernels) roofline: clicking a kernel's dot in the plot or its row in the legend shows only that kernel's intensity dots; clicking it again restores the whole workload. This makes it easy to switch between whole-workload and single-kernel analysis. The dot click works in any zoom state (with a drag check so panning a zoomed plot is not treated as a click) and stacks with the memory-peak filter. Isolation resets when the workload changes. Co-authored-by: Cursor <cursoragent@cursor.com>
Add single-click bandwidth-peak isolation to the roofline: clicking a bandwidth ceiling row in the legend, or its line in the plot, shows only that peak (HBM/L2/L1/LDS) and hides the others. Clicking it again restores all peaks. The legend keeps all bandwidth rows and highlights the active one, so peaks can be switched with a single click; the plot line toggles the shown peak (a drag check keeps a zoomed-plot pan from counting as a click). Compute ceilings and kernel dots are unaffected, and this stacks with the memory-peak filter and kernel isolation. Isolation resets on workload change. Co-authored-by: Cursor <cursoragent@cursor.com>
dhingora-amd
requested review from
Sajeeth-Wimalasuriyan,
amokiche-amd,
drchen-amd and
tomk-amd
as code owners
July 22, 2026 18:51
Member
|
Controls should be moved under presets header. Instead of the 6 buttons we have now, there should be a dropdown for each of the filters you added
The click shortcut for 2 & 3 can stay to compliment the dropdowns since they are not easily discoverable by themselves. |
drchen-amd
reviewed
Jul 22, 2026
This comment was marked as resolved.
This comment was marked as resolved.
…ility Address roofline PR review feedback: - Replace the preset buttons and scattered filter controls with dropdowns under the Presets header: Preset, Kernel intensity, Kernel, and Bandwidth peak. The dot/line/legend click shortcuts still complement them. - Drive all filtering through each item's visibility (like ApplyPreset) instead of separate display predicates, so the Options menu always lists the full data set; only the chart, legend, and hover react to filters. - Recompute visibility sets m_options_changed so ceiling changes re-fit the ridges (fixes bandwidth-peak discontinuities). - Offer only the intensity/bandwidth levels actually present in the workload. - Elide long kernel names in the Kernel dropdown so its popup stays put. Co-authored-by: Cursor <cursoragent@cursor.com>
drchen-amd
reviewed
Jul 23, 2026
| m_isolated_kernel = nullptr; | ||
| RecomputeVisibility(); | ||
| } | ||
| for(const KernelInfo* kernel : m_workload->ordered_kernels) |
Member
There was a problem hiding this comment.
Some kernels do not have roofline data (kernel.roofline.intensities.empty()) and should not be in this list.
| ImVec2(menus_content_width, 0), | ||
| ImVec2(menus_content_width, | ||
| max_menus_height - header_height - footer_height)); | ||
| std::max(0.0f, max_menus_height - header_height - footer_height))); |
Member
There was a problem hiding this comment.
Is this needed? It breaks the custom region when the height is small.
Member
|
I think the order and names of the dropdowns should be changed to:
Thoughts @tomk-amd? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The roofline shows every kernel's dot for every memory level plus all
bandwidth ceilings at once, which is noisy and takes many clicks to pare
down. This PR adds single-click ways to focus the view; they compose and
don't change the default.
Technical Details
All in
src/view/src/compute/rocprofvis_compute_roofline.{h,cpp}:limits kernel dots to one memory level.
kernel; click again to restore all.
only that peak; the legend keeps all rows and highlights the active one.
Filters apply to the plot, legend, and hover; plot clicks use a drag check so
panning isn't treated as a click; state resets on workload change.