-
Notifications
You must be signed in to change notification settings - Fork 174
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
7451: Add heat map view #337
Conversation
👋 Welcome back cimi! A progress list of the required criteria for merging this PR into |
Webrevs
|
@cimi this pull request can not be integrated into git checkout cimi/heatmap
git fetch https://git.openjdk.java.net/jmc master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push |
This looks great! :) |
application/org.openjdk.jmc.flightrecorder.heatmap/src/main/resources/heatmap.js
Show resolved
Hide resolved
application/org.openjdk.jmc.flightrecorder.heatmap/src/main/resources/heatmap.js
Outdated
Show resolved
Hide resolved
application/org.openjdk.jmc.flightrecorder.heatmap/src/main/resources/page.template
Show resolved
Hide resolved
@cimi This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 3 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@thegreystone) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
@cimi - this one is ready for /integrate. |
/integrate |
/sponsor |
Going to push as commit ba365fb.
Your commit was automatically rebased without conflicts. |
@thegreystone @cimi Pushed as commit ba365fb. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
This PR adds a new plugin (
flightrecorder.heatmap
) which renders a visualisation similar to the one in this Observable notebook.The code is simpler than both the flame view and the graph view, because of less rendering options and less dependencies - the heatmap only requires d3 and currently does not support any user configuration.
Each cell represents 100 milliseconds from the recording. We decided to automatically adjust the size of the cell so that the entire heatmap fits inside the window. This is different from the observable notebook which ensured each row represented 6s (i.e. it had a fixed number of 60 columns). The auto-sizing seems to work well with both small and large recordings, although we lose consistency when we resize the window and some periodic patterns are no longer visible when we vary the number of columns.
Feedback welcome, here's how this looks now:
(short recording - 2 min)
(long recording - 15 min)
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jmc pull/337/head:pull/337
$ git checkout pull/337
Update a local copy of the PR:
$ git checkout pull/337
$ git pull https://git.openjdk.java.net/jmc pull/337/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 337
View PR using the GUI difftool:
$ git pr show -t 337
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jmc/pull/337.diff