RFR: 8055: Improvement in stacktrace view for Duration [v6]

Alex Macdonald aptmac at openjdk.org
Tue Jun 6 18:39:12 UTC 2023


On Tue, 6 Jun 2023 17:07:21 GMT, Suchita Chaturvedi <schaturvedi at openjdk.org> wrote:

>> 1. Added a new column for Duration. It will be populated only when the "Show Duration" toggle icon is clicked. Not by default.
>> 2. Fixed NaN issue for screens where duration is not present.
>> 3. Added timeunit to make tooltip more meaningful.
>> 
>> Please refer JIRA description for more details.
>
> Suchita Chaturvedi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Implemented the review comments to handle ticks better

This is looking a lot better on my end now, thanks for making the changes. I have a couple of points around formatting but otherwise the functionality looks good.

application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/views/stacktrace/StacktraceView.java line 993:

> 991: 
> 992: 	private String formatDuration(long duration) {
> 993: 		Duration rawDuration = null;

`Duration.ofNanos()` will not return null [[0]](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/time/Duration.html#ofNanos(long)), so there's no need for a null check below, and it should be okay to just make `Duration rawDuration = Duration.ofNanos(duration);`

[0] https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/time/Duration.html#ofNanos(long)

application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/views/stacktrace/StacktraceView.java line 997:

> 995: 		rawDuration = Duration.ofNanos(duration);
> 996: 		if (rawDuration != null) {
> 997: 			formattedTime = String.format("%d h %d m %d s %d ms %d ns", rawDuration.toHoursPart(),

Could just return this and remove the need of `String formattedTime`

-------------

PR Review: https://git.openjdk.org/jmc/pull/475#pullrequestreview-1465778613
PR Review Comment: https://git.openjdk.org/jmc/pull/475#discussion_r1220084455
PR Review Comment: https://git.openjdk.org/jmc/pull/475#discussion_r1220085960


More information about the jmc-dev mailing list