[JENKINS-19446] Saving Jenkins configuration in onRenamed/onDeleted i…

…s often overkill. This is already handled by ListView.Listener (other kinds of views are now responsible for their own listeners). And handled better—if there are no actual changes (because you have no such views, or they do not mention this job), there is no need to save. Also moving the actual saving in those remaining cases out of the lock on the ListView itself, just in case.
jenkinsci · Feb 28, 2014 · 3aa0dd5 · 3aa0dd5
1 parent f3558e1
commit 3aa0dd5
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 14 deletions.
diff --git a/core/src/main/java/hudson/model/ListView.java b/core/src/main/java/hudson/model/ListView.java
@@ -397,18 +397,20 @@ public static List<ListViewColumn> getDefaultColumns() {
                     for (View v : vg.getViews()) {
                         if (v instanceof ListView) {
                             ListView lv = (ListView) v;
+                            boolean needsSave;
                             synchronized (lv) {
                                 Set<String> oldJobNames = new HashSet<String>(lv.jobNames);
                                 lv.jobNames.clear();
                                 for (String oldName : oldJobNames) {
                                     lv.jobNames.add(Items.computeRelativeNamesAfterRenaming(oldFullName, newFullName, oldName, vg.getItemGroup()));
                                 }
-                                if (!oldJobNames.equals(lv.jobNames)) {
-                                    try {
-                                        g.save();
-                                    } catch (IOException x) {
-                                        Logger.getLogger(ListView.class.getName()).log(Level.WARNING, null, x);
-                                    }
+                                needsSave = !oldJobNames.equals(lv.jobNames);
+                            }
+                            if (needsSave) { // do not hold ListView lock at the time
+                                try {
+                                    g.save();
+                                } catch (IOException x) {
+                                    Logger.getLogger(ListView.class.getName()).log(Level.WARNING, null, x);
                                 }
                             }
                         }
@@ -423,13 +425,15 @@ public static List<ListViewColumn> getDefaultColumns() {
                     for (View v : vg.getViews()) {
                         if (v instanceof ListView) {
                             ListView lv = (ListView) v;
+                            boolean needsSave;
                             synchronized (lv) {
-                                if (lv.jobNames.remove(item.getRelativeNameFrom(vg.getItemGroup()))) {
-                                    try {
-                                        g.save();
-                                    } catch (IOException x) {
-                                        Logger.getLogger(ListView.class.getName()).log(Level.WARNING, null, x);
-                                    }
+                                needsSave = lv.jobNames.remove(item.getRelativeNameFrom(vg.getItemGroup()));
+                            }
+                            if (needsSave) {
+                                try {
+                                    g.save();
+                                } catch (IOException x) {
+                                    Logger.getLogger(ListView.class.getName()).log(Level.WARNING, null, x);
                                 }
                             }
                         }

diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java
@@ -2440,7 +2440,6 @@ public void onRenamed(TopLevelItem job, String oldName, String newName) throws I
         // For compatibility with old views:
         for (View v : views)
             v.onJobRenamed(job, oldName, newName);
-        save();
     }
 
     /**
@@ -2453,7 +2452,6 @@ public void onDeleted(TopLevelItem item) throws IOException {
         // For compatibility with old views:
         for (View v : views)
             v.onJobRenamed(item, item.getName(), null);
-        save();
     }
 
     @Override public boolean canAdd(TopLevelItem item) {