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

Poor performance of RelocationAction.getIconFileName

    XMLWordPrintable

Details

    Description

      Encountered something like this in a thread dump:

      "Handling GET /job/.../job/.../job/.../view/All/ from ... : RequestHandlerThread[#...] View/index.jelly View/sidepanel.jelly AbstractFolder/tasks-bottom.jelly" ...
   java.lang.Thread.State: RUNNABLE
	at java.lang.String$CaseInsensitiveComparator.compare(Unknown Source)
	at java.lang.String.compareToIgnoreCase(Unknown Source)
	at hudson.model.Items$2.compare(Items.java:360)
	at hudson.model.Items$2.compare(Items.java:358)
	at java.util.TimSort.countRunAndMakeAscending(Unknown Source)
	at java.util.TimSort.sort(Unknown Source)
	at java.util.Arrays.sort(Unknown Source)
	at java.util.ArrayList.sort(Unknown Source)
	at java.util.Collections.sort(Unknown Source)
	at hudson.model.Items.getAllItems(Items.java:358)
	at hudson.model.Items.getAllItems(Items.java:377)
	at hudson.model.Items.getAllItems(Items.java:377)
	at hudson.model.Items.getAllItems(Items.java:377)
	at hudson.model.Items.getAllItems(Items.java:353)
	at jenkins.model.Jenkins.getAllItems(Jenkins.java:1445)
	at jenkins.model.Jenkins.getAllItems(Jenkins.java:1454)
	at com.cloudbees.hudson.plugins.folder.relocate.StandardHandler.validDestinations(StandardHandler.java:85)
	at com.cloudbees.hudson.plugins.folder.relocate.StandardHandler.applicability(StandardHandler.java:54)
	at com.cloudbees.hudson.plugins.folder.relocate.DefaultRelocationUI.isAvailable(DefaultRelocationUI.java:67)
	at com.cloudbees.hudson.plugins.folder.relocate.RelocationAction.getIconFileName(RelocationAction.java:90)

      This is silly. We are enumerating all items in the system, and sorting them, just so we can see if there is any place something could be moved into and thus whether we should display a Move link in the sidebar!

      To start with, the sort is unnecessary. Not sure if core APIs currently allow this to be optimized out. Then we should have a lazy iterator and break at the first valid destination; there is no need to collect them all.

      And perhaps we should just unconditionally display the link, at least if there is at least one top-level Folder in Jenkins. If there turns out to be no valid destination when you click the link, we can just show a message to that effect. No need to calculate all this in advance.

      Attachments

        Issue Links

          is duplicated by

          Improvement - An improvement or enhancement to an existing feature or task. JENKINS-45624 feature flag to use cloudbees-folder-plugin for dependency requirement

          • Critical - Crashes, loss of data, severe memory leak.
          • Resolved
          links to

          Web Link CloudBees Internal OSS-1769

          Web Link PR 110

          mentioned in

          Page Loading...

          Activity

            Descending order - Click to sort in ascending order
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Jesse Glick
            Path:
            pom.xml
            src/main/java/com/cloudbees/hudson/plugins/folder/computed/ComputedFolder.java
            src/main/java/com/cloudbees/hudson/plugins/folder/computed/FolderCron.java
            src/main/java/com/cloudbees/hudson/plugins/folder/computed/ThrottleComputationQueueTaskDispatcher.java
            src/main/java/com/cloudbees/hudson/plugins/folder/relocate/StandardHandler.java
            src/test/java/com/cloudbees/hudson/plugins/folder/relocate/StandardHandlerTest.java
            http://jenkins-ci.org/commit/cloudbees-folder-plugin/533c71e20349cea16c0b67f51103c20018e42ead
            Log:
            Merge pull request #110 from jglick/hasValidDestination-JENKINS-40612

            JENKINS-40612 More efficient idiom for hasValidDestination

            Compare: https://github.com/jenkinsci/cloudbees-folder-plugin/compare/84bdaaf3bbdd...533c71e20349

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: pom.xml src/main/java/com/cloudbees/hudson/plugins/folder/computed/ComputedFolder.java src/main/java/com/cloudbees/hudson/plugins/folder/computed/FolderCron.java src/main/java/com/cloudbees/hudson/plugins/folder/computed/ThrottleComputationQueueTaskDispatcher.java src/main/java/com/cloudbees/hudson/plugins/folder/relocate/StandardHandler.java src/test/java/com/cloudbees/hudson/plugins/folder/relocate/StandardHandlerTest.java http://jenkins-ci.org/commit/cloudbees-folder-plugin/533c71e20349cea16c0b67f51103c20018e42ead Log: Merge pull request #110 from jglick/hasValidDestination- JENKINS-40612 JENKINS-40612 More efficient idiom for hasValidDestination Compare: https://github.com/jenkinsci/cloudbees-folder-plugin/compare/84bdaaf3bbdd...533c71e20349
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Jesse Glick
            Path:
            pom.xml
            src/main/java/com/cloudbees/hudson/plugins/folder/relocate/StandardHandler.java
            src/test/java/com/cloudbees/hudson/plugins/folder/relocate/StandardHandlerTest.java
            http://jenkins-ci.org/commit/cloudbees-folder-plugin/075a19deda9500dd27bbb59657e6314df875804a
            Log:
            JENKINS-40612 More efficient idiom for hasValidDestination.

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: pom.xml src/main/java/com/cloudbees/hudson/plugins/folder/relocate/StandardHandler.java src/test/java/com/cloudbees/hudson/plugins/folder/relocate/StandardHandlerTest.java http://jenkins-ci.org/commit/cloudbees-folder-plugin/075a19deda9500dd27bbb59657e6314df875804a Log: JENKINS-40612 More efficient idiom for hasValidDestination.
            jglick Jesse Glick added a comment -

            the sort is unnecessary. Not sure if core APIs currently allow this to be optimized out

            Yes, as of 2.37.

            jglick Jesse Glick added a comment - the sort is unnecessary. Not sure if core APIs currently allow this to be optimized out Yes, as of 2.37.
            jglick Jesse Glick added a comment -

            Items.getAllItems also does ACL checks, which can be expensive, as seen in other thread dumps from the same instance.

            jglick Jesse Glick added a comment - Items.getAllItems also does ACL checks, which can be expensive, as seen in other thread dumps from the same instance.

            People

              jglick Jesse Glick
              jglick Jesse Glick
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: