Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-20052

ListView.getItems makes too many passes through jobs list

    • Jenkins 2.239

      Since pull 757 in 1.512, ListView can include items in subfolders. Unfortunately it seems that getItems calls ViewJobFilter with anomalous arguments: items may be a superset, rather than a subset, of allItems.

      Should be better tested, and simplified by inlining the includeItems method, which could also improve performance by removing the double traversal of the job hierarchy (which checks ACLs each time through).

          [JENKINS-20052] ListView.getItems makes too many passes through jobs list

          Daniel Beck added a comment -

          Didn't JENKINS-18721 resolve the performance issue sufficiently?

          Daniel Beck added a comment - Didn't JENKINS-18721 resolve the performance issue sufficiently?

          Jesse Glick added a comment -

          Looks like JENKINS-18721 made some improvements, but getItems still looks to go through all jobs twice: once in includeItems, once in the loop to translate names into items.

          Basically the issue is that instead of doing the obvious thing—looking up explicitly-specified names using the method that exists for that purpose, then scanning recursive items whose relative name match the regexp and adding those in—the view is scanning recursive items, adding them to the name list, then going back and scanning recursive items again and collecting those whose (relative) names match the list. And the job filters are reported to be called with the wrong arguments: the direct children of the folder.

          Jesse Glick added a comment - Looks like JENKINS-18721 made some improvements, but getItems still looks to go through all jobs twice: once in includeItems , once in the loop to translate names into items . Basically the issue is that instead of doing the obvious thing—looking up explicitly-specified names using the method that exists for that purpose, then scanning recursive items whose relative name match the regexp and adding those in—the view is scanning recursive items, adding them to the name list, then going back and scanning recursive items again and collecting those whose (relative) names match the list. And the job filters are reported to be called with the wrong arguments: the direct children of the folder.

          Jesse Glick added a comment -

          JENKINS-20143 should have fixed the behavioral issue with recurse (though there is no test yet), but the poor implementation remains (and the code is even more bloated with the new expand method which yet again traverses the job hierarchy).

          Jesse Glick added a comment - JENKINS-20143 should have fixed the behavioral issue with recurse (though there is no test yet), but the poor implementation remains (and the code is even more bloated with the new expand method which yet again traverses the job hierarchy).

          Jesse Glick added a comment -

          Worth investigating whether the three existing fields in ListView controlling job inclusion—jobNames, includeRegex, and statusFilter—can all be migrated to be ViewJobFilter implementations. (recurse cannot, since it determines the all parameter to filters.)

          Jesse Glick added a comment - Worth investigating whether the three existing fields in ListView controlling job inclusion— jobNames , includeRegex , and statusFilter —can all be migrated to be ViewJobFilter implementations. ( recurse cannot, since it determines the all parameter to filters.)

          Daniel Beck added a comment -

          A great side-effect of making jobNames into a ViewJobFilter would probably be that jobs are no longer automatically added to the view the user was in when she created the job. Which is really annoying if you have views with very specific, computed contents.

          Daniel Beck added a comment - A great side-effect of making jobNames into a ViewJobFilter would probably be that jobs are no longer automatically added to the view the user was in when she created the job. Which is really annoying if you have views with very specific, computed contents.

          Jesse Glick added a comment -

          jobs [would no longer be] automatically added to the view the user was in when she created the job

          Right, unless you actually had one of the new filters configured, which I guess would happen only if jobNames were originally nonempty. A good point.

          Jesse Glick added a comment - jobs [would no longer be] automatically added to the view the user was in when she created the job Right, unless you actually had one of the new filters configured, which I guess would happen only if jobNames were originally nonempty. A good point.

          Peter Wiseman added a comment -

          Noting that change ([PR-4466|https://github.com/jenkinsci/jenkins/pull/4466]) results in Views with the StatusFilter set being removed when upgrading from <2.239 to >2.239.

          Related issues: JENKINS-62661JENKINS-63786.

          Peter Wiseman added a comment - Noting that change ([PR-4466| https://github.com/jenkinsci/jenkins/pull/4466 ]) results in Views with the StatusFilter set being removed when upgrading from <2.239 to >2.239. Related issues:  JENKINS-62661 ,  JENKINS-63786 .

            raihaan Raihaan Shouhell
            jglick Jesse Glick
            Votes:
            3 Vote for this issue
            Watchers:
            8 Start watching this issue

              Created:
              Updated:
              Resolved: