[PATCH] JMC-4740: There should be a better default column ordering for event attributes

Miro Wengner miro.wengner at gmail.com
Mon Aug 13 15:25:25 UTC 2018


Hi Everyone,
 here is my patch for a review,
Thank you Marcus!

Kind Regards,
Miro


diff -r 2cc768144a86 application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/common/DataPageToolkit.java
--- a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/common/DataPageToolkit.java	Fri Aug 10 14:03:47 2018 +0530
+++ b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/common/DataPageToolkit.java	Mon Aug 13 15:37:03 2018 +0200
@@ -38,6 +38,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -145,6 +146,7 @@
 import org.openjdk.jmc.ui.charts.XYQuantities;
 import org.openjdk.jmc.ui.column.ColumnMenusFactory;
 import org.openjdk.jmc.ui.column.TableSettings;
+import org.openjdk.jmc.ui.column.TableSettings.ColumnSettings;
 import org.openjdk.jmc.ui.handlers.ActionToolkit;
 import org.openjdk.jmc.ui.handlers.MCContextMenuManager;
 import org.openjdk.jmc.ui.misc.ChartCanvas;
@@ -176,17 +178,25 @@
 		};
 	}

-	private static Map<String, Color> fieldColorMap = new HashMap<>();
+	private static final Map<String, Color> FIELD_COLOR_MAP = new HashMap<>();
+	private static final Map<String, Integer> DEFAULT_COLUMNS_ORDER;

 	static {

 		// FIXME: Create FieldAppearance class, similar to TypeAppearence?
-		fieldColorMap.put(JdkAttributes.MACHINE_TOTAL.getIdentifier(), new Color(255, 128, 0));
-		fieldColorMap.put(JdkAttributes.JVM_SYSTEM.getIdentifier(), new Color(128, 128, 128));
-		fieldColorMap.put(JdkAttributes.JVM_USER.getIdentifier(), new Color(0, 0, 255));
-		fieldColorMap.put(JdkAttributes.JVM_TOTAL.getIdentifier(), new Color(64, 64, 191));
+		FIELD_COLOR_MAP.put(JdkAttributes.MACHINE_TOTAL.getIdentifier(), new Color(255, 128, 0));
+		FIELD_COLOR_MAP.put(JdkAttributes.JVM_SYSTEM.getIdentifier(), new Color(128, 128, 128));
+		FIELD_COLOR_MAP.put(JdkAttributes.JVM_USER.getIdentifier(), new Color(0, 0, 255));
+		FIELD_COLOR_MAP.put(JdkAttributes.JVM_TOTAL.getIdentifier(), new Color(64, 64, 191));

 		// FIXME: Handle ColorProvider and combined events
+		Map<String, Integer> columnsOrderMap = new HashMap<>();
+		columnsOrderMap.put(createColumnId(JfrAttributes.START_TIME), 1);
+		columnsOrderMap.put(createColumnId(JfrAttributes.DURATION), 2);
+		columnsOrderMap.put(createColumnId(JfrAttributes.END_TIME), 3);
+		columnsOrderMap.put(createColumnId(JfrAttributes.EVENT_THREAD), 4);
+		DEFAULT_COLUMNS_ORDER = Collections.unmodifiableMap(columnsOrderMap);
+
 	}

 	public static final Color ALLOCATION_COLOR = new Color(64, 144, 230);
@@ -198,7 +208,7 @@
 	public static final String RESULT_ACTION_ID = "resultAction"; //$NON-NLS-1$

 	public static Color getFieldColor(String fieldId) {
-		return fieldColorMap.getOrDefault(fieldId, ColorToolkit.getDistinguishableColor(fieldId));
+		return FIELD_COLOR_MAP.getOrDefault(fieldId, ColorToolkit.getDistinguishableColor(fieldId));
 	}

 	public static Color getFieldColor(IAttribute<?> attribute) {
@@ -1053,4 +1063,33 @@
 				&& JfrAttributes.START_TIME.getAccessor(type) != JfrAttributes.END_TIME.getAccessor(type);
 	}

