Add NavigableMap support for CopyOnWriteMap.Tree

Optimize AbstractLazyLoadRunMap's search, newestBuild, oldestBuild, firstKey, lastKey methods
using NavigableMap features

Simplify wrapping with BuildReferenceMapAdapter by introducing single configuration class
BuildReferenceMapAdapter.Resolver

Make CopyOnWriteMap.Tree.getView method protected (was public)

Author: Dmytro Ukhlov

core/src/main/java/hudson/util/CopyOnWriteMap.java

@@ -38,6 +38,7 @@
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.NavigableMap;
+import java.util.NavigableSet;
 import java.util.Set;
 import java.util.SortedMap;
 import java.util.TreeMap;
@@ -82,7 +83,7 @@ protected void update(Map<K, V> m) {
     /**
      * Atomically replaces the entire map by the copy of the specified map.
      */
-    public void replaceBy(Map<? extends K, ? extends V> data) {
+    public synchronized void replaceBy(Map<? extends K, ? extends V> data) {
         Map<K, V> d = copy();
         d.clear();
         d.putAll(data);
@@ -224,7 +225,7 @@ public void marshal(Object source, HierarchicalStreamWriter writer, MarshallingC
     /**
      * {@link CopyOnWriteMap} backed by {@link TreeMap}.
      */
-    public static final class Tree<K, V> extends CopyOnWriteMap<K, V> implements SortedMap<K, V> {
+    public static final class Tree<K, V> extends CopyOnWriteMap<K, V> implements NavigableMap<K, V> {
         private final Comparator<K> comparator;
 
         public Tree(Map<K, V> core, Comparator<K> comparator) {
@@ -259,14 +260,106 @@ protected NavigableMap<K, V> createView() {
         }
 
         @Override
-        public NavigableMap<K, V> getView() {
+        protected NavigableMap<K, V> getView() {
             return (NavigableMap<K, V>) super.getView();
         }
 
+        @Override
+        public synchronized Entry<K, V> pollFirstEntry() {
+            TreeMap<K, V> d = copy();
+            Entry<K, V> res = d.pollFirstEntry();
+            update(d);
+            return res;
+        }
+
+        @Override
+        public synchronized Entry<K, V> pollLastEntry() {
+            TreeMap<K, V> d = copy();
+            Entry<K, V> res = d.pollLastEntry();
+            update(d);
+            return res;
+        }
+
+        @Override
+        public Entry<K, V> lowerEntry(K key) {
+            return getView().lowerEntry(key);
+        }
+
+        @Override
+        public K lowerKey(K key) {
+            return getView().lowerKey(key);
+        }
+
+        @Override
+        public Entry<K, V> floorEntry(K key) {
+            return getView().floorEntry(key);
+        }
+
+        @Override
+        public K floorKey(K key) {
+            return getView().floorKey(key);
+        }
+
+        @Override
+        public Entry<K, V> ceilingEntry(K key) {
+            return getView().ceilingEntry(key);
+        }
+
+        @Override
+        public K ceilingKey(K key) {
+            return getView().ceilingKey(key);
+        }
+
+        @Override
+        public Entry<K, V> higherEntry(K key) {
+            return getView().higherEntry(key);
+        }
+
+        @Override
+        public K higherKey(K key) {
+            return getView().higherKey(key);
+        }
+
+        @Override
+        public Entry<K, V> firstEntry() {
+            return getView().firstEntry();
+        }
+
+        @Override
+        public Entry<K, V> lastEntry() {
+            return getView().lastEntry();
+        }
+
+        @Override
         public NavigableMap<K, V> descendingMap() {
             return getView().descendingMap();
         }
 
+        @Override
+        public NavigableSet<K> navigableKeySet() {
+            return getView().navigableKeySet();
+        }
+
+        @Override
+        public NavigableSet<K> descendingKeySet() {
+            return getView().descendingKeySet();
+        }
+
+        @Override
+        public NavigableMap<K, V> subMap(K fromKey, boolean fromInclusive, K toKey, boolean toInclusive) {
+            return getView().subMap(fromKey, fromInclusive, toKey, toInclusive);
+        }
+
+        @Override
+        public NavigableMap<K, V> headMap(K toKey, boolean inclusive) {
+            return getView().headMap(toKey, inclusive);
+        }
+
+        @Override
+        public NavigableMap<K, V> tailMap(K fromKey, boolean inclusive) {
+            return getView().tailMap(fromKey, inclusive);
+        }
+
         @Override
         public Comparator<? super K> comparator() {
             return getView().comparator();

core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java

@@ -24,9 +24,6 @@
 
 package jenkins.model.lazy;
 
-import static jenkins.model.lazy.AbstractLazyLoadRunMap.Direction.ASC;
-import static jenkins.model.lazy.AbstractLazyLoadRunMap.Direction.DESC;
-
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import hudson.model.Job;
 import hudson.model.Run;
@@ -39,8 +36,8 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.Iterator;
 import java.util.Map;
+import java.util.NavigableMap;
 import java.util.NoSuchElementException;
 import java.util.Set;
 import java.util.SortedMap;
@@ -96,8 +93,9 @@
 public abstract class AbstractLazyLoadRunMap<R> extends AbstractMap<Integer, R> implements SortedMap<Integer, R> {
     private final CopyOnWriteMap.Tree<Integer, BuildReference<R>> core = new CopyOnWriteMap.Tree<>(
             Collections.reverseOrder());
-    private final BuildReferenceMapAdapter<R> adapter = new BuildReferenceMapAdapter<>(core,
-            this::resolveBuildRef, this::getNumberOf, this::getBuildClass) {
+    private final BuildReferenceMapAdapter.Resolver<R> buildResolver = new BuildReferenceMapAdapter.Resolver<>(
+            this::resolveBuildRef, this::getNumberOf, this::getBuildClass);
+    private final BuildReferenceMapAdapter<R> adapter = new BuildReferenceMapAdapter<>(core, buildResolver) {
         @Override
         protected boolean removeValue(R value) {
             return AbstractLazyLoadRunMap.this.removeValue(value);
@@ -260,7 +258,7 @@ public SortedMap<Integer, R> getLoadedBuilds() {
                 res.put(entry.getKey(), buildRef);
             }
         }
-        return new BuildReferenceMapAdapter<>(res, this::resolveBuildRef, this::getNumberOf, this::getBuildClass);
+        return new BuildReferenceMapAdapter<>(res, buildResolver);
     }
 
     /**
@@ -286,24 +284,21 @@ public SortedMap<Integer, R> tailMap(Integer fromKey) {
 
     @Override
     public Integer firstKey() {
-        R r = newestBuild();
-        if (r == null)    throw new NoSuchElementException();
-        return getNumberOf(r);
+        return adapter.firstKey();
     }
 
     @Override
     public Integer lastKey() {
-        R r = oldestBuild();
-        if (r == null)    throw new NoSuchElementException();
-        return getNumberOf(r);
+        return adapter.lastKey();
     }
 
     public R newestBuild() {
-        return search(Integer.MAX_VALUE, DESC);
+        return adapter.values().stream().findFirst().orElse(null);
     }
 
     public R oldestBuild() {
-        return search(Integer.MIN_VALUE, ASC);
+        return new BuildReferenceMapAdapter<>(core.descendingMap(), buildResolver)
+                .values().stream().findFirst().orElse(null);
     }
 
     @Override
@@ -339,48 +334,27 @@ public boolean runExists(int number) {
      *      If DESC, finds the closest #M that satisfies M ≤ N.
      */
     public @CheckForNull R search(final int n, final Direction d) {
-        switch (d) {
-        case EXACT:
-            return getByNumber(n);
-        case ASC:
-            for (int m : core.descendingMap().keySet()) {
-                if (m < n) {
-                    continue;
-                }
-                R r = getByNumber(m);
-                if (r != null) {
-                    return r;
-                }
-            }
-            return null;
-        case DESC:
-            Iterator<Integer> iterator = core.keySet().iterator();
-            while (iterator.hasNext()) {
-                int m = iterator.next();
-                if (m > n) {
-                    continue;
-                }
-                R r = getByNumber(m);
-                if (r != null) {
-                    return r;
-                }
-            }
-            return null;
-        default:
-            throw new AssertionError();
+        if (d == Direction.EXACT) {
+            return this.adapter.get(n);
         }
+        // prepare sub map, where we need to find first resolvable entry
+        NavigableMap<Integer, BuildReference<R>> subCore = (d == Direction.ASC)
+                ? core.headMap(n, true).descendingMap()
+                : core.tailMap(n, true);
+        // wrap with BuildReferenceMapAdapter to skip unresolvable entries
+        return new BuildReferenceMapAdapter<>(subCore, buildResolver).values().stream().findFirst().orElse(null);
     }
 
     public R getById(String id) {
         return getByNumber(Integer.parseInt(id));
     }
 
     /**
-     * Ensure load referent object if needed, cache it and return
-     * Save that object is unloadable in case of failure to avoid next load attempts
+     * Ensure loading referent object if needed, cache it and return
+     * Save that object as 'unloadable' in case of failure to avoid next load attempts
      *
      * @param ref reference object to be resolved
-     * @return R referent build object, or null if can't be resolved
+     * @return R referent build object, or null if it can't be resolved
      */
     private R resolveBuildRef(BuildReference<R> ref) {
         if (ref == null || ref.isUnloadable()) {