Review Request for JMC-5923: Add allocation pressure column to stacktrace view

[Salman Siddiqui]

Hi all,

Here is a webrev for the patch:
https://cr.openjdk.java.net/~aptmac/JMC-5923/webrev.00/

Thanks,
Salman

On Thu, Jul 25, 2019 at 9:42 PM Salman Siddiqui <sasiddiq at redhat.com> wrote:

> Hi all,
>
> Here is patch that adds a column to the stacktrace view to display
> normalized allocation pressure. The allocation pressure is a ratio of the
> size of the allocations of a particular class to the size of total
> allocations from that stacktrace frame.
>
> Also, as per suggestion from Marcus, I have normalized the count that is
> displayed in the stacktrace view (this is similar to what is already being
> displayed in the tool tip for the count).
>
> JIRA issue: https://bugs.openjdk.java.net/browse/JMC-5923
> Related GitHub issue: https://github.com/JDKMissionControl/jmc/issues/6
>
> Patch is below.
>
> Thanks,
> Salman
>
>
> diff --git
> a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/messages/internal/Messages.java
> b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/messages/internal/Messages.java
> index 7b576f3..cd8234a 100644
> ---
> a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/messages/internal/Messages.java
> +++
> b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/messages/internal/Messages.java
> @@ -435,6 +435,7 @@ public class Messages extends NLS {
>   public static String SELECT_RANGE_WIZARD_TEXT;
>   public static String SELECT_RANGE_WIZARD_TITLE;
>   public static String SELECT_RANGE_WIZARD_TO_MUCH_SELECTED_WARNING;
> + public static String STACKTRACE_VIEW_ALLOC_PRESSURE_COLUMN_NAME;
>   public static String STACKTRACE_VIEW_COUNT_COLUMN_NAME;
>   public static String STACKTRACE_VIEW_DISTINGUISH_FRAMES_BY;
>   public static String STACKTRACE_VIEW_FRAME_GROUP_CHOOSE;
> diff --git
> a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/views/stacktrace/StacktraceView.java
> b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/views/stacktrace/StacktraceView.java
> index 2b51e80..9d96016 100644
> ---
> a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/views/stacktrace/StacktraceView.java
> +++
> b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/views/stacktrace/StacktraceView.java
> @@ -92,10 +92,16 @@ import org.eclipse.ui.part.ViewPart;
>  import org.openjdk.jmc.common.IDisplayable;
>  import org.openjdk.jmc.common.IMCFrame;
>  import org.openjdk.jmc.common.IState;
> +import org.openjdk.jmc.common.collection.IteratorToolkit;
>  import org.openjdk.jmc.common.collection.SimpleArray;
> +import org.openjdk.jmc.common.item.IItem;
>  import org.openjdk.jmc.common.item.IItemCollection;
> +import org.openjdk.jmc.common.item.IItemFilter;
> +import org.openjdk.jmc.common.item.ItemFilters;
>  import org.openjdk.jmc.common.unit.UnitLookup;
>  import org.openjdk.jmc.common.util.StateToolkit;
> +import org.openjdk.jmc.flightrecorder.jdk.JdkAggregators;
> +import org.openjdk.jmc.flightrecorder.jdk.JdkFilters;
>  import org.openjdk.jmc.flightrecorder.stacktrace.FrameSeparator;
>  import
> org.openjdk.jmc.flightrecorder.stacktrace.FrameSeparator.FrameCategorization;
>  import org.openjdk.jmc.flightrecorder.stacktrace.StacktraceFormatToolkit;
> @@ -181,7 +187,7 @@ public class StacktraceView extends ViewPart
> implements ISelectionListener {
>   private static final Color COUNT_COLOR = SWTColorToolkit.getColor(new
> RGB(100, 200, 100));
>   private static final String SIBLINGS_IMG_KEY = "siblingsColor";
> //$NON-NLS-1$
>   private static final Color SIBLINGS_COUNT_COLOR =
> SWTColorToolkit.getColor(new RGB(170, 250, 170));
> - private static final int[] DEFAULT_COLUMN_WIDTHS = {700, 150};
> + private static final int[] DEFAULT_COLUMN_WIDTHS = {700, 150, 150};
>   private static final String THREAD_ROOT_KEY = "threadRootAtTop";
> //$NON-NLS-1$
>   private static final String FRAME_OPTIMIZATION_KEY =
> "distinguishFramesByOptimization"; //$NON-NLS-1$
>   private static final String FRAME_CATEGORIZATION_KEY =
> "distinguishFramesCategorization"; //$NON-NLS-1$
> @@ -491,7 +497,7 @@ public class StacktraceView extends ViewPart
> implements ISelectionListener {
>   new StacktraceViewToolTipSupport(viewer);
>   MCContextMenuManager mm =
> MCContextMenuManager.create(viewer.getControl());
>   CopySelectionAction copyAction = new CopySelectionAction(viewer,
> - FormatToolkit.selectionFormatter(stackTraceLabelProvider,
> countLabelProvider));
> + FormatToolkit.selectionFormatter(stackTraceLabelProvider,
> countLabelProvider, allocPressureLabelProvider));
>   InFocusHandlerActivator.install(viewer.getControl(), copyAction);
>   mm.appendToGroup(MCContextMenuManager.GROUP_EDIT, copyAction);
>   mm.appendToGroup(MCContextMenuManager.GROUP_EDIT,
> CopySettings.getInstance().createContributionItem());
> @@ -515,6 +521,8 @@ public class StacktraceView extends ViewPart
> implements ISelectionListener {
>   .setLabelProvider(stackTraceLabelProvider);
>   buildColumn(viewer, Messages.STACKTRACE_VIEW_COUNT_COLUMN_NAME,
> SWT.RIGHT, columnWidths[1])
>   .setLabelProvider(countLabelProvider);
> + buildColumn(viewer, Messages.STACKTRACE_VIEW_ALLOC_PRESSURE_COLUMN_NAME,
> SWT.RIGHT, columnWidths[2])
> + .setLabelProvider(allocPressureLabelProvider);
>
>   PlatformUI.getWorkbench().getHelpSystem().setHelp(viewer.getControl(),
> HELP_CONTEXT_ID);
>
> @@ -731,7 +739,13 @@ public class StacktraceView extends ViewPart
> implements ISelectionListener {
>   private final ColumnLabelProvider countLabelProvider = new
> ColumnLabelProvider() {
>   @Override
>   public String getText(Object element) {
> - return Integer.toString(((StacktraceFrame) element).getItemCount());
> + // display normalized count (percentage of total count)
> + StacktraceFrame frame = (StacktraceFrame) element;
> + Fork rootFork = getRootFork(frame.getBranch().getParentFork());
> + int itemCount = frame.getItemCount();
> + int totalCount = rootFork.getItemsInFork();
> + return UnitLookup.PERCENT_UNITY.quantity(itemCount / (double) totalCount)
> + .displayUsing(IDisplayable.AUTO);
>   }
>
>   @Override
> @@ -756,6 +770,31 @@ public class StacktraceView extends ViewPart
> implements ISelectionListener {
>   }
>   };
>
> + private final ColumnLabelProvider allocPressureLabelProvider = new
> ColumnLabelProvider() {
> + @Override
> + public String getText(Object element) {
> + StacktraceFrame frame = (StacktraceFrame) element;
> +
> + SimpleArray<IItem> items = frame.getItems();
> + Branch branch = frame.getBranch();
> + SimpleArray<IItem> allItems = new
> SimpleArray<IItem>(branch.getFirstFrame().getItems().elements());
> + for (StacktraceFrame f: branch.getTailFrames()) {
> + allItems.addAll(f.getItems().elements());
> + }
> +
> + IItemCollection itemsCollection =
> ItemCollectionToolkit.build(IteratorToolkit.toList(items.iterator(),
> items.size()).stream());
> + IItemCollection allItemsCollection =
> ItemCollectionToolkit.build(IteratorToolkit.toList(allItems.iterator(),
> items.size()).stream());
> +
> + try {
> + Double itemsAlloc =
> itemsCollection.getAggregate(JdkAggregators.ALLOCATION_TOTAL).numberValue().doubleValue();
> + Double allItemsAlloc =
> allItemsCollection.getAggregate(JdkAggregators.ALLOCATION_TOTAL).numberValue().doubleValue();
> + return String.format("%.1f %%", (itemsAlloc / allItemsAlloc ) * 100);
> + } catch (NullPointerException e) { // ALLOCATION_TOTAL is only available
> in Memory page
> + return "";
> + }
> + }
> + };
> +
>   private final ColumnLabelProvider stackTraceLabelProvider = new
> ColumnLabelProvider() {
>
>   @Override
> diff --git
> a/application/org.openjdk.jmc.flightrecorder.ui/src/main/resources/org/openjdk/jmc/flightrecorder/ui/messages/internal/messages.properties
> b/application/org.openjdk.jmc.flightrecorder.ui/src/main/resources/org/openjdk/jmc/flightrecorder/ui/messages/internal/messages.properties
> index cd954bd..96ce600 100644
> ---
> a/application/org.openjdk.jmc.flightrecorder.ui/src/main/resources/org/openjdk/jmc/flightrecorder/ui/messages/internal/messages.properties
> +++
> b/application/org.openjdk.jmc.flightrecorder.ui/src/main/resources/org/openjdk/jmc/flightrecorder/ui/messages/internal/messages.properties
> @@ -418,6 +418,7 @@ STORED_SELECTIONS_SIZE_UNPARSABLE=Number of stored
> selections to keep: {0}
>  STORE_SELECTION_ACTION=Store Selection
>  STORE_AND_ACTIVATE_SELECTION_ACTION=Store and Set As Focused Selection
>
> +STACKTRACE_VIEW_ALLOC_PRESSURE_COLUMN_NAME=Allocation Pressure
>  STACKTRACE_VIEW_COUNT_COLUMN_NAME=Count
>  STACKTRACE_VIEW_DISTINGUISH_FRAMES_BY=Distinguish Frames By
>  STACKTRACE_VIEW_FRAME_GROUP_CHOOSE=Choose Frame Group
>
>
>