RFR: JMC-4466 - Hide thread directly from Thread graph context menu

[Alex Macdonald]

On Fri, Apr 12, 2019 at 10:19 AM Mario Torre <neugens at redhat.com> wrote:

> On Fri, 2019-04-05 at 13:28 -0400, Alex Macdonald wrote:
> > Hi,
> >
> > The following webrev [0] addresses issue JMC-4466 [1], in which the
> > user
> > should be able to hide threads from the thread chart using context
> > menu
> > actions.
> >
> > This patch adds functionality to the context menu to hide threads
> > from the
> > chart. Additionally, I thought there should be an option to restore
> > the
> > chart to the current selection to "unhide" the threads, so I've also
> > included a menu item that does such that. I've included a GIF [2] to
> > demonstrate how the actions work (note the gif was made before I
> > touched up
> > some of the strings, so some of the wording may be off).
> >
> > The "hide thread" action is only enabled while there are threads that
> > can
> > be hidden, and the "reset chart" option is only enabled while the
> > chart has
> > been modified (i.e., has thread(s) hidden) [3][4]. I've also included
> > uitests that perform the new actions on the chart, and verify the
> > results
> > based on the enablement values of the menu items.
> >
> > Lastly, I've tested these tests and changes locally on my machine and
> > using
> > Travis [5], but these are both Linux environments and it would be
> > nice to
> > make sure there are no issues on Windows / Mac.
> >
> > Thoughts?
> >
> > Cheers,
> >
> > Alex
> >
> > [0] http://cr.openjdk.java.net/~aptmac/JMC-4466/webrev.00/
> > [1] https://bugs.openjdk.java.net/browse/JMC-4466
> > [2] https://imgur.com/BkeXkVX
> > [3] https://imgur.com/dhuTmkE
> > [4] https://imgur.com/W7NzOj7
> > [5] https://travis-ci.org/aptmac/jmc-qa/builds/516268811
>
> Hi Alex,
>
> The patch looks generally good, the only question I have is regarding
> the thread selection by name:
>
> + private int indexOfThreadName(String name) {
> +   for (int i = 0; i < this.threadRows.size() && name != null; i++) {
> +     if (this.threadRows.get(i) instanceof QuantitySpanRenderer) {
> +       if (name.equals(((QuantitySpanRenderer)
> +           this.threadRows.get(i)).getName())) {
> +             return i;
> +       }
> +     }
> +   }
> +  return -1;
> + }
>
> What happens when two threads have the same name? This will make
> disappear the very fist one with this name encountered, not necessarily
> the one the user clicked on, isn't it?
>

Ah good call, I hadn't thought about that case. I'll come back with a
revised webrev.


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

Thanks,

Alex