Skip to content

subprocess.Popen() is sometimes slower in python3 under illumos #81971

Closed

Description

@AlexanderPyhalov
BPO 37790
Nosy @pitrou, @vstinner, @MojoVampire, @nickhendo
Files
  • 1.py
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2020-04-24.00:28:51.566>
    created_at = <Date 2019-08-07.20:36:00.343>
    labels = ['library']
    title = 'subprocess.Popen() is sometimes slower in python3 under illumos'
    updated_at = <Date 2020-05-19.14:39:22.058>
    user = 'https://bugs.python.org/AlexanderPyhalov'

    bugs.python.org fields:

    activity = <Date 2020-05-19.14:39:22.058>
    actor = 'movement'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-04-24.00:28:51.566>
    closer = 'vstinner'
    components = ['Library (Lib)']
    creation = <Date 2019-08-07.20:36:00.343>
    creator = 'Alexander.Pyhalov'
    dependencies = []
    files = ['48549']
    hgrepos = []
    issue_num = 37790
    keywords = []
    message_count = 15.0
    messages = ['349197', '349207', '349211', '349857', '349893', '349894', '349897', '350064', '351343', '351352', '367162', '367181', '367182', '367185', '369358']
    nosy_count = 6.0
    nosy_names = ['pitrou', 'vstinner', 'movement', 'Alexander.Pyhalov', 'josh.r', 'nickhendo']
    pr_nums = []
    priority = 'normal'
    resolution = 'out of date'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue37790'
    versions = ['Python 3.5']

    Activity

    1. AlexanderPyhalov commented on Aug 7, 2019

      AlexanderPyhalovmannequin
      MannequinAuthor

      We've moved illumos-gate wsdiff python tool from Python 2 to Python 3.
      The tool does the following - for each file from old and new proto area compares file attributes to find differences in binary otput (spawning elfdump, dump and other utilities).
      Performance has degraded in two times when commands.getstatusoutput() was replaced by subprocess.getstatusoutput().
      os.popen() now is Popen() wrapper, so it also has poor performance.
      Even naive popen() implementation using os.system() and os.mkstemp() behaved much more efficiently (about two times faster e.g. 20 minuts vs 40 minutes for single tool pass).

    2. added
      stdlibStandard Library Python modules in the Lib/ directory
      on Aug 7, 2019
    3. MojoVampire commented on Aug 8, 2019

      MojoVampiremannequin
      Mannequin

      Have you tried switching to using Popen itself (or run, which keeps it to one layer of convenience wrapping)?

      subprocess.getstatusoutput is three layers of wrapping (it calls check_output, which in turn calls run, which in turn calls Popen), and (unlike Python 2) has to decode all the output. run would avoid two layers of wrapping, can optionally receive the raw bytes instead of decoding to str, and avoids needing to wrap the whole thing in a shell (which system, older popen, and getstatusoutput all do).

      Beyond that, it looks like when 3.8 releases, Popen should get *much* faster if the call meets certain conditions, see https://docs.python.org/3.8/whatsnew/3.8.html#optimizations for details. If you can make your use cases conform to those requirements (e.g. by using shutil.which to determine the absolute paths to your utilities instead of relying on PATH lookup), the speed up should eliminate (or more than eliminate) the performance regression you're seeing (per bpo-35537, on macOS, which got the least benefit, it was still a 1.7x improvement; on other OSes, the multiplier hits 61x or 106x).

    4. AlexanderPyhalov commented on Aug 8, 2019

      AlexanderPyhalovmannequin
      MannequinAuthor

      I've tried to rewrite subporcess.getstatusoutput() usage with subprocess.Popen() and switch to shell=False, it didn't help, so I doubti it getstatusoutput() overhead, it's Popen() issue.

    5. pitrou commented on Aug 16, 2019

      @pitrou
      Member

      Can you post a simple script showcasing the performance degradation?

    6. AlexanderPyhalov commented on Aug 17, 2019

      AlexanderPyhalovmannequin
      MannequinAuthor

      I couldn't reproduce entire test, as wsdiff script is rather large, but here's the simplified version. If Popen is used more often, difference is much more significant.

      # Using workaround
      $ python3.5 ~/tmp/1.py 1
      10.780487537384033
      # Without workaround
      $ python3.5 ~/tmp/1.py
      13.347045660018921
      # Using python2.7
      $ python2.7 ~/tmp/1.py
      9.83385586739

    7. AlexanderPyhalov commented on Aug 17, 2019

      AlexanderPyhalovmannequin
      MannequinAuthor

      Even if I use
      import subprocess
      process = subprocess.Popen(cmd,
      stdout=subprocess.PIPE,
      stderr=subprocess.PIPE,
      shell=True,
      universal_newlines=True)
      return process.stdout.read()
      difference in times are the same.

    8. pitrou commented on Aug 17, 2019

      @pitrou
      Member

      I don't seen any significant difference here (Ubuntu 18.04):

      $ time python2 sp.py 
      10.3433089256

      real 0m10,362s
      user 0m6,565s
      sys 0m4,372s

      $ time python3 sp.py 
      11.746907234191895

      real 0m11,781s
      user 0m7,356s
      sys 0m5,239s

    9. vstinner commented on Aug 21, 2019

      @vstinner
      Member

      This issue lacks a lot of information:

      • What is your operating system (name and version)? On Linux, what is your Linux kernel version?
      • Which Python version did you try?
      • Which command are you running?
      • Do you use a shell?
      • Do you use bytes (default) or Unicode (universal_newlines=True or text=True)?
      • Can you provide a minimum reproducer? I don't know how to use msg349894: cmd is not defined.

      Attached 1.py uses 2 commands, appararently both use a shell:

      • "find /usr/bin -type f 2>/dev/null"
      • "objdump '%s'"

      "objdump '%s'" is unsafe and can lead to shell injection: try to avoid the usage of a shell. Use subprocess.Popen directly, or an helper which doesn't use shell=True.

    10. nickhendo commented on Sep 9, 2019

      nickhendomannequin
      Mannequin

      We have recently bumped into a similar problem. Using FreeBSD, subprocess calls were taking more than 10 times the usual time to execute after migrating to python3.6. After some digging, the default for 'close_fds' was changed to 'True'. On linux, this actually made things faster, but for unix, much slower. Passing 'close_fds=False' solved this for us.

    11. vstinner commented on Sep 9, 2019

      @vstinner
      Member

      I create bpo-38061 "FreeBSD: Optimize subprocess.Popen(close_fds=True) using closefrom()".

    12. vstinner commented on Apr 24, 2020

      @vstinner
      Member

      We've moved illumos-gate wsdiff python tool from Python 2 to Python 3.

      I guess that you're talking about https://github.com/illumos/illumos-gate/blob/master/usr/src/tools/scripts/wsdiff.py

      This project is part of illumos: "An open-source Unix operating system". It's a fork of Solaris.

      Python does not officially support Solaris not Illumos. If someone provides a patch, we can review it. But I'm not aware of anyone working actively on supporting Illumos.

      In the meanwhile, there are two workarounds:

      • Use subprocess.Popen(close_fds=False): it should be safe since PEP-446 has been implemented in Python 3.4... except for C extension modules and C libraries which don't implement it.
      • Reduce the maximum number of file descriptors.

      On Linux and FreeBSD (if fdescfs kernel module is loaded and mounted), Python 3 iterates on /proc/self/fd/. On FreeBSD, the closefrom() syscall can be used: PR 19697. But I'm not aware of any more efficient solution for Illumos.

      Since the latest comment of the original poster was last August, I close the issue.

    13. changed the title [-]subprocess.Popen() is extremely slow[/-] [+]subprocess.Popen() is extremely slow (with close_fds=True which is the default) on Illumos[/+] on Apr 24, 2020
    14. movement commented on Apr 24, 2020

      movementmannequin
      Mannequin

      closefrom() is on both Solaris and illumos too - and might even have originated there as an API - so if that's the issue, it should be trivially fixable

    15. pitrou commented on Apr 24, 2020

      @pitrou
      Member

      I think we would accept a PR since it would probably be trivial, but someone has to submit it :-)

    16. vstinner commented on Apr 24, 2020

      @vstinner
      Member

      I merged FreeBSD changes:

      New changeset 162c567 by Victor Stinner in branch 'master':
      bpo-38061: os.closerange() uses closefrom() on FreeBSD (GH-19696)
      162c567

      New changeset e6f8abd by Victor Stinner in branch 'master':
      bpo-38061: subprocess uses closefrom() on FreeBSD (GH-19697)
      e6f8abd

      Free feel to propose a more generic way to detect when closefrom() function is available ;-) Or just a patch to add "|| defined(sun)".

    17. movement commented on May 19, 2020

      movementmannequin
      Mannequin

      I checked, and the supposition this is due to lack of closefrom() doesn't seem to be correct. Running the test case and looking at 'truss' output, there is no large number of close() that one would expect if this was the issue.

      I don't see Alexander's 2-time speedup however:

      root@piano:/export/src/cpython# /export/python3/bin/python3 ./1.py
      11.679689645767212
      root@piano:/export/src/cpython# vi 1.p^C
      root@piano:/export/src/cpython# /export/python3/bin/python3 ./1.py foo
      10.402687549591064
      root@piano:/export/src/cpython# python2.7 ./1.py
      10.0434100628

      Any difference doesn't seem to be distinguishable from noise on my system.

      If I run this:

      from __future__ import print_function
      import os
      import sys
      import time
      
      def getoutput(cmd):
              # Hand-crafted variant
              if len(sys.argv) >1:
                      import shlex, tempfile
                      f, fpath = tempfile.mkstemp()
                      status = os.system("{ " + cmd + "; } >" +
                              shlex.quote(fpath) + " 2>&1")
                      with os.fdopen(f, "r") as tfile:
                              output = tfile.read()
                      os.unlink(fpath)
                      if output[-1:] == '\n':
                              output = output[:-1]
                      return output
              else:
                      import subprocess
                      p = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=None, close_fds=True)
                      return p.communicate()[0]
              
      
      t = time.time()
      for file in getoutput("find /usr/bin -type f 2>/dev/null").decode().split('\n'):
              diff = getoutput("/usr/bin/objdump '%s' 2>/dev/null" % file)

      then I get more variation than can be measured by changing close_fds.

      Running something similar (no decode()) under python 2.7 is *slower* than python3.

      So, something other than closefrom() is going on here.

    18. changed the title [-]subprocess.Popen() is extremely slow (with close_fds=True which is the default) on Illumos[/-] [+]subprocess.Popen() is sometimes slower in python3 under illumos[/+] on May 19, 2020
    19. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      stdlibStandard Library Python modules in the Lib/ directory

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions