JMC-6256: JMC doesn't respect Long.MIN_VALUE as a missing value

[Marcus Hirt]

Hi Alex,

Quick comment before going to bed:

I don't believe we should externalize Integer.MIN_VALUE etc. They are defined 
in the java core libraries. One could argue for MISSING_VALUE and 
MISSING_VALUE_TOOLTIP to be externalized though.

Kind regards,
Marcus


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:

    Hi!
    
    This patch addresses JMC-6256 [0], in which MIN_VALUEs are displayed in
    their numeric form instead of being displayed as a N/A or missing value.
    For testing purposes, I have been using the memoryleak.jfr recording that
    is available as an attachment on JMC-6127 [1].
    
    As per the comments in the bug report, there are 6 cases to consider for
    missing values in JMC [2]. Additionally, it may be nice to show the actual
    value in parenthesis in the tooltip [3].
    
    Screenshots (album [4]):
    Before: https://imgur.com/aoGgzRn [5]
    After: https://imgur.com/DVnHQGZ [6]
    
    The culprit here seems to be the way the conditionals in the
    "getValueString()" [7][8] end up processing the value. There is a
    duplication of the "instanceof IDisplayable" in the JfrPropertySheet [7]
    and the TypeHandling [8], and the TypeHandling.getValueString() ends up
    being called anyways in the JfrPropertySheet.getValueString(). As a result,
    the conditional to see if it's an IDisplayable from the JfrPropertySheet
    happens before the conditional to see if it is a min value [9] (which is
    where we want to end up in this case).
    
    This patch removes the duplicated conditional from the JfrPropertySheet,
    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 figure
    out which missing value type it is, and display it in the tooltip. If the
    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".
    
    Thoughts?
    
    Cheers,
    
    Alex
    
    [0] https://bugs.openjdk.java.net/browse/JMC-6256
    [1] https://bugs.openjdk.java.net/browse/JMC-6127
    [2]
    https://bugs.openjdk.java.net/browse/JMC-6256?focusedCommentId=14229297&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14229297
    [3]
    https://bugs.openjdk.java.net/browse/JMC-6256?focusedCommentId=14229296&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14229296
    [4] https://imgur.com/a/3BZNHnG
    [5] https://imgur.com/aoGgzRn
    [6] https://imgur.com/DVnHQGZ
    [7]
    http://hg.openjdk.java.net/jmc/jmc/file/a76a464b3764/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/JfrPropertySheet.java#l307
    [8]
    http://hg.openjdk.java.net/jmc/jmc/file/efeb1e97bec3/core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/util/TypeHandling.java#l171
    [9]
    http://hg.openjdk.java.net/jmc/jmc/file/efeb1e97bec3/core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/util/TypeHandling.java#l167