-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
[JENKINS-75986] Refactor BuildReferenceMapAdapter
#10946
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
48768b1
2d8019b
e36ec32
ac7f883
b37f13c
5a44930
39e329d
5fff544
955a8ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,9 +35,7 @@ | |
| import hudson.util.CopyOnWriteMap; | ||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.util.AbstractCollection; | ||
| import java.util.AbstractMap; | ||
| import java.util.AbstractSet; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
| import java.util.Comparator; | ||
|
|
@@ -46,10 +44,7 @@ | |
| import java.util.NoSuchElementException; | ||
| import java.util.Set; | ||
| import java.util.SortedMap; | ||
| import java.util.Spliterator; | ||
| import java.util.Spliterators; | ||
| import java.util.TreeMap; | ||
| import java.util.function.IntConsumer; | ||
| import java.util.function.IntPredicate; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
|
|
@@ -101,146 +96,28 @@ | |
| 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 LazyLoadRunMapEntrySet<R> entrySet = new LazyLoadRunMapEntrySet<>(this); | ||
|
|
||
| private transient volatile Set<Integer> keySet; | ||
| private transient volatile Collection<R> values; | ||
| private final BuildReferenceMapAdapter<R> adapter = new BuildReferenceMapAdapter<>(core, | ||
| this::resolveBuildRef, this::getNumberOf, this::getBuildClass) { | ||
| @Override | ||
| protected boolean removeValue(R value) { | ||
| return AbstractLazyLoadRunMap.this.removeValue(value); | ||
| } | ||
| }; | ||
|
|
||
| @Override | ||
| public Set<Integer> keySet() { | ||
| Set<Integer> ks = keySet; | ||
| if (ks == null) { | ||
| ks = new AbstractSet<>() { | ||
| @Override | ||
| public Iterator<Integer> iterator() { | ||
| return new Iterator() { | ||
| private final Iterator<Entry<Integer, R>> it = entrySet().iterator(); | ||
|
|
||
| @Override | ||
| public boolean hasNext() { | ||
| return it.hasNext(); | ||
| } | ||
|
|
||
| @Override | ||
| public Integer next() { | ||
| return it.next().getKey(); | ||
| } | ||
|
|
||
| @Override | ||
| public void remove() { | ||
| it.remove(); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| @Override | ||
| public Spliterator<Integer> spliterator() { | ||
| return new Spliterators.AbstractIntSpliterator( | ||
| Long.MAX_VALUE, | ||
| Spliterator.DISTINCT | Spliterator.ORDERED | Spliterator.SORTED) { | ||
| private final Iterator<Integer> it = iterator(); | ||
|
|
||
| @Override | ||
| public boolean tryAdvance(IntConsumer action) { | ||
| if (action == null) { | ||
| throw new NullPointerException(); | ||
| } | ||
| if (it.hasNext()) { | ||
| action.accept(it.next()); | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| @Override | ||
| public Comparator<Integer> getComparator() { | ||
| return Collections.reverseOrder(); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| @Override | ||
| public int size() { | ||
| return AbstractLazyLoadRunMap.this.size(); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isEmpty() { | ||
| return AbstractLazyLoadRunMap.this.isEmpty(); | ||
| } | ||
|
|
||
| @Override | ||
| public void clear() { | ||
| AbstractLazyLoadRunMap.this.clear(); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean contains(Object k) { | ||
| return AbstractLazyLoadRunMap.this.containsKey(k); | ||
| } | ||
| }; | ||
| keySet = ks; | ||
| } | ||
| return ks; | ||
| return adapter.keySet(); | ||
| } | ||
|
|
||
| @Override | ||
| public Collection<R> values() { | ||
| Collection<R> vals = values; | ||
| if (vals == null) { | ||
| vals = new AbstractCollection<>() { | ||
| @Override | ||
| public Iterator<R> iterator() { | ||
| return new Iterator<>() { | ||
| private final Iterator<Entry<Integer, R>> it = entrySet().iterator(); | ||
|
|
||
| @Override | ||
| public boolean hasNext() { | ||
| return it.hasNext(); | ||
| } | ||
|
|
||
| @Override | ||
| public R next() { | ||
| return it.next().getValue(); | ||
| } | ||
|
|
||
| @Override | ||
| public void remove() { | ||
| it.remove(); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| @Override | ||
| public Spliterator<R> spliterator() { | ||
| return Spliterators.spliteratorUnknownSize( | ||
| iterator(), Spliterator.DISTINCT | Spliterator.ORDERED); | ||
| } | ||
|
|
||
| @Override | ||
| public int size() { | ||
| return AbstractLazyLoadRunMap.this.size(); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isEmpty() { | ||
| return AbstractLazyLoadRunMap.this.isEmpty(); | ||
| } | ||
|
|
||
| @Override | ||
| public void clear() { | ||
| AbstractLazyLoadRunMap.this.clear(); | ||
| } | ||
| return adapter.values(); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean contains(Object v) { | ||
| return AbstractLazyLoadRunMap.this.containsValue(v); | ||
| } | ||
| }; | ||
| values = vals; | ||
| } | ||
| return vals; | ||
| @Override | ||
| public Set<Entry<Integer, R>> entrySet() { | ||
| assert baseDirInitialized(); | ||
| return adapter.entrySet(); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -354,18 +231,22 @@ public final void recognizeNumber(int buildNumber) { | |
|
|
||
| @Override | ||
| public Comparator<? super Integer> comparator() { | ||
| return Collections.reverseOrder(); | ||
| return core.comparator(); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isEmpty() { | ||
| return search(Integer.MAX_VALUE, DESC) == null; | ||
| return adapter.isEmpty(); | ||
| } | ||
|
|
||
| @Override | ||
| public Set<Entry<Integer, R>> entrySet() { | ||
| assert baseDirInitialized(); | ||
| return entrySet; | ||
| public boolean containsKey(Object value) { | ||
| return adapter.containsKey(value); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean containsValue(Object value) { | ||
| return adapter.containsValue(value); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -379,7 +260,7 @@ public SortedMap<Integer, R> getLoadedBuilds() { | |
| res.put(entry.getKey(), buildRef); | ||
| } | ||
| } | ||
| return Collections.unmodifiableSortedMap(new BuildReferenceMapAdapter<>(this, res)); | ||
| return new BuildReferenceMapAdapter<>(res, this::resolveBuildRef, this::getNumberOf, this::getBuildClass); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -390,33 +271,17 @@ public SortedMap<Integer, R> getLoadedBuilds() { | |
| */ | ||
| @Override | ||
| public SortedMap<Integer, R> subMap(Integer fromKey, Integer toKey) { | ||
| // TODO: if this method can produce a lazy map, that'd be wonderful | ||
| // because due to the lack of floor/ceil/higher/lower kind of methods | ||
| // to look up keys in SortedMap, various places of Jenkins rely on | ||
| // subMap+firstKey/lastKey combo. | ||
|
|
||
| R start = search(fromKey, DESC); | ||
| if (start == null) return EMPTY_SORTED_MAP; | ||
|
|
||
| R end = search(toKey, ASC); | ||
| if (end == null) return EMPTY_SORTED_MAP; | ||
|
|
||
| for (R i = start; i != end; ) { | ||
| i = search(getNumberOf(i) - 1, DESC); | ||
| assert i != null; | ||
| } | ||
|
Comment on lines
-404
to
-407
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unclear to me what this loop was even doing (when assertions are disabled).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code should be removed in scope of #10759. As far as I understood, this loop was loading builds which belong to subMap interval to Index.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now AbstractLazyLoadRunMap.core (which replaces Index) has all set of build references available on disk by design |
||
|
|
||
| return Collections.unmodifiableSortedMap(new BuildReferenceMapAdapter<>(this, core.subMap(fromKey, toKey))); | ||
| return adapter.subMap(fromKey, toKey); | ||
| } | ||
|
|
||
| @Override | ||
| public SortedMap<Integer, R> headMap(Integer toKey) { | ||
| return subMap(Integer.MAX_VALUE, toKey); | ||
| return adapter.headMap(toKey); | ||
| } | ||
|
|
||
| @Override | ||
| public SortedMap<Integer, R> tailMap(Integer fromKey) { | ||
| return subMap(fromKey, Integer.MIN_VALUE); | ||
| return adapter.tailMap(fromKey); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -443,11 +308,7 @@ public R oldestBuild() { | |
|
|
||
| @Override | ||
| public R get(Object key) { | ||
| if (key instanceof Integer) { | ||
| int n = (Integer) key; | ||
| return get(n); | ||
| } | ||
| return super.get(key); | ||
| return adapter.get(key); | ||
| } | ||
|
|
||
| public R get(int n) { | ||
|
|
@@ -552,7 +413,7 @@ private R resolveBuildRef(BuildReference<R> ref) { | |
| } | ||
|
|
||
| public R getByNumber(int n) { | ||
| return resolveBuildRef(core.get(n)); | ||
| return adapter.get(n); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -597,14 +458,9 @@ public synchronized void putAll(Map<? extends Integer, ? extends R> newData) { | |
| core.putAll(newWrapperData); | ||
| } | ||
|
|
||
| /** | ||
| * Return underlining {@link BuildReference} core map. | ||
| * | ||
| * @return | ||
| * full build reference map. | ||
| */ | ||
| /*package*/ SortedMap<Integer, BuildReference<R>> all() { | ||
| return core; | ||
| @Override | ||
| public R remove(Object key) { | ||
| return adapter.remove(key); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -652,7 +508,6 @@ protected BuildReference<R> createReference(R r) { | |
| return new BuildReference<>(getIdOf(r), r); | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * Parses {@code R} instance from data in the specified directory. | ||
| * | ||
|
|
@@ -664,6 +519,8 @@ protected BuildReference<R> createReference(R r) { | |
| */ | ||
| protected abstract R retrieve(File dir) throws IOException; | ||
|
|
||
| protected abstract Class<R> getBuildClass(); | ||
|
|
||
| public synchronized boolean removeValue(R run) { | ||
| return core.remove(getNumberOf(run)) != null; | ||
| } | ||
|
|
@@ -694,7 +551,5 @@ public enum Direction { | |
| ASC, DESC, EXACT | ||
| } | ||
|
|
||
| private static final SortedMap EMPTY_SORTED_MAP = Collections.unmodifiableSortedMap(new TreeMap()); | ||
|
|
||
| static final Logger LOGGER = Logger.getLogger(AbstractLazyLoadRunMap.class.getName()); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left over from the
LazyBuildMixInchange in #10759 I guess.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right