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

hudson.util.ProcessTree$windows.killAll() is slow on windows

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Minor Minor
    • core
    • None
    • 2.372

      after every build the processtree killer will kick off.

      on my reasonably powererful windows laptop with lots of stuff open this can take 700+ms

      This impacts every unit test and has been the cause of having to disable some tests on windows - which is not great.

      The code needs to be improved.

      One way to do this is to pass the list of environment variables to check down to C++ and only return the pids for them rather then every pid - which is then parsed and put into a map (which consumes over half of the time)

      I have ~490 processes running,

          [JENKINS-67681] hudson.util.ProcessTree$windows.killAll() is slow on windows

          James Nord added a comment -

          a crude JMH benchmark backs up that the native code is not so much the problem but the parsing of the long string afterwards.

          the native call and parsing is using ~2.2ms per call , the native call only is using approx 0.1ms

          I have currently 370+ processes that need to be enumerated to see if they need to be killed for each build. so that is the approximate 700ms per build that the unit test kicks off (and it kicks off a lot of them).

          James Nord added a comment - a crude JMH benchmark backs up that the native code is not so much the problem but the parsing of the long string afterwards. the native call and parsing is using ~2.2ms per call , the native call only is using approx 0.1ms I have currently 370+ processes that need to be enumerated to see if they need to be killed for each build. so that is the approximate 700ms per build that the unit test kicks off (and it kicks off a lot of them).

          Kalle Niemitalo added a comment - - edited

          On Windows, further speedup would be possible by creating a job object for the process tree, enumerating the processes of the job with QueryInformationJobObject(, JobObjectBasicProcessIdList, …), and then checking for environment variables of only those processes. This would require more changes at the Jenkins side, though, as the agent would have to keep a handle to the job object open.

          One could even kill the whole job with TerminateJobObject, but then a pipeline developer would not be able to protect a process from being killed, just by clearing the HUDSON_COOKIE environment variable.

          Kalle Niemitalo added a comment - - edited On Windows, further speedup would be possible by creating a job object for the process tree, enumerating the processes of the job with QueryInformationJobObject(, JobObjectBasicProcessIdList, …), and then checking for environment variables of only those processes. This would require more changes at the Jenkins side, though, as the agent would have to keep a handle to the job object open. One could even kill the whole job with TerminateJobObject, but then a pipeline developer would not be able to protect a process from being killed, just by clearing the HUDSON_COOKIE environment variable.

          James Nord added a comment - - edited

          kon do you have more information on `job object`? the link leads to a 404 for me.

          found the info at https://docs.microsoft.com/en-us/windows/win32/procthread/job-objects

          this does seem like it may help, however the code now takes approx 1/10th of the time it did before so may not actually bring any benifit.

          We also would need to correctly close the JOB handle at the end of the job to prevent a handle leak, and it would not catch any processes started using `Win32_Process.Create` (I have no idea if that is common or rare)

          James Nord added a comment - - edited kon do you have more information on `job object`? the link leads to a 404 for me. found the info at https://docs.microsoft.com/en-us/windows/win32/procthread/job-objects this does seem like it may help, however the code now takes approx 1/10th of the time it did before so may not actually bring any benifit. We also would need to correctly close the JOB handle at the end of the job to prevent a handle leak, and it would not catch any processes started using `Win32_Process.Create` (I have no idea if that is common or rare)

          I searched for Win32_Process in GitHub org:jenkinsci and user:kohsuke, and did not find anything that would call Win32_Process.Create.

          • In jenkinsci/ecutest-plugin, RESTART_JENKINS_AGENT.xml would terminate a process, not create one.
          • In jenkinsci/durable-task-plugin, PowershellScriptTest.java would examine the CommandLine of an existing process. This is a test anyway so does not run in production.
          • In kohsuke/com4j, Main.java would monitor new processes being created. This is sample code.

          Kalle Niemitalo added a comment - I searched for Win32_Process in GitHub org:jenkinsci and user:kohsuke , and did not find anything that would call Win32_Process.Create. In jenkinsci/ecutest-plugin, RESTART_JENKINS_AGENT.xml would terminate a process, not create one. In jenkinsci/durable-task-plugin, PowershellScriptTest.java would examine the CommandLine of an existing process. This is a test anyway so does not run in production. In kohsuke/com4j, Main.java would monitor new processes being created. This is sample code.

          We also would need to correctly close the JOB handle at the end of the job to prevent a handle leak

          Another option might be to assign a name to the job object when creating it. Then Jenkins should be able to reopen it later by name, without needing to keep a handle open. However, named kernel objects are generally temporary, i.e. they disappear when they are no longer referenced. I am not sure whether having processes in the job counts as such a reference. I guess it does.

          Kalle Niemitalo added a comment - We also would need to correctly close the JOB handle at the end of the job to prevent a handle leak Another option might be to assign a name to the job object when creating it. Then Jenkins should be able to reopen it later by name, without needing to keep a handle open. However, named kernel objects are generally temporary, i.e. they disappear when they are no longer referenced. I am not sure whether having processes in the job counts as such a reference. I guess it does.

            teilo James Nord
            teilo James Nord
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: