[RFC] Threads Page Improvements

Jie Kang jkang at redhat.com
Fri Nov 1 14:17:24 UTC 2019


Hey Alex,

Wow this is an extensive re-work of the page. It looks pretty neat.
I've put my comments below in a bullet-point sort of format. I
understand this is a WIP, hopefully not too many of the comments are
known and going to be addressed already :p

* The thread table can have text cutoff depending on the space given.
It would be good to add tooltips for the text content being shown.
Tooltips should also be added to the buttons that inform users of the
purpose (in case the icons aren't obvious)

* The orange bar at the bottom overlaps with larger system font sizes.

* Opinion: The zoom in and out selector buttons should just function
as zoom in/out. The pointer selector seems like it could be scrapped
then while the pop-out map enable/disable button can remain. For
example, having to click zoom in and then click on the view, when
there isn't any other view to click zoom in on doesn't make sense.
When user testing there were many times even after I realized the
functionality where I tried to zoom in or out by pressing the button
repeatedly.

* I believe the standard is not to add Red Hat copyright to existing
files, only to completely new ones

* The filter button to set the view's duration need not exist, it
should just update the view either when the user enters valid input or
after the user hits enter, after entering valid input

* For duration entry, it could be smarter and allow somewhat invalid
entries that could be augmented to be valid: e.g. 3:25 -> 03:25:00:00

* Opinion: Thread state selection should modify current Thread
Activity Lane profile rather than creating a new "Quick Filter".

* The view does not horizontally scroll or fit to window size like the
other pages so if the user has a small window, they cannot see a large
portion of the threads page. Either fit to window size like the other
pages, or allow for appropriate scrolling.

* I feel like you should be able to zoom out from the "100%" view,
mainly to see more threads on the screen at once. Is this even useful?
No idea...

* The zoom slider being visible but not usable is very strange. I'd
make it functional or drop it from the design

+               List<IRenderedRow> subdivision = row.getNestedRows();
// height 1450, has all 32 rows
* Can you elaborate on the comment added above.

```
                        resetButton.addListener(SWT.Selection, e ->
on.accept(false));
+                       resetButton.addListener(SWT.Selection, new Listener() {
+                               @Override
+                               public void handleEvent(Event event) {
+                                       on.accept(false);
+                               }
+                       });
```

* This added listener seems to be doing the same thing as the listener
added just before it. Am I misreading it this?

```
+       public Set<String> getEnabledLanes() {
+               List<IItemFilter> laneFilters = laneDefs.stream()
+                               .filter((Predicate<? super
LaneDefinition>) LaneDefinition::isEnabledAndNotRestLane).map(ld ->
ld.getFilter())
+                               .collect(Collectors.toList());
+               return ((Types) ItemFilters.or(laneFilters.toArray(new
IItemFilter[laneFilters.size()]))).getTypes();
+       }
```

This cast makes me wonder if there's a better way to get the
information needed. If necessary, maybe the underlying classes can be
augmented to provide it?


Regards,
Jie Kang


On Tue, Oct 22, 2019 at 9:31 AM Alex Macdonald <almacdon at redhat.com> wrote:
>
> Hi,
>
> Demo video: https://www.youtube.com/watch?v=H_szS2gfXlo
>
> GIFs: https://imgur.com/a/KhmQThl#RWmGCbW
>
> Repo (on top of JMC GitHub mirror): https://github.com/aptmac/jmc
>
> Webrev (on top of current Mercurial repo):
> http://cr.openjdk.java.net/~aptmac/jfr-threads-page/webrev.00/
>
> The following work has been in-development to improve upon the usability of
> the JFR Threads Page.
>
> While taking a look into the JIRA ticket for improving the Threads Page
> [0], Mario and I discussed some of the pain-points and use-cases of the
> Threads page with one of our UX teams. The end result of these discussions
> were some mock-ups that would leverage the existing components and codebase
> to make the chart and table more interactable. At a high-level, the idea
> was to relocate the existing table to regain screen real-estate, add a
> secondary canvas (Text Canvas) for rendering the Thread names, and add
> toolbars (x2) to help filter through the information and affect how it is
> displayed on the screen. In an attempt to not create a wall of text here
> describing the specific changes I’ll include some bullet-point notes
> towards the end of this e-mail that will go more into the changes and
> additions. Additionally, I have included a demo video at the top of this
> e-mail that goes over the changes, and an imgur album that contains GIFs of
> the features shown in the video. This work has been a combined effort from
> myself and Jessye, so if you have any specific questions about the
> implementation just let us know.
>
> Currently this update runs correctly on Fedora (Linux), but there seem to
> be a couple issues (with regards to painting) in Windows that I’ll be
> investigating shortly. We also don’t have easy access to Mac machines at
> the moment, so I’d be curious to see what it looks like and if it needs
> further modifications there too.
>
> For the most part the development of the functional elements is wrapped-up
> and we’re working on cleanup and testing. It’d be nice to have some
> feedback prior to writing a bunch of uitests just in case things have to be
> modified.
>
> These updates to the Threads Page also happen to coincide with (and
> address) a number of outstanding issues on the JIRA, such as:
>
> [0] JMC-6364: Improvements to the Thread Graph (Tracking bug)
>
> [1] JMC-4475: Zoom out to full range in chart is not obvious
>
>    -
>
>    SWT Scale in the display bar indicates current zoom power
>
> [2] JMC-5001: Difficult to unselect in graph
>
>    -
>
>    The ESC key now un-highlights all lanes
>
> [3] JMC-5301: Shorter events hidden if drawn in same pixels as a long one
>
>    -
>
>    Not completely addressed, but can toggle activity visibility to find
>    hidden events
>
> [4] JMC-6165: Missing thread names in graph view (Thread view)
>
>    -
>
>    There’s now a fixed amount of vertical space for each lane; the names
>    are always visible
>
> [5] JMC-6366: [JFR] Event graph, make label area resizable
>
>    -
>
>    Text and Chart canvases connected by SashForm; no more distinct SWT
>    table & AWT chart
>
> [6] JMC-6368: Foldable threads table in the Threads page
>
>    -
>
>    Table converted to a popup table to save on screen real-estate
>
>
> Summary of changes:
>
> - New: Chart Filter Bar (Top)
>
> - - Controls: time filters, reset button, dropdown thread activity filter
>
> - New: Dropdown Thread Activity Filter
>
> - - Dropdown combobox that allows for toggling of visible lane activity
>
> - New: Time Filter
>
> - - Displays for the start and end time of the visible range
>
> - - Can input specific start & end times to update the visible range
>
>
>
> - New: Chart Display Bar (Right)
>
> - - Controls: selection, zoom-in, zoom level indicator, zoom-out, zoom-pan,
> and scale-to-fit
>
> - New: Zoom-In Mode
>
> - - Zoom in chart incrementally on chart click
>
> - - Zoom in chart continuously on long press of button
>
> - - Zoom to selection automatically by highlighting area on chart
>
> - New: Zoom Level Indicator
>
> - - Vertical SWT Scale + Label to show the current level of zoom
>
> - New: Zoom-Out Mode
>
> - - Zoom out chart incrementally on chart click
>
> - - Zoom out chart continuously on long press of button
>
> - New: Zoom-Pan Functionality
>
> - - Moving the display moves the location of the chart view
>
> - Changed: Timeline Display
>
> - - Rendered timeline display previously located below the chart has been
> relocated to its own component
>
> - - The orange range indicator can be moved horizontally to pan the chart
> view
>
> - Changed: Chart Canvas
>
> - - Chart moved into a scrolled composite that allows for vertical
> scrolling from the chart canvas, horizontal scrolling from the timeline
> canvas
>
> - Changed: Text Canvas
>
> - - ESC key can be pressed to remove lane highlighting
>
> - Changed: Zoom & Pan Mechanics
>
> Let us know what you think!
>
> Cheers,
>
> Alex
>
> [0] https://bugs.openjdk.java.net/browse/JMC-6364
>
> [1] https://bugs.openjdk.java.net/browse/JMC-4475
>
> [2] https://bugs.openjdk.java.net/browse/JMC-5001
>
> [3] https://bugs.openjdk.java.net/browse/JMC-5301
>
> [4] https://bugs.openjdk.java.net/browse/JMC-6165
>
> [5] https://bugs.openjdk.java.net/browse/JMC-6366
>
> [6] https://bugs.openjdk.java.net/browse/JMC-6368


More information about the jmc-dev mailing list