RFR: 7894: Provide an alternative Java based flamegraph visualization [v20]
Christoph Langer
clanger at openjdk.org
Sun Jun 11 20:28:08 UTC 2023
On Fri, 9 Jun 2023 16:46:09 GMT, Brice Dutheil <bdutheil at openjdk.org> wrote:
>> This change introduces a possible replacement for the current flamegraph view.
>>
>> **Motivation**
>> The current is based on a web view. In the current state,
>> - the web view can be slow to render especially when the tree is large.
>> - the web view don't feel well integrated, in particular when popups are shown.
>> - web view are difficult to work with from a JMC developer perspective.
>>
>> 
>>
>> **Description**
>> Concretely this PR relies on the swing component to render flamegraphs : https://github.com/bric3/fireplace. And plays with the bridge between SWT and AWT via the `SWT_AWT` class.
>>
>> As the intent of this view is to eventually replace the current one, the icons are the same.
>>
>> Since fireplace has no actual release, only snapshots, in order to try this PR, it is necessary to install the snapshot manually before starting the p2 server.
>>
>>
>> cd releng/third-party
>>
>> mvn dependency:get -DrepoUrl=https://s01.oss.sonatype.org/content/repositories/snapshots -Dartifact=io.github.bric3.fireplace:fireplace-swing:0.0.1-SNAPSHOT:jar
>>
>> # if sources are wanted
>> mvn dependency:get -DrepoUrl=https://s01.oss.sonatype.org/content/repositories/snapshots -Dartifact=io.github.bric3.fireplace:fireplace-swing:0.0.1-SNAPSHOT:jar:sources
>>
>> # make the p2 site, don't forget the -U
>> mvn p2:site -U; mvn jetty:run
>>
>>
>> **Outstanding issues / limitation**
>> - [x] Fireplace has not yet a stable release as some part of its API are a bit rough.
>> - [x] Currently the view does not initializes correctly:
>> the swing `JScrollPane` don't show scroll bars, until the view is resized by the user. I lack the SWT / Swing expertise to understand why at this time.
>> https://github.com/bric3/fireplace/issues/79
>> - [x] Fireplace only supports _icicle_ view at this time.
>> https://github.com/bric3/fireplace/issues/22
>> - [x] Icons for minimap toggle and zoom reset
>> <img width="237" alt="image" src="https://user-images.githubusercontent.com/803621/183126781-07af0e94-a5d5-458c-97e3-a5e012177248.png">
>> - [x] Export to image
>> https://github.com/bric3/fireplace/issues/99
>> - [x] Export to print ?
>> Can be done at a later time
>> - [x] release of fireplace
>
> Brice Dutheil has updated the pull request incrementally with one additional commit since the last revision:
>
> fix: Make spotless happy
No review for the implementation from my end but some meta info comments.
application/org.openjdk.jmc.flightrecorder.flamegraph/build.properties line 1:
> 1: source.. = src/main/java/
Any particular reason why src/main/resources is missing here now?
application/org.openjdk.jmc.flightrecorder.flamegraph/plugin.properties line 3:
> (failed to retrieve contents of file, check the PR for context)
Since you merely move the files from one project to another, wouldn't it be more appropriate to use copyright `...2019, 2023, ...` here?
application/org.openjdk.jmc.flightrecorder.flamegraph/plugin.xml line 3:
> 1: <?xml version="1.0" encoding="UTF-8"?>
> 2: <!--
> 3: Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
Same here.
application/org.openjdk.jmc.flightrecorder.flamegraph/pom.xml line 54:
> 52: <plugins>
> 53: </plugins>
> 54: </build>
I guess the build section is not necessary here
releng/platform-definitions/platform-definition-2022-09/pom.xml line 3:
> 1: <?xml version="1.0" encoding="UTF-8"?>
> 2: <!--
> 3: Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
Not necessary, no change in file.
-------------
Changes requested by clanger (Committer).
PR Review: https://git.openjdk.org/jmc/pull/408#pullrequestreview-1473815496
PR Review Comment: https://git.openjdk.org/jmc/pull/408#discussion_r1225917562
PR Review Comment: https://git.openjdk.org/jmc/pull/408#discussion_r1225917756
PR Review Comment: https://git.openjdk.org/jmc/pull/408#discussion_r1225917783
PR Review Comment: https://git.openjdk.org/jmc/pull/408#discussion_r1225917850
PR Review Comment: https://git.openjdk.org/jmc/pull/408#discussion_r1225918457
More information about the jmc-dev
mailing list