[PATCH] JMC-5128/JMC-5149 - Utilize EventThread field in GC Phase Events

Mon Aug 20 16:09:44 UTC 2018

Hi Marcus,

I've updated the patch. It now checks if the pause events have thread
information before proceeding with the rendering, as well as disabling the
thread activity actions if there is no thread information present in the
events, as in that situation there is no point to those actions.

Cheers,

- Josh

On Fri, Aug 17, 2018 at 12:49 PM, Marcus Hirt <marcus.hirt at oracle.com>
wrote:

> Hi Josh,
>
> In older recordings, the thread information may not be available:
> <-->
> java.lang.NullPointerException
>         at org.openjdk.jmc.flightrecorder.ui.pages.GarbageCollectionsPage$
> GarbageCollectionsUi.buildChart(GarbageCollectionsPage.java:515)
>         at org.openjdk.jmc.flightrecorder.ui.pages.GarbageCollectionsPage$
> GarbageCollectionsUi.lambda$0(GarbageCollectionsPage.java:255)
>         at org.openjdk.jmc.ui.handlers.ActionToolkit$3.run(
> ActionToolkit.java:166)
>         at org.openjdk.jmc.ui.misc.ActionUiToolkit.lambda$4(
> ActionUiToolkit.java:129)
>         at org.eclipse.jface.viewers.CheckboxTableViewer$1.run(
> CheckboxTableViewer.java:212)
>         at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
>         at org.eclipse.ui.internal.JFaceUtil.lambda$0(JFaceUtil.java:44)
> <-->
>
> Ping me on IRC and I can share a representative recording with you!
>
> Kind regards,
> Marcus
>
> From: Joshua Matsuoka <jmatsuok at redhat.com>
> Date: Tuesday, 14 August 2018 at 17:25
> To: Marcus Hirt <marcus.hirt at oracle.com>
> Cc: <jmc-dev at openjdk.java.net>
> Subject: Re: [PATCH] JMC-5128/JMC-5149 - Utilize EventThread field in GC
> Phase Events
>
> Hi Marcus,
>
> Heres an updated patch, I've removed the thread table and made the thread
> activity lane dependent on the table selection.
>
> Cheers,
>
> - Josh
>
> On Mon, Aug 13, 2018 at 4:07 PM, Joshua Matsuoka <mailto:
> jmatsuok at redhat.com> wrote:
> Hi Marcus,
>
> Sounds good to me! I agree the thread chart seems a little odd looking at
> it again. I'll remove it but keep the activity lane as you suggested.
>
> Would you be opposed to a separate patch adding the thread lane context
> menu entries to the application page? It seems inconsistent with the other
> uses of activity lanes.
>
> Cheers,
>
> - Josh
>
> On Mon, Aug 13, 2018 at 3:59 PM, Marcus Hirt <mailto:
> marcus.hirt at oracle.com> wrote:
> Hi Joshua,
>
> I agree that the thread field should be added to the phases table. I worry
> about having a threads table in the beginning of the page, as it moves the
> focus from GCs to threads. If you are only interested in thread halts in
> relation to other events in the recording, you should really go to the
> Application page. Then you see all the halts (for example other
> vm-operations), and not only the GC-related ones.
>
> You could keep the Thread Activity lane, but for example make it show
> events for the threads involved in whatever selection you do in the tables
> above (without having an explicit threads table).
>
> What do you think?
>
> Kind regards,
> Marcus
>
> From: Joshua Matsuoka <mailto:jmatsuok at redhat.com>
> Date: Monday, 13 August 2018 at 20:51
> To: Marcus Hirt <mailto:marcus.hirt at oracle.com>
> Cc: <mailto:jmc-dev at openjdk.java.net>
> Subject: Re: [PATCH] JMC-5128/JMC-5149 - Utilize EventThread field in GC
> Phase Events
>
> Hi Marcus,
>
> Thanks for the feedback!
>
> I was following the suggestions in the comments/history on
> https://bugs.openjdk.java.net/browse/JMC-5128 . I think there's value to
> having the thread lane able to be visualized alongside the rest of the GC
> information. It's nice being able to see the thread events/activity align
> with the pause visualization and heap activity. I can also seeing this be
> useful for users who may have custom events defined, where it may be useful
> to see these events alongside the heap/gc behaviour.  I noticed as well
> that the thread activity display on the Java Application page lacks the
> context menu options to display additional types of events in the
> visualization. Perhaps if we feel that these changes are unnecessary, I can
> make an additional patch that could add that functionality to the thread
> lane on the application page. In either case though, we should probably
> still add the event thread to the information displayed for pause events.
>
> Cheers,
>
> - Josh
>
> On Mon, Aug 13, 2018 at 11:56 AM, Marcus Hirt <mailto:mailto:marcus.hirt@
> oracle.com> wrote:
> Hi Josh,
>
> Thanks for the suggestion! The normal work process is to focus the UI on
> whatever is of interest, and then just go to the Applications view. That
> way
> we end up not having to replicate that functionality on every page.
>
> Can you tell me a little bit more about your use case and how you see this
> being used?
>
> (Good initiative! I think you're the first non-Oracle employee attempting
> to
> improve on the visualization, so kudos!)
>
> Kind regards,
> Marcus
> On 2018-08-13, 17:41, "jmc-dev on behalf of Joshua Matsuoka" <mailto:
> mailto:jmc-dev-bounces at openjdk.java.net on behalf of mailto:mailto:
> jmatsuok at redhat.com> wrote:
>
>     Hi,
>
>     This is a patch that addresses JMC-5128 and JMC-5149. Currently the
> event
>     thread field in the GC Phase Events isn't used by the UI. This patch
>     accomplishes the following:
>
>     - Adds this field to the phases table.
>     - Adds a Thread table similar to the Java Application page which
> displays
>     the threads that the phase events occurred on.
>     - Adds a Thread lane to visualize the thread selected by the above
> table.
>
>     This is a screenshot of the updated UI: https://imgur.com/a/CFQz9G6
>
>     Thoughts?
>
>     Cheers,
>
>     - Josh
>
>
>
>
>
>
>
>
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: JMC-5128-2.patch
Type: text/x-patch
Size: 7754 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/jmc-dev/attachments/20180820/928d58e9/JMC-5128-2.patch>