JMC-6149: Distinguish by package with default packages in the traces does not work

[Alex Macdonald]

Hi Marcus,

On Mon, Dec 3, 2018 at 10:40 AM Marcus Hirt <marcus.hirt at oracle.com> wrote:

> Looks good Alex!
>
> Kind regards,
> Marcus
>

Great! Thanks for reviewing it.

I've attached an exported patch ready for pushing, Mario are you able to
sponsor this one?

Cheers,

Alex


>
> On 2018-12-03, 16:25, "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-6149 [0], in which the stack trace tree
> disappears
>     when a default package is encountered when selecting the option
>     "Distinguish Frames by Package". For testing purposes, I have been
> using
>     the memoryleak.jfr recording that is available as an attachment on
> JMC-6127
>     [1]
>
>     The problem here is a NPE being thrown when checking whether if the
>     category object equals the cached lastCategory object [2]. If the
> current
>     frame has no corresponding package then the package attribute will be
> null
>     [3], which then causes "category" to be null.
>
>     In StructTypes.java [4], there are precautions in place for handling
>     unknown methods and classes [5] so null values aren't passed around,
> but
>     nothing in place to handle unknown packages. This patch introduces a
> null
>     check in the IMCType implementation to mimic how the method [6] and
> class
>     [7] implementations currently handle null values. Additionally, a null
>     check must be made in the convertNames() and getName() methods that
> access
>     the package name, because if it is null then the
>     MethodToolkit.refTypeToBinaryJLS() [8] will throw an NPE when trying
> to get
>     the length of the name.
>
>     I've taken a couple screenshots to show the before and after of this
> patch,
>     as well as a couple gifs to show how to reproduce the behaviour. Link
> to
>     imgur album: https://imgur.com/a/0hNekjh
>
>     Before (image): https://imgur.com/VnvwMkQ
>     Before (gif): https://imgur.com/gZcPDCF
>     After (image): https://imgur.com/TOyalSG
>     After (gif): https://imgur.com/7GaCMII
>
>     Thoughts?
>
>     Cheers,
>
>     Alex
>
>     [0] https://bugs.openjdk.java.net/browse/JMC-6149
>     [1] https://bugs.openjdk.java.net/browse/JMC-6127
>     [2]
>
> http://hg.openjdk.java.net/jmc/jmc/file/997060a24c42/core/org.openjdk.jmc.flightrecorder/src/main/java/org/openjdk/jmc/flightrecorder/stacktrace/StacktraceModel.java#l197
>     [3] https://imgur.com/ktUJvo5
>     [4]
>
> http://hg.openjdk.java.net/jmc/jmc/file/997060a24c42/core/org.openjdk.jmc.flightrecorder/src/main/java/org/openjdk/jmc/flightrecorder/internal/parser/v1/StructTypes.java
>     [5]
>
> http://hg.openjdk.java.net/jmc/jmc/file/997060a24c42/core/org.openjdk.jmc.flightrecorder/src/main/java/org/openjdk/jmc/flightrecorder/internal/parser/v1/StructTypes.java#l67
>     [6]
>
> http://hg.openjdk.java.net/jmc/jmc/file/997060a24c42/core/org.openjdk.jmc.flightrecorder/src/main/java/org/openjdk/jmc/flightrecorder/internal/parser/v1/StructTypes.java#l580
>     [7]
>
> http://hg.openjdk.java.net/jmc/jmc/file/997060a24c42/core/org.openjdk.jmc.flightrecorder/src/main/java/org/openjdk/jmc/flightrecorder/internal/parser/v1/StructTypes.java#l661
>     [8]
>
> http://hg.openjdk.java.net/jmc/jmc/file/997060a24c42/core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/util/MethodToolkit.java#l219
>
>
>
>
>