Compileall script: add option to use multiple cores #60308

dholth · 2012-10-01T20:46:54Z

BPO	16104
Nosy	@brettcannon, @gpshead, @merwok, @dholth, @PCManticore, @JimJJewett
Files	compileall_v1.patch issue16104.patch: Remove trailing whitespaces issue16104_1.patch issue16104_2.patch: Minor doc fixes. issue16104_3.patch issue16104_4.patch issue16104_5.patch issue16104_6.patch issue16104_7.patch: Skip a couple of tests if concurrent.futures is unavailable issue16104_8.patch issue16104_9.patch issue16104_10.patch issue16104_11.patch: Minor doc update issue16104_12.patch 16104.patch: Update the patch for the tip.

^{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 = 'https://github.com/brettcannon'
closed_at = <Date 2014-09-12.14:40:30.609>
created_at = <Date 2012-10-01.20:46:54.073>
labels = ['type-feature', 'library']
title = 'Compileall script: add option to use multiple cores'
updated_at = <Date 2020-02-27.08:02:38.705>
user = 'https://github.com/dholth'

bugs.python.org fields:

activity = <Date 2020-02-27.08:02:38.705>
actor = 'gregory.p.smith'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2014-09-12.14:40:30.609>
closer = 'brett.cannon'
components = ['Library (Lib)']
creation = <Date 2012-10-01.20:46:54.073>
creator = 'dholth'
dependencies = []
files = ['33079', '34339', '34368', '34381', '34383', '34384', '34400', '34401', '34404', '35018', '35054', '35055', '35056', '35106', '36590']
hgrepos = []
issue_num = 16104
keywords = ['patch']
message_count = 28.0
messages = ['171744', '171758', '205805', '213200', '213209', '213298', '213301', '213303', '213304', '213307', '213308', '213317', '213340', '213417', '213419', '213422', '214450', '217118', '217173', '217261', '217264', '217399', '217586', '226684', '226822', '226823', '226824', '362786']
nosy_count = 7.0
nosy_names = ['brett.cannon', 'gregory.p.smith', 'eric.araujo', 'dholth', 'Claudiu.Popa', 'python-dev', 'Jim.Jewett']
pr_nums = []
priority = 'low'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue16104'
versions = ['Python 3.5']

dholth · 2012-10-01T20:46:54Z

compileall would benefit approximately linearly from additional CPU cores. There should be an option.

The noisy output would have to change. Right now it prints "compiling" and then "done" synchronously with doing the actual work.

brettcannon · 2012-10-01T23:16:10Z

This should probably use concurrent.futures instead of multiprocessing directly, but yes it would be useful.

Then again, the whole module should probably be rewritten to use importlib as well.

PCManticore · 2013-12-10T12:30:34Z

Hello!

Here's a draft patch. It adds a new *processes* parameter to *compile_dir* and a new command line parameter as well.

merwok · 2014-03-12T05:50:10Z

Patch looks good. Some comments on Rietveld.

PCManticore · 2014-03-12T07:26:02Z

Thank you for the review, Éric! Here's the updated patch.

merwok · 2014-03-12T21:05:58Z

FTR, py_compile and compileall use importlib in 3.4.

merwok · 2014-03-12T21:50:59Z

This looks ready to me.

One thing: “make -j0” is the spelling for “run using all available cores”, whereas “compileall -j0” will use one process. I don’t know if this should be documented, changed or ignored.

brettcannon · 2014-03-12T21:52:18Z

I vote for changed so that -j0 uses all available cores as os.cpu_count() states.

PCManticore · 2014-03-12T21:53:40Z

I agree. I'll modify the patch.

merwok · 2014-03-12T22:11:35Z

+ if args.processes <= 0:
Is that correct? For make, I think I’ve always seen “-j0”, not negative values.

Could you add a test for -j0? (i.e. check that “compileall -j0” calls the function with “processes=os.cpu_count()”)

PCManticore · 2014-03-12T22:12:28Z

regrtest does that, checking for j <=0.

PCManticore · 2014-03-12T22:42:19Z

Here's a test for j0 == os.cpu_count.

pitrou · 2014-03-13T00:28:13Z

Importing ProcessExecutor at the top-level means compileall will crash on systems which don't have multiprocessing support.

PCManticore · 2014-03-13T16:53:42Z

Here's a new patch which addresses Éric's last comments.
Antoine, I don't have at my disposal a system without multiprocessing support. How does it crash?

pitrou · 2014-03-13T16:59:15Z

Here's a new patch which addresses Éric's last comments.
Antoine, I don't have at my disposal a system without multiprocessing support. How does it crash?

Neither do I, but you will probably get an ImportError of some sort.

PCManticore · 2014-03-13T17:48:33Z

Here's a new version which catches ImportError for concurrent.futures and raises ValueError in compile_dir if processes was specified and concurrent.futures is unavailable. The only issue is that I don't know if this should be a ValueError or not. For instance, zipfile uses RuntimeError if lzma is unavailable.

PCManticore · 2014-03-22T08:31:37Z

What can I do to move this forward? I believe all concerns have been addressed and it seems ready to me.

PCManticore · 2014-04-24T06:20:05Z

Added a new version of the patch which incorporates suggestions made by Jim. Thanks for the review!

jimjjewett · 2014-04-25T21:43:54Z

ProcessPoolExecutor already defaults to using cpu_count if max_workers is None. Consistency with that might be useful too. (and a default of 1 to mean nothing in parallel is sensible...)

PCManticore · 2014-04-27T13:00:18Z

Added a new patch with improvements suggested by Jim. Thanks!

I removed the handling of processes=1, because it can still be useful: having a background worker which processes the files received from _walk_dir. Also, it checks that compile_dir receives a positive *processes* value, otherwise it raises a ValueError. As a side note, I just found that ProcessPoolExecutor / ThreadPoolExecutor don't verify the value of processes, leading to certain types of errors (see bpo-21362 for more details).
Jim, the default for processes is still None, meaning "do not use multiple process", because the purpose of ProcessPoolExecutor makes it easy for it to use processes=None=os.cpu_count(). Here we want the user to be explicit about wanting multiple processes or not.

PCManticore · 2014-04-27T14:06:47Z

Add new patch with fixes proposed by Berker Peksag. Thanks for the review. Hopefully, this is the last iteration of this patch.

jimjjewett · 2014-04-28T19:20:43Z

Trying to put bounds on the disagreements. Does anyone disagree with any of the following:

(1) compileall currently runs single-threaded in a single process.

(2) This enhancement intends to allow parallelization by process.

(3) Users MAY need to express whether they (require/forbid/are expressly apathetic concerning) paralellization.

(3A) There is some doubt that this even needs to be user-controlled.

(3B) If it is user-controlled, the patch proposes adding a "processes" parameter to do this.

(3C) There have been suggestions of other names (notably "workers"), but *if* it is user-controlled, the idea of a new parameter is not controversial.

(4) Users MAY need to control the degree of parallelization.

(4A) If so, setting the value of the new parameter to a positive integer > 1 is an acceptable solution.

(4B) There is not yet consensus on how to represent "Use multi-processing, with the default degree for this system.", "Do NOT use multiprocessing.", or "I don't care."

(4C) Suggested values have included 1, 0, -1, any negative number, None, and specific strings. The precise mapping between some of these and the three cases of 4B is not agreed.

(5) If multiprocessing is explicitly requested, what should happen when it is not available?

(5A) Fall back to the current way, without multi-processing.

(5B) Fall back to the current way, without multi-processing, but issue a Warning.

(5C) Raise an Exception. (ValueError, ImportError, NotImplemented?)

(6) Portions of the documentation unrelated to this should be fixed. But ideally, that would be done separately, and it will NOT be a pre-requisite to this patch.

---------------------------------------------------

Another potential value set

None (the default) ==> let the system parallelize as best it can -- as it does in multiprocessing. If the system picks "not in parallel at all", that is also OK, and no warning is raised.

0 ==> Do not parallelize.

positive integers ==> Use that many processes.

negative ==> ValueError

Would these uses of 0 and negative be too surprising for someone?

PCManticore · 2014-04-30T09:16:10Z

Updated patch according to the python-dev thread:

processes renamed to workers
workers defaults to 1
When workers is equal to 0, then os.cpu_count will be used
When workers > 1, multiple processes will be used
When workers == 1, run normally (no multiple processes)
Negative values raises a ValueError
Will raise NotImplementedError if multiprocessing can't be used
(when workers equals to 0 or > 1)

PCManticore · 2014-09-10T06:58:56Z

If there is nothing left to do for this patch, can it be committed?

python-dev · 2014-09-12T14:40:06Z

New changeset 9efefcab817e by Brett Cannon in branch 'default':
Issue bpo-16104: Allow compileall to do parallel bytecode compilation.
http://hg.python.org/cpython/rev/9efefcab817e

brettcannon · 2014-09-12T14:40:31Z

Thanks for the patch, Claudiu!

PCManticore · 2014-09-12T14:41:27Z

Thank you for committing it. :-)

gpshead · 2020-02-27T08:02:39Z

This caused a regression in behavior. compileall.compile_dir()'s ddir= parameter no longer does the right thing for any subdirectories.

https://bugs.python.org/issue39769

brettcannon added the stdlib Python modules in the Lib dir label Oct 1, 2012

brettcannon self-assigned this Mar 26, 2013

brettcannon removed their assignment Apr 28, 2013

merwok added the type-feature A feature request or enhancement label Mar 12, 2014

terryjreedy changed the title ~~Use multiprocessing in compileall script~~ Compileall script: add option to use multiple cores Apr 23, 2014

brettcannon self-assigned this Sep 10, 2014

brettcannon closed this as completed Sep 12, 2014

ezio-melotti transferred this issue from another repository Apr 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compileall script: add option to use multiple cores #60308

Compileall script: add option to use multiple cores #60308

dholth mannequin commented Oct 1, 2012

dholth mannequin commented Oct 1, 2012

brettcannon commented Oct 1, 2012

PCManticore mannequin commented Dec 10, 2013

merwok commented Mar 12, 2014

PCManticore mannequin commented Mar 12, 2014

merwok commented Mar 12, 2014

merwok commented Mar 12, 2014

brettcannon commented Mar 12, 2014

PCManticore mannequin commented Mar 12, 2014

