Initial publication of smp module for Python #1
Conversation
setup.py
Outdated
| url ="https://software.intel.com/intel-distribution-for-python", | ||
| author ="Intel Corporation", | ||
| author_email="scripting@intel.com", | ||
| license ="Intel EULA", |
| except: | ||
| ProcessPoolExecutor = None | ||
| ThreadPoolExecutor = None | ||
| from ctypes import * |
There was a problem hiding this comment.
that's fair.. we'll fix that later though
fschlimb
left a comment
fschlimb
left a comment
There was a problem hiding this comment.
The code looks very nice. As noted, I might be able to give feedback on the content once I better understand what it's doing.
| threads_per_process = oversubscription_factor | ||
| if cpu_count >= process_count: | ||
| threads_per_process = threads_per_process*int(round(float(cpu_count) | ||
| /float(process_count))) |
There was a problem hiding this comment.
I am not sure I understand what this block is really doing.
Generally, the entire file has only 2 comments/hints. I have not been involved in any of the discussions about this topic so it's hard to follow what exactly is happening. The description in the other file does not help much either (as it does not go beyond re-phrasing the names of the arguments). There is no explanation what so ever of what this module is supposed to do, e.g. what the different input arguments really mean and how this works.
I suggest you add a high-level description of this file is really doing and add more specific comment to each function. Things like lines 223-225 could also use some comment.
| process_count=1, process_id=0): | ||
| from multiprocessing.pool import worker | ||
| set_proc_affinity(process_count, process_id) | ||
| worker(inqueue, outqueue, initializer, initargs, maxtasks, wrap_exception) |
There was a problem hiding this comment.
This function is almost identical with the 27 version. I see no reason for this code-duplication.
| w.name = w.name.replace('Process', 'PoolWorker') | ||
| w.daemon = True | ||
| w.start() | ||
| debug('added worker') |
There was a problem hiding this comment.
I guess generally all this code duplication should be avoided.
| global max_top_workers | ||
|
|
||
| if not sys.platform.startswith('linux'): | ||
| raise "Only linux is currently supported" |
There was a problem hiding this comment.
some platform specific is encoded right now, Anton G will rewrite it later
2 participants
@avgorshk