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

ListView.getItems makes too many passes through jobs list

    Details

    • Similar Issues:

      Description

      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).

        Attachments

          Issue Links

            Activity

            Hide
            danielbeck Daniel Beck added a comment -

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

            Show
            danielbeck Daniel Beck added a comment - Didn't JENKINS-18721 resolve the performance issue sufficiently?
            Hide
            jglick 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.

            Show
            jglick 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.
            Hide
            jglick 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).

            Show
            jglick 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).
            Hide
            jglick 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.)

            Show
            jglick 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.)
            Hide
            danielbeck 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.

            Show
            danielbeck 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.
            Hide
            jglick 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.

            Show
            jglick 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.

              People

              • Assignee:
                jglick Jesse Glick
                Reporter:
                jglick Jesse Glick
              • Votes:
                3 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated: