RFR: 8223697: jfr tool can't format duration values greater than 1 minute

[Chihiro Ito]

Hi Erik,

Thank you for reviewing.

I fixed the formatter strings.

webrev http://cr.openjdk.java.net/~cito/JDK-8223697/webrev.01/

This fix passed test-tier1 and test/jdk/jdk/jfr.

If you want to take on other formatting issues as well. The tool should:
> - round values correctly, i.e 60.6 s should become 61 s instead of 60 s.
> - work with negative durations.
> - handle larger units, i.e. minutes, hours and days. I don't think we need
> weeks, months or years.  To make it easy to read, we shouldn't use
> fractions. i.e "1 h 22 m" instead of "1.36 h"
> The above could be done in a separate bug, but maybe more convenient to do
> in the same bug for back porting purposes.


I also think we should improve these. However, it should be fixed as a new
enhancement, not as a bug this time.
We will also need to clarify the rules of formatter strings and to consider
whether to apply these fixes to XML and JSON.
If these are decided, I will fix them.

Regards,
Chihiro


2019年10月3日(木) 1:56 Erik Gahlin <erik.gahlin at oracle.com>:

> Hi Chihiro,
>
> Hi Erik,
>
> I think we should use Duration instead of fixing the expression.
> Also, the if statement using s is complex, so I simplified it for each
> unit.
>
> OK
>
>
>
> You wrote on JBS as follows, but if I modify only the expression, 1234.567
> seconds will be output as 1234.0. Is this correct?
>
> For example, Duration.ofMillis (1234567) economies "34.567. s" instead of
> "1234.567 s"
>
> Over 1000 seconds, 0.1 seconds is too small. Therefore, I think even ".0"
> is not necessary.
>
> I agree, we should leave out fractions on larger values, i.e.
> format("%d",...)
>
> If you want to take on other formatting issues as well. The tool should:
>
> - round values correctly, i.e 60.6 s should become 61 s instead of 60 s.
> - work with negative durations.
> - handle larger units, i.e. minutes, hours and days. I don't think we need
> weeks, months or years.  To make it easy to read, we shouldn't use
> fractions. i.e "1 h 22 m" instead of "1.36 h"
>
> The above could be done in a separate bug, but maybe more convenient to do
> in the same bug for back porting purposes. Any way you like.
>
> The unit could be added to the formatter string, i.e. String.format("%.3f
> us", (double) d.toNanos() / 1_000));
>
> Thanks
> Erik
>
>
> Regards,
> Chihiro
>
> 2019年9月27日(金) 18:32 Erik Gahlin <erik.gahlin at oracle.com>:
>
>> Hi Chihiro,
>>
>> The bug seems to be that:
>>
>> double s = d.toNanosPart() / 1000_000_000.0 + d.toSecondsPart();
>>
>> should be:
>>
>> double s = d.toNanosPart() / 1000_000_000.0 + d.toSeconds();
>>
>> The other changes is not strictly necessary. Is that correct?
>>
>> Thanks
>> Erik
>>
>> > Hi,
>> >
>> > Can anyone review this?
>> >
>> > JBS https://bugs.openjdk.java.net/browse/JDK-8223697
>> > webrev http://cr.openjdk.java.net/~cito/JDK-8223697/webrev.00/
>> >
>> > Regards,
>> > Chihiro
>> >
>> >
>> > 2019年9月14日(土) 0:06 Chihiro Ito < <chiroito107 at gmail.com>
>> chiroito107 at gmail.com>:
>> >
>> >> Hi,
>> >>
>> >> Could you review this tiny change, please?
>> >>
>> >> http://cr.openjdk.java.net/~cito/JDK-8223697/webrev.00/
>> >>
>> >> Regards,
>> >> Chihiro
>> >>
>>
>>
>