+	public static TableSettings createTableSettingsByOrderByAndColumnsWithDefaultOrdering(final String orderBy, final Collection<ColumnSettings> columns) {
+		final Stream<ColumnSettings> defaultOrderColumns = columns.stream()
+				.filter(c -> DEFAULT_COLUMNS_ORDER.containsKey(c.getId()))
+				.filter(c -> !c.isHidden())
+				.sorted((c1, c2) ->
+					Integer.compare(DEFAULT_COLUMNS_ORDER.get(c1.getId()),
+							DEFAULT_COLUMNS_ORDER.get(c2.getId()))
+				);
+		final Stream<ColumnSettings> naturalOrderColumns = columns.stream()
+				.filter(c -> !DEFAULT_COLUMNS_ORDER.containsKey(c.getId()))
+				.sorted((c1, c2) -> String.CASE_INSENSITIVE_ORDER.compare(c1.getId(), c2.getId()));
+		final List<ColumnSettings> resultColumns = Stream.concat(defaultOrderColumns, naturalOrderColumns).collect(Collectors.toList());
+		return new TableSettings(orderBy, resultColumns);
+	}
+
+	public static TableSettings createTableSettingsByAllAndVisibleColumns(final Collection<String> allColumns, final Collection<String> visibleColumns) {
+		final List<ColumnSettings> defaultListCols = new ArrayList<>();
+		for (String columnId : allColumns) {
+			defaultListCols.add(new ColumnSettings(columnId, !visibleColumns.contains(columnId), null, null));
+		}
+		return createTableSettingsByOrderByAndColumnsWithDefaultOrdering(null, defaultListCols);
+	}
+
+	private static String createColumnId(IAttribute<?> attr) {
+		return new StringBuilder().append(attr.getIdentifier())
+				.append(":")
+				.append(attr.getContentType().getIdentifier())
+				.toString();
+	}
 }




diff -r 2cc768144a86 application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/EventBrowserPage.java
--- a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/EventBrowserPage.java	Fri Aug 10 14:03:47 2018 +0530
+++ b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/EventBrowserPage.java	Mon Aug 13 15:37:38 2018 +0200
@@ -335,7 +335,8 @@
 			Composite parent = oldListControl.getParent();
 			oldListControl.dispose();
 			list = DataPageToolkit.createSimpleItemList(parent, itemListBuilder, container,
-					new TableSettings(orderBy, listColumns), Messages.EventBrowserPage_EVENT_BROWSER_SELECTION);
+					DataPageToolkit.createTableSettingsByOrderByAndColumnsWithDefaultOrdering(orderBy, listColumns),
+					Messages.EventBrowserPage_EVENT_BROWSER_SELECTION);
 			parent.layout();
 			list.show(filteredItems);
 		}



diff -r 2cc768144a86 application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/itemhandler/ItemListAndChart.java
--- a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/itemhandler/ItemListAndChart.java	Fri Aug 10 14:03:47 2018 +0530
+++ b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/pages/itemhandler/ItemListAndChart.java	Mon Aug 13 15:38:01 2018 +0200
@@ -67,6 +67,7 @@
 import org.openjdk.jmc.flightrecorder.ui.IPageContainer;
 import org.openjdk.jmc.flightrecorder.ui.ItemCollectionToolkit;
 import org.openjdk.jmc.flightrecorder.ui.StreamModel;
+import org.openjdk.jmc.flightrecorder.ui.common.DataPageToolkit;
 import org.openjdk.jmc.flightrecorder.ui.common.FilterComponent;
 import org.openjdk.jmc.flightrecorder.ui.common.ImageConstants;
 import org.openjdk.jmc.flightrecorder.ui.common.ItemHistogram.HistogramSelection;
@@ -153,10 +154,11 @@

 		// FIXME: Should we use the state here, if the columns have been updated?
 		// FIXME: Should we change the column state if the user explicitly has configured the columns?
-		TableSettings itemListSettings = TableSettings.forStateAndColumns(
-				state != null ? state.getChild(LIST_SETTINGS) : null, acc.getAllAttributes().keySet(),
-				acc.getCommonAttributes().keySet());
-
+		final TableSettings itemListSettings = state == null ? DataPageToolkit.createTableSettingsByAllAndVisibleColumns(
+				acc.getAllAttributes().keySet(), acc.getCommonAttributes().keySet()) :
+					TableSettings.forStateAndColumns(state.getChild(LIST_SETTINGS), acc.getAllAttributes().keySet(),
+							acc.getCommonAttributes().keySet());
+
 		Composite listComposite = toolkit.createComposite(tabFolder);
 		listComposite.setLayout(GridLayoutFactory.swtDefaults().create());
 		itemList = itemListBuilder.buildWithoutBorder(listComposite, itemListSettings);


More information about the jmc-dev mailing list