Merely calling job.getBuilds().isEmpty() breaks lazy-loading:

      	at hudson.model.Run.onLoad(Run.java:319)
      	at hudson.model.RunMap.retrieve(RunMap.java:226)
      	at hudson.model.RunMap.retrieve(RunMap.java:59)
      	at jenkins.model.lazy.AbstractLazyLoadRunMap.load(AbstractLazyLoadRunMap.java:667)
      	at jenkins.model.lazy.AbstractLazyLoadRunMap.load(AbstractLazyLoadRunMap.java:650)
      	at jenkins.model.lazy.AbstractLazyLoadRunMap.all(AbstractLazyLoadRunMap.java:602)
      	at jenkins.model.lazy.AbstractLazyLoadRunMap.entrySet(AbstractLazyLoadRunMap.java:264)
      	at java.util.AbstractMap$2$1.<init>(AbstractMap.java:378)
      	at java.util.AbstractMap$2.iterator(AbstractMap.java:377)
      	at hudson.util.RunList$2.iterator(RunList.java:213)
      	at hudson.util.RunList.iterator(RunList.java:103)
      	at hudson.util.RunList.isEmpty(RunList.java:173)
      

      You would reasonably expect this method, and anything simply iterating builds, to be lazy, but it is eager.

          [JENKINS-18065] AbstractLazyLoadRunMap.iterator() calls .all()

          Jesse Glick added a comment -

          Seems to me that AbstractLazyLoadRunMap.all could simply create an empty BuildReference for each ID on disk, rather than calling load. Then the entrySet would have the right keys, and if and when you actually tried to access a value for an entry, BuildReferenceMapAdapter.unwrap would call getById and thus load for that one build. Right?

          Jesse Glick added a comment - Seems to me that AbstractLazyLoadRunMap.all could simply create an empty BuildReference for each ID on disk, rather than calling load . Then the entrySet would have the right keys, and if and when you actually tried to access a value for an entry, BuildReferenceMapAdapter.unwrap would call getById and thus load for that one build. Right?

          Jesse Glick added a comment -

          To answer my own question: wrong—because it is returning a map keyed by build number, which cannot be determined until records are loaded. So a fix would need to be trickier: would need to make some operations on the entry set eager (preferably ones no one is likely to call), but make .entrySet().iterator() (as used by .size() or simply anyone using a for-loop) load entries on demand in reverse numeric order.

          Jesse Glick added a comment - To answer my own question: wrong—because it is returning a map keyed by build number, which cannot be determined until records are loaded. So a fix would need to be trickier: would need to make some operations on the entry set eager (preferably ones no one is likely to call), but make .entrySet().iterator() (as used by .size() or simply anyone using a for-loop) load entries on demand in reverse numeric order.

          Code changed in jenkins
          User: Jesse Glick
          Path:
          core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
          http://jenkins-ci.org/commit/jenkins/5903870d318a337548e3f7ef75603f75a378e7ac
          Log:
          JENKINS-18065 Documenting current misbehavior in a unit test.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java http://jenkins-ci.org/commit/jenkins/5903870d318a337548e3f7ef75603f75a378e7ac Log: JENKINS-18065 Documenting current misbehavior in a unit test.

          dogfood added a comment -

          Integrated in jenkins_main_trunk #2574
          JENKINS-18065 Documenting current misbehavior in a unit test. (Revision 5903870d318a337548e3f7ef75603f75a378e7ac)

          Result = SUCCESS
          Jesse Glick : 5903870d318a337548e3f7ef75603f75a378e7ac
          Files :

          • core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java

          dogfood added a comment - Integrated in jenkins_main_trunk #2574 JENKINS-18065 Documenting current misbehavior in a unit test. (Revision 5903870d318a337548e3f7ef75603f75a378e7ac) Result = SUCCESS Jesse Glick : 5903870d318a337548e3f7ef75603f75a378e7ac Files : core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java

          Jesse Glick added a comment -

          Seems to block the attempted fix of JENKINS-20892 from actually working, at least in the case of BuildTimelineWidget.

          Jesse Glick added a comment - Seems to block the attempted fix of JENKINS-20892 from actually working, at least in the case of BuildTimelineWidget .

          The problematic part is this:

              at jenkins.model.lazy.AbstractLazyLoadRunMap.all(AbstractLazyLoadRunMap.java:617)
              at jenkins.model.lazy.AbstractLazyLoadRunMap.entrySet(AbstractLazyLoadRunMap.java:277)
              at java.util.AbstractMap$2$1.<init>(Unknown Source)
              at java.util.AbstractMap$2.iterator(Unknown Source)
              at hudson.util.RunList.iterator(RunList.java:97)
          

          In looking at the code, what's going on is that RunList calls job.getBuilds():

              public RunList(View view) {// this is a type unsafe operation
                  Set<Job> jobs = new HashSet<Job>();
                  for (TopLevelItem item : view.getItems())
                      jobs.addAll(item.getAllJobs());
          
                  List<Iterable<R>> runLists = new ArrayList<Iterable<R>>();
                  for (Job job : jobs) {
                      runLists.add(job.getBuilds());
                  }
                  this.base = combine(runLists);
              }
          

          and job.getBuilds() does this:

              public RunList<RunT> getBuilds() {
                  return RunList.fromRuns(_getRuns().values());
              }
          

          ... which results in RunMap.values() which is implemented by AbstractMap, which calls ALLRM.entrySet(), that calls all().

          ALLRM.entrySet() impl must be smarter, or job.getBuilds() need to avoid calling values().

          Kohsuke Kawaguchi added a comment - The problematic part is this: at jenkins.model.lazy.AbstractLazyLoadRunMap.all(AbstractLazyLoadRunMap.java:617) at jenkins.model.lazy.AbstractLazyLoadRunMap.entrySet(AbstractLazyLoadRunMap.java:277) at java.util.AbstractMap$2$1.<init>(Unknown Source) at java.util.AbstractMap$2.iterator(Unknown Source) at hudson.util.RunList.iterator(RunList.java:97) In looking at the code, what's going on is that RunList calls job.getBuilds() : public RunList(View view) {// this is a type unsafe operation Set<Job> jobs = new HashSet<Job>(); for (TopLevelItem item : view.getItems()) jobs.addAll(item.getAllJobs()); List<Iterable<R>> runLists = new ArrayList<Iterable<R>>(); for (Job job : jobs) { runLists.add(job.getBuilds()); } this.base = combine(runLists); } and job.getBuilds() does this: public RunList<RunT> getBuilds() { return RunList.fromRuns(_getRuns().values()); } ... which results in RunMap.values() which is implemented by AbstractMap , which calls ALLRM.entrySet() , that calls all() . ALLRM.entrySet() impl must be smarter, or job.getBuilds() need to avoid calling values() .

          Code changed in jenkins
          User: Kohsuke Kawaguchi
          Path:
          changelog.html
          core/src/main/java/hudson/util/RunList.java
          core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java
          core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java
          core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
          http://jenkins-ci.org/commit/jenkins/54c084613f83f9b7a02957eb98dbd8dc661d1c9c
          Log:
          [FIXED JENKINS-18065] made ALLRM.entrySet() smarter

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Kohsuke Kawaguchi Path: changelog.html core/src/main/java/hudson/util/RunList.java core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java http://jenkins-ci.org/commit/jenkins/54c084613f83f9b7a02957eb98dbd8dc661d1c9c Log: [FIXED JENKINS-18065] made ALLRM.entrySet() smarter

          dogfood added a comment -

          Integrated in jenkins_main_trunk #3511
          [FIXED JENKINS-18065] made ALLRM.entrySet() smarter (Revision 54c084613f83f9b7a02957eb98dbd8dc661d1c9c)

          Result = SUCCESS
          kohsuke : 54c084613f83f9b7a02957eb98dbd8dc661d1c9c
          Files :

          • core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
          • core/src/main/java/hudson/util/RunList.java
          • changelog.html
          • core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java
          • core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java

          dogfood added a comment - Integrated in jenkins_main_trunk #3511 [FIXED JENKINS-18065] made ALLRM.entrySet() smarter (Revision 54c084613f83f9b7a02957eb98dbd8dc661d1c9c) Result = SUCCESS kohsuke : 54c084613f83f9b7a02957eb98dbd8dc661d1c9c Files : core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java core/src/main/java/hudson/util/RunList.java changelog.html core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java

          Code changed in jenkins
          User: Jesse Glick
          Path:
          core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java
          http://jenkins-ci.org/commit/jenkins/da57ad5c30b1c9ca8365f869638abd29c8fb5896
          Log:
          JENKINS-18065 54c0846 amendment: presumably RunMap.entrySet().iterator().next().setValue(…) should be illegal.

          Compare: https://github.com/jenkinsci/jenkins/compare/54c084613f83...da57ad5c30b1

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java http://jenkins-ci.org/commit/jenkins/da57ad5c30b1c9ca8365f869638abd29c8fb5896 Log: JENKINS-18065 54c0846 amendment: presumably RunMap.entrySet().iterator().next().setValue(…) should be illegal. Compare: https://github.com/jenkinsci/jenkins/compare/54c084613f83...da57ad5c30b1

          Code changed in jenkins
          User: Jesse Glick
          Path:
          cli/pom.xml
          core/pom.xml
          plugins/pom.xml
          pom.xml
          test/pom.xml
          war/pom.xml
          http://jenkins-ci.org/commit/jenkins/0faad29a5594fc20f9b3670797a3b7ebb198b63b
          Log:
          Set version to: 1.554.3.JENKINS-18065-ALLRM-all

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: cli/pom.xml core/pom.xml plugins/pom.xml pom.xml test/pom.xml war/pom.xml http://jenkins-ci.org/commit/jenkins/0faad29a5594fc20f9b3670797a3b7ebb198b63b Log: Set version to: 1.554.3. JENKINS-18065 -ALLRM-all

          Code changed in jenkins
          User: Kohsuke Kawaguchi
          Path:
          core/src/main/java/hudson/util/RunList.java
          core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java
          core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java
          core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
          http://jenkins-ci.org/commit/jenkins/b227250a63beab888a25c2a781b6fe8e493284d3
          Log:
          [FIXED JENKINS-18065] made ALLRM.entrySet() smarter

          (cherry picked from commit 54c084613f83f9b7a02957eb98dbd8dc661d1c9c)

          Conflicts:
          changelog.html

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Kohsuke Kawaguchi Path: core/src/main/java/hudson/util/RunList.java core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java http://jenkins-ci.org/commit/jenkins/b227250a63beab888a25c2a781b6fe8e493284d3 Log: [FIXED JENKINS-18065] made ALLRM.entrySet() smarter (cherry picked from commit 54c084613f83f9b7a02957eb98dbd8dc661d1c9c) Conflicts: changelog.html

          Code changed in jenkins
          User: Jesse Glick
          Path:
          core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java
          http://jenkins-ci.org/commit/jenkins/525793d741c5f209b27bdc94844798b6410fce7b
          Log:
          JENKINS-18065 54c0846 amendment: presumably RunMap.entrySet().iterator().next().setValue(…) should be illegal.
          (cherry picked from commit da57ad5c30b1c9ca8365f869638abd29c8fb5896)

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java http://jenkins-ci.org/commit/jenkins/525793d741c5f209b27bdc94844798b6410fce7b Log: JENKINS-18065 54c0846 amendment: presumably RunMap.entrySet().iterator().next().setValue(…) should be illegal. (cherry picked from commit da57ad5c30b1c9ca8365f869638abd29c8fb5896)

          Code changed in jenkins
          User: Jesse Glick
          Path:
          core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
          core/src/test/java/jenkins/model/lazy/FakeMap.java
          http://jenkins-ci.org/commit/jenkins/c3e73296b5af0a4aaa06a3bdf867ac50fe211888
          Log:
          JENKINS-18065 Uncommenting original test.
          The actual implementation does not behave as nicely as one would hope, for a few reasons:
          1. isEmpty actually tries to load the newestBuild, rather than simply checking whether there is some idOnDisk.
          Arguably this is necessary, since there could be an unloadable build record, in which case it would be technically correct for the map to be considered empty.
          2. Loading AbstractLazyLoadRunMap.newestBuild calls search(MAX_VALUE, DESC), which behaves oddly, by doing a binary search, and thus perhaps loading lg(|map|) entries.
          You would think that it would suffice to check for the last member of idOnDisk in index.byId.
          3. The iterator eagerly loads the next value before hasNext has even been called.
          Looks bad in a test, though it probably has little practical impact since most callers would be calling hasNext soon afterward anyway.
          Might cause one extra build record to be loaded unnecessarily from a limited RunList.

          (cherry picked from commit 2f58ceb1a66c8ed621fa2562c5ed3445b29ea200)

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java core/src/test/java/jenkins/model/lazy/FakeMap.java http://jenkins-ci.org/commit/jenkins/c3e73296b5af0a4aaa06a3bdf867ac50fe211888 Log: JENKINS-18065 Uncommenting original test. The actual implementation does not behave as nicely as one would hope, for a few reasons: 1. isEmpty actually tries to load the newestBuild, rather than simply checking whether there is some idOnDisk. Arguably this is necessary, since there could be an unloadable build record, in which case it would be technically correct for the map to be considered empty. 2. Loading AbstractLazyLoadRunMap.newestBuild calls search(MAX_VALUE, DESC), which behaves oddly, by doing a binary search, and thus perhaps loading lg(|map|) entries. You would think that it would suffice to check for the last member of idOnDisk in index.byId. 3. The iterator eagerly loads the next value before hasNext has even been called. Looks bad in a test, though it probably has little practical impact since most callers would be calling hasNext soon afterward anyway. Might cause one extra build record to be loaded unnecessarily from a limited RunList. (cherry picked from commit 2f58ceb1a66c8ed621fa2562c5ed3445b29ea200)

          dogfood added a comment -

          Integrated in jenkins_main_trunk #3512
          JENKINS-18065 54c0846 amendment: presumably RunMap.entrySet().iterator().next().setValue(…) should be illegal. (Revision da57ad5c30b1c9ca8365f869638abd29c8fb5896)

          Result = SUCCESS
          Jesse Glick : da57ad5c30b1c9ca8365f869638abd29c8fb5896
          Files :

          • core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java

          dogfood added a comment - Integrated in jenkins_main_trunk #3512 JENKINS-18065 54c0846 amendment: presumably RunMap.entrySet().iterator().next().setValue(…) should be illegal. (Revision da57ad5c30b1c9ca8365f869638abd29c8fb5896) Result = SUCCESS Jesse Glick : da57ad5c30b1c9ca8365f869638abd29c8fb5896 Files : core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java

          Code changed in jenkins
          User: Jesse Glick
          Path:
          core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
          core/src/test/java/jenkins/model/lazy/FakeMap.java
          http://jenkins-ci.org/commit/jenkins/2f58ceb1a66c8ed621fa2562c5ed3445b29ea200
          Log:
          JENKINS-18065 Uncommenting original test.
          The actual implementation does not behave as nicely as one would hope, for a few reasons:
          1. isEmpty actually tries to load the newestBuild, rather than simply checking whether there is some idOnDisk.
          Arguably this is necessary, since there could be an unloadable build record, in which case it would be technically correct for the map to be considered empty.
          2. Loading AbstractLazyLoadRunMap.newestBuild calls search(MAX_VALUE, DESC), which behaves oddly, by doing a binary search, and thus perhaps loading lg(|map|) entries.
          You would think that it would suffice to check for the last member of idOnDisk in index.byId.
          3. The iterator eagerly loads the next value before hasNext has even been called.
          Looks bad in a test, though it probably has little practical impact since most callers would be calling hasNext soon afterward anyway.
          Might cause one extra build record to be loaded unnecessarily from a limited RunList.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java core/src/test/java/jenkins/model/lazy/FakeMap.java http://jenkins-ci.org/commit/jenkins/2f58ceb1a66c8ed621fa2562c5ed3445b29ea200 Log: JENKINS-18065 Uncommenting original test. The actual implementation does not behave as nicely as one would hope, for a few reasons: 1. isEmpty actually tries to load the newestBuild, rather than simply checking whether there is some idOnDisk. Arguably this is necessary, since there could be an unloadable build record, in which case it would be technically correct for the map to be considered empty. 2. Loading AbstractLazyLoadRunMap.newestBuild calls search(MAX_VALUE, DESC), which behaves oddly, by doing a binary search, and thus perhaps loading lg(|map|) entries. You would think that it would suffice to check for the last member of idOnDisk in index.byId. 3. The iterator eagerly loads the next value before hasNext has even been called. Looks bad in a test, though it probably has little practical impact since most callers would be calling hasNext soon afterward anyway. Might cause one extra build record to be loaded unnecessarily from a limited RunList.

          Jesse Glick added a comment - Deployed https://github.com/jenkinsci/jenkins/compare/jenkins-1.554.3...JENKINS-18065-ALLRM-all to: http://repo.jenkins-ci.org/public/org/jenkins-ci/main/jenkins-war/1.554.3.JENKINS-18065-ALLRM-all/jenkins-war-1.554.3.JENKINS-18065-ALLRM-all.war

          dogfood added a comment -

          Integrated in jenkins_main_trunk #3514
          JENKINS-18065 Uncommenting original test. (Revision 2f58ceb1a66c8ed621fa2562c5ed3445b29ea200)

          Result = SUCCESS
          Jesse Glick : 2f58ceb1a66c8ed621fa2562c5ed3445b29ea200
          Files :

          • core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
          • core/src/test/java/jenkins/model/lazy/FakeMap.java

          dogfood added a comment - Integrated in jenkins_main_trunk #3514 JENKINS-18065 Uncommenting original test. (Revision 2f58ceb1a66c8ed621fa2562c5ed3445b29ea200) Result = SUCCESS Jesse Glick : 2f58ceb1a66c8ed621fa2562c5ed3445b29ea200 Files : core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java core/src/test/java/jenkins/model/lazy/FakeMap.java

          Code changed in jenkins
          User: Daniel Beck
          Path:
          changelog.html
          http://jenkins-ci.org/commit/jenkins/fdc0b5c5650b3ee849f02e3c5d94d23c12886adc
          Log:
          Noting #1314, #1316, #1308, JENKINS-17667, JENKINS-22395, JENKINS-18065

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Daniel Beck Path: changelog.html http://jenkins-ci.org/commit/jenkins/fdc0b5c5650b3ee849f02e3c5d94d23c12886adc Log: Noting #1314, #1316, #1308, JENKINS-17667 , JENKINS-22395 , JENKINS-18065

          Code changed in jenkins
          User: Daniel Beck
          Path:
          changelog.html
          http://jenkins-ci.org/commit/jenkins/72f37e414533c02624f9deffc85a2b50f03905e6
          Log:
          Remove JENKINS-22395, JENKINS-18065 from 1.572 section

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Daniel Beck Path: changelog.html http://jenkins-ci.org/commit/jenkins/72f37e414533c02624f9deffc85a2b50f03905e6 Log: Remove JENKINS-22395 , JENKINS-18065 from 1.572 section

          Code changed in jenkins
          User: Kohsuke Kawaguchi
          Path:
          core/src/main/java/hudson/util/RunList.java
          core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java
          core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java
          core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
          http://jenkins-ci.org/commit/jenkins/632d7fe0d01d492e0803495691f4397980cf44e0
          Log:
          [FIXED JENKINS-18065] made ALLRM.entrySet() smarter

          (cherry picked from commit 54c084613f83f9b7a02957eb98dbd8dc661d1c9c)

          Conflicts:
          changelog.html

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Kohsuke Kawaguchi Path: core/src/main/java/hudson/util/RunList.java core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java http://jenkins-ci.org/commit/jenkins/632d7fe0d01d492e0803495691f4397980cf44e0 Log: [FIXED JENKINS-18065] made ALLRM.entrySet() smarter (cherry picked from commit 54c084613f83f9b7a02957eb98dbd8dc661d1c9c) Conflicts: changelog.html

          Code changed in jenkins
          User: Jesse Glick
          Path:
          core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java
          http://jenkins-ci.org/commit/jenkins/f1b0779c6d356db8fb0de6c89bd5bc03483d6b34
          Log:
          JENKINS-18065 54c0846 amendment: presumably RunMap.entrySet().iterator().next().setValue(…) should be illegal.
          (cherry picked from commit da57ad5c30b1c9ca8365f869638abd29c8fb5896)

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java http://jenkins-ci.org/commit/jenkins/f1b0779c6d356db8fb0de6c89bd5bc03483d6b34 Log: JENKINS-18065 54c0846 amendment: presumably RunMap.entrySet().iterator().next().setValue(…) should be illegal. (cherry picked from commit da57ad5c30b1c9ca8365f869638abd29c8fb5896)

          Code changed in jenkins
          User: Jesse Glick
          Path:
          core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
          core/src/test/java/jenkins/model/lazy/FakeMap.java
          http://jenkins-ci.org/commit/jenkins/8c70a2b76875d2ca6a114c45fd54c0b1e9ecb7d2
          Log:
          JENKINS-18065 Uncommenting original test.
          The actual implementation does not behave as nicely as one would hope, for a few reasons:
          1. isEmpty actually tries to load the newestBuild, rather than simply checking whether there is some idOnDisk.
          Arguably this is necessary, since there could be an unloadable build record, in which case it would be technically correct for the map to be considered empty.
          2. Loading AbstractLazyLoadRunMap.newestBuild calls search(MAX_VALUE, DESC), which behaves oddly, by doing a binary search, and thus perhaps loading lg(|map|) entries.
          You would think that it would suffice to check for the last member of idOnDisk in index.byId.
          3. The iterator eagerly loads the next value before hasNext has even been called.
          Looks bad in a test, though it probably has little practical impact since most callers would be calling hasNext soon afterward anyway.
          Might cause one extra build record to be loaded unnecessarily from a limited RunList.

          (cherry picked from commit 2f58ceb1a66c8ed621fa2562c5ed3445b29ea200)

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java core/src/test/java/jenkins/model/lazy/FakeMap.java http://jenkins-ci.org/commit/jenkins/8c70a2b76875d2ca6a114c45fd54c0b1e9ecb7d2 Log: JENKINS-18065 Uncommenting original test. The actual implementation does not behave as nicely as one would hope, for a few reasons: 1. isEmpty actually tries to load the newestBuild, rather than simply checking whether there is some idOnDisk. Arguably this is necessary, since there could be an unloadable build record, in which case it would be technically correct for the map to be considered empty. 2. Loading AbstractLazyLoadRunMap.newestBuild calls search(MAX_VALUE, DESC), which behaves oddly, by doing a binary search, and thus perhaps loading lg(|map|) entries. You would think that it would suffice to check for the last member of idOnDisk in index.byId. 3. The iterator eagerly loads the next value before hasNext has even been called. Looks bad in a test, though it probably has little practical impact since most callers would be calling hasNext soon afterward anyway. Might cause one extra build record to be loaded unnecessarily from a limited RunList. (cherry picked from commit 2f58ceb1a66c8ed621fa2562c5ed3445b29ea200)

            kohsuke Kohsuke Kawaguchi
            jglick Jesse Glick
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: