Review Request for JMC-5923: Add allocation pressure column to stacktrace view
Joshua Matsuoka
jmatsuok at redhat.com
Fri Aug 2 19:25:50 UTC 2019
Hi Salman,
The changes look good to me, a few notes though
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;
s/TO_MUCH_SELECTED/TOO_MUCH_SELECTED, although that typo was there prior to
these changes, it still would be good to fix
It would also be good to have a test for these changes to make sure that
they get calculated correctly.
Nice Work!
Cheers,
- Josh
On Fri, Jul 26, 2019 at 10:15 AM Salman Siddiqui <sasiddiq at redhat.com>
wrote:
> 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
> >
> >
> >
>
More information about the jmc-dev
mailing list