Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Making RunMap work properly for non-AbstractProject-based jobs using …
…lazy loading.

JENKINS-22052 is not fixed by this for existing Job types like ExternalJob which still do not use lazy loading.
  • Loading branch information
jglick committed Mar 6, 2014
1 parent 6c4a104 commit f024f59
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 109 deletions.
95 changes: 0 additions & 95 deletions core/src/main/java/hudson/model/AbstractBuild.java
Expand Up @@ -58,8 +58,6 @@
import hudson.tasks.test.AggregatedTestResultAction;
import hudson.util.*;
import jenkins.model.Jenkins;
import jenkins.model.lazy.AbstractLazyLoadRunMap.Direction;
import jenkins.model.lazy.BuildReference;
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.Stapler;
import org.kohsuke.stapler.StaplerRequest;
Expand Down Expand Up @@ -156,23 +154,6 @@ public abstract class AbstractBuild<P extends AbstractProject<P,R>,R extends Abs
*/
protected transient List<Environment> buildEnvironments;

/**
* Pointers to form bi-directional link between adjacent {@link AbstractBuild}s.
*
* <p>
* Unlike {@link Run}, {@link AbstractBuild}s do lazy-loading, so we don't use
* {@link Run#previousBuild} and {@link Run#nextBuild}, and instead use these
* fields and point to {@link #selfReference} (or {@link #none}) of adjacent builds.
*/
private volatile transient BuildReference<R> previousBuild, nextBuild;

@SuppressWarnings({"unchecked", "rawtypes"}) private static final BuildReference NONE = new BuildReference("NONE", null);
@SuppressWarnings("unchecked") private BuildReference<R> none() {return NONE;}

/*package*/ final transient BuildReference<R> selfReference = new BuildReference<R>(getId(),_this());



protected AbstractBuild(P job) throws IOException {
super(job);
}
Expand All @@ -189,82 +170,6 @@ public final P getProject() {
return getParent();
}

@Override
void dropLinks() {
super.dropLinks();

if(nextBuild!=null) {
AbstractBuild nb = nextBuild.get();
if (nb!=null) {
nb.previousBuild = previousBuild;
}
}
if(previousBuild!=null) {
AbstractBuild pb = previousBuild.get();
if (pb!=null) pb.nextBuild = nextBuild;
}
}

@Override
public R getPreviousBuild() {
while (true) {
BuildReference<R> r = previousBuild; // capture the value once

if (r==null) {
// having two neighbors pointing to each other is important to make RunMap.removeValue work
P _parent = getParent();
if (_parent == null) {
throw new IllegalStateException("no parent for " + number + " in " + workspace);
}
R pb = _parent._getRuns().search(number-1, Direction.DESC);
if (pb!=null) {
((AbstractBuild)pb).nextBuild = selfReference; // establish bi-di link
this.previousBuild = pb.selfReference;
return pb;
} else {
this.previousBuild = none();
return null;
}
}
if (r==none())
return null;

R referent = r.get();
if (referent!=null) return referent;

// the reference points to a GC-ed object, drop the reference and do it again
this.previousBuild = null;
}
}

@Override
public R getNextBuild() {
while (true) {
BuildReference<R> r = nextBuild; // capture the value once

if (r==null) {
// having two neighbors pointing to each other is important to make RunMap.removeValue work
R nb = getParent()._getRuns().search(number+1, Direction.ASC);
if (nb!=null) {
((AbstractBuild)nb).previousBuild = selfReference; // establish bi-di link
this.nextBuild = nb.selfReference;
return nb;
} else {
this.nextBuild = none();
return null;
}
}
if (r==none())
return null;

R referent = r.get();
if (referent!=null) return referent;

// the reference points to a GC-ed object, drop the reference and do it again
this.nextBuild = null;
}
}

/**
* Returns a {@link Slave} on which this build was done.
*
Expand Down
113 changes: 107 additions & 6 deletions core/src/main/java/hudson/model/Run.java
Expand Up @@ -124,6 +124,9 @@
import jenkins.model.PeepholePermalink;
import jenkins.model.StandardArtifactManager;
import jenkins.model.RunAction2;
import jenkins.model.lazy.AbstractLazyLoadRunMap;
import jenkins.model.lazy.BuildReference;
import jenkins.model.lazy.LazyBuildMixIn;
import jenkins.util.VirtualFile;

/**
Expand Down Expand Up @@ -154,7 +157,7 @@ public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,Run

/**
* Previous build. Can be null.
* These two fields are maintained and updated by {@link RunMap}.
* TODO JENKINS-22052 this is not actually implemented any more
*
* External code should use {@link #getPreviousBuild()}
*/
Expand All @@ -169,6 +172,21 @@ public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,Run
@Restricted(NoExternalUse.class)
protected volatile transient RunT nextBuild;

/**
* Pointers to form bi-directional link between adjacent runs using {@link LazyBuildMixIn}.
*
* <p>
* Some {@link Run}s do lazy-loading, so we don't use
* {@link #previousBuild} and {@link #nextBuild}, and instead use these
* fields and point to {@link #selfReference} (or {@link #none}) of adjacent builds.
*/
private volatile transient BuildReference<RunT> previousBuildR, nextBuildR;

@SuppressWarnings({"unchecked", "rawtypes"}) private static final BuildReference NONE = new BuildReference("NONE", null);
@SuppressWarnings("unchecked") private BuildReference<RunT> none() {return NONE;}

