Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multiprocessing, default assumption of Pool size unhelpful #77167

Open
mj-harvey mannequin opened this issue Mar 2, 2018 · 12 comments
Open

multiprocessing, default assumption of Pool size unhelpful #77167

mj-harvey mannequin opened this issue Mar 2, 2018 · 12 comments
Labels
3.8 only security fixes expert-multiprocessing stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@mj-harvey
Copy link
Mannequin

mj-harvey mannequin commented Mar 2, 2018

BPO 32986
Nosy @pitrou, @njsmith, @eryksun, @applio, @mj-harvey
PRs
  • bpo-32986: multiprocessing: change default pool size to look for NCPUS envvar be… #5959
  • 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 = None
    created_at = <Date 2018-03-02.17:51:29.312>
    labels = ['3.8', 'type-feature', 'library']
    title = 'multiprocessing, default assumption of Pool size unhelpful'
    updated_at = <Date 2019-02-26.07:54:27.392>
    user = 'https://github.com/mj-harvey'

    bugs.python.org fields:

    activity = <Date 2019-02-26.07:54:27.392>
    actor = 'eryksun'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2018-03-02.17:51:29.312>
    creator = 'M J Harvey'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32986
    keywords = ['patch']
    message_count = 12.0
    messages = ['313150', '313381', '313382', '313385', '313386', '313387', '313392', '313401', '313402', '313403', '313406', '313408']
    nosy_count = 7.0
    nosy_names = ['pitrou', 'njs', 'eryksun', 'davin', 'Matthew Rocklin', 'Matt Harvey', 'M J Harvey']
    pr_nums = ['5959']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue32986'
    versions = ['Python 3.8']

    @mj-harvey
    Copy link
    Mannequin Author

    mj-harvey mannequin commented Mar 2, 2018

    Hi,

    multiprocessing's default assumption about Pool size is os.cpu_count() ie all the cores visible to the OS.

    This is tremendously unhelpful when running multiprocessing code inside an HPC batch system (PBS Pro in my case), as there's no way to hint to the code that the # of cpus actually allocated to it may be fewer.

    It's quite tedious to have to explain this to every single person trying to use it.

    Proposal: multiprocessing should look for a hint for default Pool size from the environment variable "NCPUS" which most batch systems set. If that's not set, or its value is invalid, fall back to os.cpu_count() as before

    @mj-harvey mj-harvey mannequin added the stdlib Python modules in the Lib dir label Mar 2, 2018
    @pitrou
    Copy link
    Member

    pitrou commented Mar 7, 2018

    Matt, what do you think about this proposal? Is NCPUS the right thing to look at?

    @pitrou pitrou added 3.7 only security fixes 3.8 only security fixes type-bug An unexpected behavior, bug, or error labels Mar 7, 2018
    @njsmith
    Copy link
    Contributor

    njsmith commented Mar 7, 2018

    This is a duplicate of bpo-26692 and bpo-23530, and possibly others.

    My impression is that len(os.sched_getaffinity(os.getpid())) is the Right Guess. Currently sched_getaffinity isn't implemented on Windows, but bpo-23530 has some example code for how it could/should be implemented.

    @m J Harvey: does this return the right thing for your batch jobs?

    @pitrou pitrou added type-feature A feature request or enhancement and removed 3.7 only security fixes type-bug An unexpected behavior, bug, or error labels Mar 7, 2018
    @MatthewRocklin
    Copy link
    Mannequin

    MatthewRocklin mannequin commented Mar 7, 2018

    I agree that this is a common issue. We see it both when people use batch schedulers as well as when people use Docker containers. I don't have enough experience with batch schedulers to verify that NCPUS is commonly set. I would encourage people to also look at what Docker uses.

    After a quick (two minute) web search I couldn't find the answer, but I suspect that one exists. I've raised a question on Stack Overflow here: https://stackoverflow.com/questions/49151296/how-many-cpus-does-my-docker-container-have

    @pitrou
    Copy link
    Member

    pitrou commented Mar 7, 2018

    I don't think we want to hardcode special cases for each resource-limiting framework out there: some people care about Docker, others about cgroups, CPU affinity settings, etc. NCPUS has the nice property that each of those frameworks can set it, and users or sysadmins can also override it easily. It's also trivially queried from Python.

    @MattHarvey
    Copy link
    Mannequin

    MattHarvey mannequin commented Mar 7, 2018

    Hi,

    No, using the affinity's not useful to us as, in the general case, the batch system (PBS Pro in our case) isn't using cgroups or cpusets (it does control ave cpu use by monitoring rusage of the process group).

    Several other batch system I've worked with either set NCPUS directly or have a method for site-specific customisation of the job's environment.

    That doesn't preclude using the affinity as an alternative to os.cpu_count()

    As @pitrou correctly observes, probably better to have a simple, well-sign-posted way for the sysadmins to influence the pool default than try to overload multiprocessing with complex heuristics.

    @pitrou
    Copy link
    Member

    pitrou commented Mar 7, 2018

    Note that to avoid any kind of environment variable-driven Denial of Service, we should probably ignore NCPUS when sys.flags.ignore_environment is set.

    @njsmith
    Copy link
    Contributor

    njsmith commented Mar 7, 2018

    That stackoverflow thread points to the GNU coreutils 'nproc', which is an interesting compendium of knowledge about this problem.

    It looks like their algorithm is roughly:

    1. Determine how many CPUs *could* this program access, by going down a list of possible options and using the first that works: pthread_getaffinity_np -> sched_getaffinity -> GetProcessAffinityMask -> sysconf(_SC_NUMPROCESSORS_ONLN) -> some arcane stuff specific to HP-UX, IRIX, etc.

    2. Parse the OMP_NUM_THREADS and OMP_THREAD_LIMIT envvars (this is not quite trivial, there's some handling of whitespace and commas and references to the OMP spec)

    3. If OMP_NUM_THREADS is set, return min(OMP_NUM_THREADS, OMP_THREAD_LIMIT). Otherwise, return min(available_processors, OMP_THREAD_LIMIT).

    Step (1) handles both the old affinity APIs, and also the cpuset system that docker uses. (From cpuset(7): "Cpusets are integrated with the sched_setaffinity(2) scheduling affinity mechanism".) Step (2) relies on the quasi-standard OMP_* envvars to let you choose something explicitly.

    The PBS Pro docs say that they set both NPROCS and OMP_NUM_THREADS. See section 6.1.7 of https://pbsworks.com/pdfs/PBSUserGuide14.2.pdf

    So this seems like a reasonable heuristic approach to me.

    @pitrou
    Copy link
    Member

    pitrou commented Mar 7, 2018

    So this seems like a reasonable heuristic approach to me.

    You mean duplicating "nproc"'s logic in Python? If someone wants to do the grunt work of implementing/testing it...

    There's also the question of how that affects non-scientific workloads. People can use thread pools or process pools for other purposes, such as distributing (blocking) I/O.

    @njsmith
    Copy link
    Contributor

    njsmith commented Mar 7, 2018

    I can't find any evidence that NPROCS is used by other batch schedulers (I looked at SLURM, Torque, and SGE). @m J Harvey, do you have any other examples of systems that use it?

    @njsmith
    Copy link
    Contributor

    njsmith commented Mar 7, 2018

    You mean duplicating "nproc"'s logic in Python?

    Yeah.

    If someone wants to do the grunt work of implementing/testing it...

    Well, that's true of any bug fix / improvement :-). The logic isn't terribly complicated though, something roughly like:

    def parse_omp_envvar(env_value):
        return int(env_value.strip().split(",")[0])
    
    def estimate_cpus():
        limit = float("inf")
        if "OMP_THREAD_LIMIT" in os.environ:
            limit = parse_omp_envvar(os.environ["OMP_THREAD_LIMIT"])
    
        if "OMP_NUM_THREADS" in os.environ:
            cpus = parse_omp_envvar(os.environ["OMP_NUM_THREADS"])
        else:
            try:
                cpus = len(os.sched_getaffinity(os.getpid()))
            except AttributeError, OSError:
                cpus = os.cpu_count()
    
        return min(cpus, limit)

    There's also the question of how that affects non-scientific workloads. People can use thread pools or process pools for other purposes, such as distributing (blocking) I/O.

    We already have some heuristics for this: IIRC the thread pool executor defaults to cpu_count() * 5 threads (b/c Python threads are really intended for I/O-bound workloads), and the process pool executor and multiprocessing.Pool defaults to cpu_count() processes (b/c processes are better suited to CPU-bound workloads). Neither of these heuristics is perfect. But inasmuch as it makes sense at all to use the cpu count as part of the heuristic, it surely will work better to use a more accurate estimate of the available cpus.

    @MattHarvey
    Copy link
    Mannequin

    MattHarvey mannequin commented Mar 7, 2018

    @njs your sketch in msg313406 looks good. Probably better to go with OMP_NUM_THREADS than NCPUS.
    M

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes expert-multiprocessing stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    Status: No status
    Development

    No branches or pull requests

    3 participants