merwok commented Mar 12, 2014

PCManticore mannequin commented Mar 12, 2014

PCManticore mannequin commented Mar 12, 2014

pitrou commented Mar 13, 2014

PCManticore mannequin commented Mar 13, 2014

pitrou commented Mar 13, 2014

PCManticore mannequin commented Mar 13, 2014

PCManticore mannequin commented Mar 22, 2014

PCManticore mannequin commented Apr 24, 2014

jimjjewett mannequin commented Apr 25, 2014

PCManticore mannequin commented Apr 27, 2014

PCManticore mannequin commented Apr 27, 2014

jimjjewett mannequin commented Apr 28, 2014

PCManticore mannequin commented Apr 30, 2014

PCManticore mannequin commented Sep 10, 2014

python-dev mannequin commented Sep 12, 2014

brettcannon commented Sep 12, 2014

PCManticore mannequin commented Sep 12, 2014

gpshead commented Feb 27, 2020

Compileall script: add option to use multiple cores #60308

Compileall script: add option to use multiple cores #60308

Comments

dholth mannequin commented Oct 1, 2012

dholth mannequin commented Oct 1, 2012

brettcannon commented Oct 1, 2012

PCManticore mannequin commented Dec 10, 2013

merwok commented Mar 12, 2014

PCManticore mannequin commented Mar 12, 2014

merwok commented Mar 12, 2014

merwok commented Mar 12, 2014

brettcannon commented Mar 12, 2014

PCManticore mannequin commented Mar 12, 2014

merwok commented Mar 12, 2014

PCManticore mannequin commented Mar 12, 2014

PCManticore mannequin commented Mar 12, 2014

pitrou commented Mar 13, 2014

PCManticore mannequin commented Mar 13, 2014

pitrou commented Mar 13, 2014

PCManticore mannequin commented Mar 13, 2014

PCManticore mannequin commented Mar 22, 2014

PCManticore mannequin commented Apr 24, 2014

jimjjewett mannequin commented Apr 25, 2014

PCManticore mannequin commented Apr 27, 2014

PCManticore mannequin commented Apr 27, 2014

jimjjewett mannequin commented Apr 28, 2014

PCManticore mannequin commented Apr 30, 2014

PCManticore mannequin commented Sep 10, 2014

python-dev mannequin commented Sep 12, 2014

brettcannon commented Sep 12, 2014

PCManticore mannequin commented Sep 12, 2014

gpshead commented Feb 27, 2020