/*package*/ final transient BuildReference<RunT> selfReference = new BuildReference<RunT>(getId(), _this());

/**
* Pointer to the next younger build in progress. This data structure is lazily updated,
* so it may point to the build that's already completed. This pointer is set to 'this'
Expand Down Expand Up @@ -772,19 +790,72 @@ public int getNumber() {
return number;
}

final boolean isLazy() {
return getParent() instanceof LazyBuildMixIn.LazyLoadingJob;
}

/**
* Called by {@link RunMap} to drop bi-directional links in preparation for
* deleting a build.
*/
/*package*/ void dropLinks() {
@SuppressWarnings({"unchecked", "rawtypes"})
/*package*/ final void dropLinks() {
if(nextBuild!=null)
nextBuild.previousBuild = previousBuild;
if(previousBuild!=null)
previousBuild.nextBuild = nextBuild;
if (isLazy()) {
if (nextBuildR != null) {
Run nb = nextBuildR.get();
if (nb != null) {
nb.previousBuildR = previousBuildR;
}
}
if (previousBuildR != null) {
Run pb = previousBuildR.get();
if (pb != null) {
pb.nextBuildR = nextBuildR;
}
}
}
}

public RunT getPreviousBuild() {
return previousBuild;
@SuppressWarnings({"unchecked", "rawtypes"})
public /* ought to be final */ RunT getPreviousBuild() {
if (!isLazy()) {
return previousBuild;
}
while (true) {
BuildReference<RunT> r = previousBuildR; // capture the value once

if (r == null) {
// having two neighbors pointing to each other is important to make RunMap.removeValue work
JobT _parent = getParent();
if (_parent == null) {
throw new IllegalStateException("no parent for " + number);
}
Run pb = ((LazyBuildMixIn.LazyLoadingJob) _parent).getLazyBuildMixIn()._getRuns().search(number - 1, AbstractLazyLoadRunMap.Direction.DESC);
if (pb != null) {
pb.nextBuildR = selfReference; // establish bi-di link
this.previousBuildR = pb.selfReference;
return (RunT) pb;
} else {
this.previousBuildR = none();
return null;
}
}
if (r == none()) {
return null;
}

RunT referent = r.get();
if (referent != null) {
return referent;
}

// the reference points to a GC-ed object, drop the reference and do it again
this.previousBuildR = null;
}
}

/**
Expand Down Expand Up @@ -905,8 +976,38 @@ public List<RunT> getPreviousBuildsOverThreshold(int numberOfBuilds, Result thre
return builds;
}

public RunT getNextBuild() {
return nextBuild;
@SuppressWarnings({"unchecked", "rawtypes"})
public /* ought to be final */ RunT getNextBuild() {
if (!isLazy()) {
return nextBuild;
}
while (true) {
BuildReference<RunT> r = nextBuildR; // capture the value once

if (r == null) {
// having two neighbors pointing to each other is important to make RunMap.removeValue work
Run nb = ((LazyBuildMixIn.LazyLoadingJob) getParent()).getLazyBuildMixIn()._getRuns().search(number + 1, AbstractLazyLoadRunMap.Direction.ASC);
if (nb != null) {
nb.previousBuildR = selfReference; // establish bi-di link
this.nextBuildR = nb.selfReference;
return (RunT) nb;
} else {
this.nextBuildR = none();
return null;
}
}
if (r == none()) {
return null;
}

RunT referent = r.get();
if (referent != null) {
return referent;
}

// the reference points to a GC-ed object, drop the reference and do it again
this.nextBuildR = null;
}
}

/**
Expand Down
13 changes: 5 additions & 8 deletions core/src/main/java/hudson/model/RunMap.java
Expand Up @@ -23,10 +23,6 @@
*/
package hudson.model;

import jenkins.model.lazy.AbstractLazyLoadRunMap;
import jenkins.model.lazy.BuildReference;
import org.apache.commons.collections.comparators.ReverseComparator;

import java.io.File;
import java.io.FilenameFilter;
import java.io.IOException;
Expand All @@ -39,10 +35,13 @@
import java.util.NoSuchElementException;
import java.util.SortedMap;
import java.util.logging.Level;
import java.util.logging.Logger;

import static java.util.logging.Level.*;
import java.util.logging.Logger;
import jenkins.model.lazy.AbstractLazyLoadRunMap;
import static jenkins.model.lazy.AbstractLazyLoadRunMap.Direction.*;
import jenkins.model.lazy.BuildReference;
import org.apache.commons.collections.comparators.ReverseComparator;

/**
* {@link Map} from build number to {@link Run}.
Expand All @@ -54,8 +53,6 @@
*
* @author Kohsuke Kawaguchi
*/
// in practice R is always bound by AbstractBuild, but making that change causes all kinds of
// signature breakage.
public final class RunMap<R extends Run<?,R>> extends AbstractLazyLoadRunMap<R> implements Iterable<R> {
/**
* Read-only view of this map.
Expand Down Expand Up @@ -185,7 +182,7 @@ public R put(R r) {
*/
@Override
protected BuildReference<R> createReference(R r) {
if (r instanceof AbstractBuild) return ((AbstractBuild)r).selfReference;
if (r.isLazy()) return r.selfReference;
else return super.createReference(r);
}

Expand Down

0 comments on commit f024f59

Please sign in to comment.