JMC-6256: JMC doesn't respect Long.MIN_VALUE as a missing value
almacdon at redhat.com
Thu Dec 13 21:15:26 UTC 2018
Whoops, just realized I hit reply instead of reply-all so this didn't get
sent to list, please find attached an updated patch.
On Thu, Dec 13, 2018 at 10:00 AM Alex Macdonald <almacdon at redhat.com> wrote:
> Hi Marcus,
> Thanks for the review and suggestion, I've amended the patch to keep the
> string constants inside TypeHandling.
> On Wed, Dec 12, 2018 at 4:48 PM Marcus Hirt <marcus.hirt at oracle.com>
>> Hi Alex,
>> Quick comment before going to bed:
>> I don't believe we should externalize Integer.MIN_VALUE etc. They are
>> in the java core libraries. One could argue for MISSING_VALUE and
>> MISSING_VALUE_TOOLTIP to be externalized though.
>> Kind regards,
>> On 2018-12-12, 22:10, "jmc-dev on behalf of Alex Macdonald" <
>> jmc-dev-bounces at openjdk.java.net on behalf of almacdon at redhat.com> wrote:
>> This patch addresses JMC-6256 , in which MIN_VALUEs are displayed
>> their numeric form instead of being displayed as a N/A or missing
>> For testing purposes, I have been using the memoryleak.jfr recording
>> is available as an attachment on JMC-6127 .
>> As per the comments in the bug report, there are 6 cases to consider
>> missing values in JMC . Additionally, it may be nice to show the
>> value in parenthesis in the tooltip .
>> Screenshots (album ):
>> Before: https://imgur.com/aoGgzRn 
>> After: https://imgur.com/DVnHQGZ 
>> The culprit here seems to be the way the conditionals in the
>> "getValueString()"  end up processing the value. There is a
>> duplication of the "instanceof IDisplayable" in the JfrPropertySheet
>> and the TypeHandling , and the TypeHandling.getValueString() ends
>> being called anyways in the JfrPropertySheet.getValueString(). As a
>> the conditional to see if it's an IDisplayable from the
>> happens before the conditional to see if it is a min value  (which
>> where we want to end up in this case).
>> This patch removes the duplicated conditional from the
>> because it is already correctly handled in TypeHandling.
>> Additionally, a
>> function "getNumericString" has been introduced for the display of the
>> tooltip. If the value is an IQuantity then "getNumericString" will
>> out which missing value type it is, and display it in the tooltip. If
>> value is an IQuantity but not a missing value, then it delegates to
>> TypeHandling.getValueString(). I've also included a quick unit test to
>> verify the behaviour of "getNumericString".
>>  https://bugs.openjdk.java.net/browse/JMC-6256
>>  https://bugs.openjdk.java.net/browse/JMC-6127
>>  https://imgur.com/a/3BZNHnG
>>  https://imgur.com/aoGgzRn
>>  https://imgur.com/DVnHQGZ
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 8827 bytes
Desc: not available
More information about the jmc-dev