Profiler settings improvements

 - show Settings by default
 - check invalid roots/filters
 - display invalid settings on profiling start

Author: jisedlac

visualvm/profiler/src/com/sun/tools/visualvm/profiler/ApplicationProfilerView.java

@@ -31,6 +31,7 @@
 import com.sun.tools.visualvm.core.ui.DataSourceView;
 import com.sun.tools.visualvm.core.ui.DesktopUtils;
 import com.sun.tools.visualvm.core.ui.components.DataViewComponent;
+import com.sun.tools.visualvm.core.ui.components.DataViewComponent.DetailsView;
 import com.sun.tools.visualvm.profiling.presets.PresetSelector;
 import com.sun.tools.visualvm.profiling.presets.ProfilerPresets;
 import com.sun.tools.visualvm.uisupport.HTMLLabel;
@@ -79,6 +80,7 @@ final class ApplicationProfilerView extends DataSourceView {
     private static final String IMAGE_PATH =
             "com/sun/tools/visualvm/profiler/resources/profiler.png"; // NOI18N
 
+    private DataViewComponent dvc;
     private MasterViewSupport masterViewSupport;
     private CPUSettingsSupport cpuSettings;
     private MemorySettingsSupport memorySettings;
@@ -140,20 +142,35 @@ private PresetSelector createSelector(Runnable presetSynchronizer, Application a
     protected DataViewComponent createComponent() {
         Application application = (Application)getDataSource();
         ProfilingResultsSupport profilingResultsSupport = new ProfilingResultsSupport();
-        masterViewSupport = new MasterViewSupport(application, profilingResultsSupport, cpuSettings, memorySettings, jdbcSettings, classSharingBreaksProfiling);
 
-        DataViewComponent dvc = new DataViewComponent(
-                masterViewSupport.getMasterView(),
-                new DataViewComponent.MasterViewConfiguration(false));
+        final DetailsView cpuSettingsView = cpuSettings.getDetailsView();
+        final DetailsView memorySettingsView = memorySettings.getDetailsView();
+        final DetailsView jdbcSettingsView = jdbcSettings.getDetailsView();
 
+        masterViewSupport = new MasterViewSupport(application, profilingResultsSupport, cpuSettings, memorySettings, jdbcSettings, classSharingBreaksProfiling) {
+            void showCPUSettings() {
+                if (dvc != null) {
+                    dvc.selectDetailsView(cpuSettingsView);
+                    dvc.showDetailsArea(DataViewComponent.TOP_RIGHT);
+                }
+            }
+            void showMemorySettings() {
+                if (dvc != null) {
+                    dvc.selectDetailsView(memorySettingsView);
+                    dvc.showDetailsArea(DataViewComponent.TOP_RIGHT);
+                }
+            }
+        };
+        
+        dvc = new DataViewComponent(masterViewSupport.getMasterView(), new DataViewComponent.MasterViewConfiguration(false));
         dvc.configureDetailsArea(new DataViewComponent.DetailsAreaConfiguration(NbBundle.getMessage(ApplicationProfilerView.class, "LBL_Profiling_results"), false), DataViewComponent.TOP_LEFT);   // NOI18N
         dvc.addDetailsView(profilingResultsSupport.getDetailsView(), DataViewComponent.TOP_LEFT);
 
         dvc.configureDetailsArea(new DataViewComponent.DetailsAreaConfiguration(NbBundle.getMessage(ApplicationProfilerView.class, "LBL_Settings"), true), DataViewComponent.TOP_RIGHT);   // NOI18N
-        dvc.addDetailsView(cpuSettings.getDetailsView(), DataViewComponent.TOP_RIGHT);
-        dvc.addDetailsView(memorySettings.getDetailsView(), DataViewComponent.TOP_RIGHT);
-        dvc.addDetailsView(jdbcSettings.getDetailsView(), DataViewComponent.TOP_RIGHT);
-        dvc.hideDetailsArea(DataViewComponent.TOP_RIGHT);
+        dvc.addDetailsView(cpuSettingsView, DataViewComponent.TOP_RIGHT);
+        dvc.addDetailsView(memorySettingsView, DataViewComponent.TOP_RIGHT);
+        dvc.addDetailsView(jdbcSettingsView, DataViewComponent.TOP_RIGHT);
+//        dvc.hideDetailsArea(DataViewComponent.TOP_RIGHT);
 
         return dvc;
     }
@@ -169,7 +186,7 @@ protected void removed() {
 
     // --- General data --------------------------------------------------------
 
-    private static class MasterViewSupport extends JPanel implements ProfilingStateListener, DataRemovedListener<Application>, ActionListener, PropertyChangeListener {
+    private static abstract class MasterViewSupport extends JPanel implements ProfilingStateListener, DataRemovedListener<Application>, ActionListener, PropertyChangeListener {
 
         private Application application;
         private ProfilingResultsSupport profilingResultsView;
@@ -215,6 +232,11 @@ public MasterViewSupport(final Application application, ProfilingResultsSupport
         }
 
 
+        abstract void showCPUSettings();
+        
+        abstract void showMemorySettings();
+        
+        
         public DataViewComponent.MasterView getMasterView() {
             return new DataViewComponent.MasterView(NbBundle.getMessage(ApplicationProfilerView.class, "LBL_Profiler"), null, this);    // NOI18N
         }
@@ -271,6 +293,7 @@ private void handleCPUProfiling() {
                 internalChange = true;
                 cpuButton.setSelected(false);
                 internalChange = false;
+                showCPUSettings();
                 ProfilerDialogs.displayError(NbBundle.getMessage(ApplicationProfilerView.class, "MSG_Incorrect_CPU_settings")); // NOI18N
             } else {
                 cpuSettingsSupport.saveSettings();
@@ -297,19 +320,27 @@ private void handleMemoryProfiling() {
             cpuButton.setSelected(false);
             jdbcButton.setSelected(false);
             internalChange = false;
-            memorySettingsSupport.saveSettings();
-            if (NetBeansProfiler.getDefaultNB().getProfilingState() == NetBeansProfiler.PROFILING_RUNNING) {
-              ProfilerUtils.runInProfilerRequestProcessor(new Runnable() {
-                public void run() {
-                    NetBeansProfiler.getDefaultNB().modifyCurrentProfiling(memorySettingsSupport.getSettings()); 
-                }
-              });
+            if (!memorySettingsSupport.settingsValid()) {
+                internalChange = true;
+                memoryButton.setSelected(false);
+                internalChange = false;
+                showMemorySettings();
+                ProfilerDialogs.displayError(NbBundle.getMessage(ApplicationProfilerView.class, "MSG_Incorrect_Memory_settings")); // NOI18N
             } else {
-              disableControlButtons();
-              ProfilerSupport.getInstance().setProfiledApplication(application);
-              ProfilerUtils.runInProfilerRequestProcessor(new Runnable() {
-                public void run() { startProfiling(application, memorySettingsSupport.getSettings()); }
-              });
+              memorySettingsSupport.saveSettings();
+              if (NetBeansProfiler.getDefaultNB().getProfilingState() == NetBeansProfiler.PROFILING_RUNNING) {
+                ProfilerUtils.runInProfilerRequestProcessor(new Runnable() {
+                  public void run() {
+                    NetBeansProfiler.getDefaultNB().modifyCurrentProfiling(memorySettingsSupport.getSettings()); 
+                  }
+                });
+              } else {
+                disableControlButtons();
+                ProfilerSupport.getInstance().setProfiledApplication(application);
+                ProfilerUtils.runInProfilerRequestProcessor(new Runnable() {
+                  public void run() { startProfiling(application, memorySettingsSupport.getSettings()); }
+                });
+              }
             }
           }
         }

visualvm/profiler/src/com/sun/tools/visualvm/profiler/Bundle.properties

@@ -70,6 +70,8 @@ MSG_Class_Sharing_Nolink=<nobr>https://visualvm.github.io/troubleshooting.html#x
 
 MSG_Incorrect_CPU_settings=Provided CPU settings are invalid.
 
+MSG_Incorrect_Memory_settings=Provided Memory settings are invalid.
+
 LBL_Profile=Profile\:
 LBL_Sample=Sample\:
 

visualvm/profiling/src/com/sun/tools/visualvm/profiling/presets/PresetsUtils.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package com.sun.tools.visualvm.profiling.presets;
+
+import java.util.regex.Pattern;
+import org.netbeans.lib.profiler.filters.GenericFilter;
+
+/**
+ *
+ * @author Jiri Sedlacek
+ */
+final class PresetsUtils {
+    
+    static String normalizeValue(String value) {
+        String[] values = value.split("\\n"); // NOI18N
+        StringBuilder normalizedValue = new StringBuilder();
+
+        for (int i = 0; i < values.length; i++) {
+            String filterValue = values[i].trim();
+            if ((i != (values.length - 1)) && !filterValue.endsWith(",")) // NOI18N
+                filterValue = filterValue + ","; // NOI18N
+            normalizedValue.append(filterValue);
+        }
+
+        return normalizedValue.toString();
+    }
+    
+    
+    static boolean isValidJavaValue(String normalizedValue, boolean allowEmpty, boolean acceptArrays) {
+        // check whether empty value is allowed
+        if (normalizedValue.isEmpty()) return allowEmpty;
+        
+        String[] values = GenericFilter.values(normalizedValue);
+        for (String value : values) {
+            // remove up to two trailing wildcards
+            boolean hadWildcard = value.endsWith("*"); // NOI18N
+            if (hadWildcard) value = value.substring(0, value.length() - 1);
+            if (value.endsWith("*")) value = value.substring(0, value.length() - 1); // NOI18N
+            
+            if (hadWildcard) {
+                // wildcards can only be standalone or prefixed by dot
+                if (!value.isEmpty() && !value.endsWith(".")) return false; // NOI18N
+            } else if (acceptArrays) {
+                int len = value.length();
+
+                // remove trailing arrays if allowed and not followed by wildcards
+                while (value.endsWith("[]")) value = value.substring(0, value.length() - 2); // NOI18N
+
+                // multiple array marks only allowed when prefixed by Java identifier
+                if (len - value.length() > 2 && value.isEmpty()) return false;
+            }
+            
+            // empty line is allowed
+            if (value.isEmpty()) continue;
+            
+            // trailing dot only allowed when followed by wildcards
+            if (value.endsWith(".")) { // NOI18N
+                if (!hadWildcard) return false;
+                value = value.substring(0, value.length() - 1); // NOI18N
+            }
+            
+            // check whether the result is a valid Java identifier
+            if (!isValidJavaIdentifier(value)) return false;
+        }
+        
+        return true;
+    }
+    
+    
+    private static Pattern JAVA_IDENTIFIER_PATTERN;
+    private static boolean isValidJavaIdentifier(String identifier) {
+        if (JAVA_IDENTIFIER_PATTERN == null) {
+            String ID_PATTERN = "\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*"; // NOI18N
+            JAVA_IDENTIFIER_PATTERN = Pattern.compile(ID_PATTERN + "(\\." + ID_PATTERN + ")*"); // NOI18N
+        }
+        return JAVA_IDENTIFIER_PATTERN.matcher(identifier).matches();
+    }
+    
+}

visualvm/profiling/src/com/sun/tools/visualvm/profiling/presets/ProfilerCPUPanel.java

@@ -89,21 +89,21 @@ public ProfilingSettings getSettings() {
                                      CommonConstants.CPU_INSTR_FULL :
                                      CommonConstants.CPU_INSTR_SAMPLED);
 
-        String[] rootsValues = GenericFilter.values(getFlatValues(getRootsValues()));
+        String[] rootsValues = GenericFilter.values(PresetsUtils.normalizeValue(getRootsValue()));
         ClientUtils.SourceCodeSelection[] roots = (rootsValues.length == 1 && rootsValues[0].isEmpty()) ?
             new ClientUtils.SourceCodeSelection[0] :
             new ClientUtils.SourceCodeSelection[rootsValues.length];
         for (int i = 0; i < roots.length; i++)
             roots[i] = new ClientUtils.SourceCodeSelection(rootsValues[i], "*", null); // NOI18N
         settings.addRootMethods(roots);
 
-        String filter = filtersArea.getTextArea().getText();
+        String filter = getFilterValue();
         if (filter.isEmpty() || "*".equals(filter) || "**".equals(filter)) { // NOI18N
             settings.setInstrumentationFilter(new JavaTypeFilter());
         } else {
             int filterType = inclFilterRadioButton.isSelected() ?
                              JavaTypeFilter.TYPE_INCLUSIVE : JavaTypeFilter.TYPE_EXCLUSIVE;
-            String filterValue = getFlatValues(getFilterValues());
+            String filterValue = PresetsUtils.normalizeValue(filter);
             settings.setInstrumentationFilter(new JavaTypeFilter(filterValue, filterType));
         }
 
@@ -112,19 +112,6 @@ public ProfilingSettings getSettings() {
         return settings;
     }
 
-    private static String getFlatValues(String[] values) {
-        StringBuilder convertedValue = new StringBuilder();
-
-        for (int i = 0; i < values.length; i++) {
-            String filterValue = values[i].trim();
-            if ((i != (values.length - 1)) && !filterValue.endsWith(",")) // NOI18N
-                filterValue = filterValue + ","; // NOI18N
-            convertedValue.append(filterValue);
-        }
-
-        return convertedValue.toString();
-    }
-    
 
     public boolean settingsValid() { return rootsValid && filtersValid; }
 
@@ -138,15 +125,18 @@ public void loadFromPreset(ProfilerPreset preset) {
         exclFilterRadioButton.setSelected(preset.getFilterModeP());
         filtersArea.getTextArea().setText(preset.getFilterP());
         internalChange = false;
+        
+        checkRootValidity();
+        checkFilterValidity();
     }
 
     public void saveToPreset(ProfilerPreset preset) {
         if (preset == null) return;
 
-        preset.setRootsP(rootsArea.getTextArea().getText());
+        preset.setRootsP(getRootsValue());
 //        preset.setRunnablesP(runnablesCheckBox.isSelected());
         preset.setFilterModeP(exclFilterRadioButton.isSelected());
-        preset.setFilterP(filtersArea.getTextArea().getText());
+        preset.setFilterP(getFilterValue());
     }
 
     public abstract void settingsChanged();
@@ -165,33 +155,12 @@ private void checkRootValidity() {
     }
 
     public boolean isRootValueValid() {
-        // TODO
-        String roots = rootsArea.getTextArea().getText().trim();
-        if (roots.isEmpty() || roots.contains("<") || roots.contains(">")) return false; // NOI18N
-
-        return true;
+        String rootsValue = PresetsUtils.normalizeValue(getRootsValue());
+        return PresetsUtils.isValidJavaValue(rootsValue, false, false);
     }
 
-//    private String getRootValue() {
-//        StringBuilder convertedValue = new StringBuilder();
-//
-//        String[] rootValues = getRootsValues();
-//
-//        for (int i = 0; i < rootValues.length; i++) {
-//            String filterValue = rootValues[i].trim();
-//
-//            if ((i != (rootValues.length - 1)) && !filterValue.endsWith(",")) { // NOI18N
-//                filterValue = filterValue + ","; // NOI18N
-//            }
-//
-//            convertedValue.append(filterValue);
-//        }
-//
-//        return convertedValue.toString();
-//    }
-    
-    private String[] getRootsValues() {
-        return rootsArea.getTextArea().getText().split("\\n"); // NOI18N
+    private String getRootsValue() {
+        return rootsArea.getTextArea().getText().trim();
     }
 
     private void checkFilterValidity() {
@@ -202,35 +171,12 @@ private void checkFilterValidity() {
     }
 
     public boolean isFilterValueValid() {
-// TODO 
-//        String[] filterParts = FilterUtils.getSeparateFilters(getFilterValue());
-
-//        for (int i = 0; i < filterParts.length; i++)
-//            if (!FilterUtils.isValidProfilerFilter(filterParts[i])) return false;
-
-        return true;
+        String filterValue = PresetsUtils.normalizeValue(getFilterValue());
+        return PresetsUtils.isValidJavaValue(filterValue, true, false);
     }
-    
-//    private String getFilterValue() {
-//        StringBuilder convertedValue = new StringBuilder();
-//
-//        String[] filterValues = getFilterValues();
-//
-//        for (int i = 0; i < filterValues.length; i++) {
-//            String filterValue = filterValues[i].trim();
-//
-//            if ((i != (filterValues.length - 1)) && !filterValue.endsWith(",")) { // NOI18N
-//                filterValue = filterValue + ", "; // NOI18N
-//            }
-//
-//            convertedValue.append(filterValue);
-//        }
-//
-//        return convertedValue.toString();
-//    }
 
-    private String[] getFilterValues() {
-        return filtersArea.getTextArea().getText().split("\\n"); // NOI18N
+    private String getFilterValue() {
+        return filtersArea.getTextArea().getText().trim();
     }