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

Tue Dec 4 10:32:04 UTC 2018

Trying that again. Heh.

Hi Mario,

Agreed. That said, there is probably a difference in how to render a stand alone 
default package in a human readable way, and what the core API itself returns. 
The API needs to return a valid package name, possibly the empty string (or 
null) for the default package. When we _render_ the default package, it depends 
on the context how we probably want to render. As part of a FQN, it should be 
the empty string. As a stand alone package only rendering in the 
StackTrace-view, when grouping on package, possibly something like <default>, 
to underline that there is a package and not a missing data error.

Kind regards,
Marcus

-----Ursprungligt meddelande-----
Från: jmc-dev <jmc-dev-bounces at openjdk.java.net> För Mario Torre
Skickat: den 4 december 2018 08:47
Till: Alex Macdonald <almacdon at redhat.com>
Kopia: jmc-dev at openjdk.java.net
Ämne: Re: JMC-6149: Distinguish by package with default packages in the traces does not work

Hi Alex,

I pushed the fix.

While checking it out, I thought it would be nice to use a different indication for the default package, "null" sounds weird, especially since the package name isn't really "null", at most an empty string.

Do you think we can follow up this fix with some other string, perhaps something like "Default Package"? Marcus, do you think this is a good idea?

Cheers,
Mario
On Mon, Dec 3, 2018 at 8:59 PM Alex Macdonald <almacdon at redhat.com> wrote:
>
> 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.j
>> ava#l219
>>
>>
>>
>>

--
Mario Torre
Associate Manager, Software Engineering
Red Hat GmbH <https://www.redhat.com>
9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898