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

Use the new selectors module in the subprocess module #63123

Closed
vstinner opened this issue Sep 4, 2013 · 8 comments
Closed

Use the new selectors module in the subprocess module #63123

vstinner opened this issue Sep 4, 2013 · 8 comments
Labels
type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

vstinner commented Sep 4, 2013

BPO 18923
Nosy @pitrou, @vstinner, @giampaolo
Dependencies
  • bpo-19172: selectors: add keys() method
  • Files
  • subprocess_selectors.diff
  • subprocess_selectors-1.diff
  • subprocess_selectors-2.diff
  • subprocess_selectors-3.diff
  • 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 2013-11-09.06:48:46.464>
    created_at = <Date 2013-09-04.18:41:35.038>
    labels = ['type-feature']
    title = 'Use the new selectors module in the subprocess module'
    updated_at = <Date 2013-11-09.06:48:46.463>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2013-11-09.06:48:46.463>
    actor = 'neologix'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-11-09.06:48:46.464>
    closer = 'neologix'
    components = []
    creation = <Date 2013-09-04.18:41:35.038>
    creator = 'vstinner'
    dependencies = ['19172']
    files = ['31966', '32429', '32448', '32511']
    hgrepos = []
    issue_num = 18923
    keywords = ['patch', 'needs review']
    message_count = 8.0
    messages = ['196936', '196937', '196995', '198989', '201774', '202234', '202241', '202441']
    nosy_count = 5.0
    nosy_names = ['pitrou', 'vstinner', 'giampaolo.rodola', 'neologix', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue18923'
    versions = ['Python 3.4']

    @vstinner
    Copy link
    Member Author

    vstinner commented Sep 4, 2013

    Python 3.4 has a new selectors module (issue bpo-16853). It would be nice to use it instead of select.poll or select.select in the subprocess module.

    @vstinner vstinner added the type-feature A feature request or enhancement label Sep 4, 2013
    @vstinner
    Copy link
    Member Author

    vstinner commented Sep 4, 2013

    Other modules using select.select() or select.poll() for more than 1 file descriptor:

    • asyncore
    • multiprocessing.connection
    • multiprocessing.forkserver

    @giampaolo
    Copy link
    Contributor

    It's likely that asyncore won't be able to take any practical advantage from this integration.

    To say one, epoll()/kqueue() pollers won't bring any speedup over select()/poll() because of how asyncore.loop() function is implemented (see http://bugs.python.org/issue6692#msg103628 and http://bugs.python.org/issue11273).

    Also, the new selectors module only takes read and write events into account, whereas asyncore explicitly closes dispatcher in case of disconnection events (POLLOUT, etc).

    In summary I'd say it's a lot wiser to leave asyncore alone and consider it frozen.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Oct 5, 2013

    Here's a patch updating subprocess to use selectors.
    (It depends on the new keys() method - issue bpo-19172.)

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Oct 30, 2013

    Here's an updated patch using the new selector.get_map() method.
    It removes ~100 lines to subprocess, which is always nice.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Nov 5, 2013

    Here's an updated patch with a better logic: in the previous version - based on current poll-based implementation, the FD was inferred from the event (i.e. read ready -> stdout/stderr, write ready -> stderr). The new version directly checks the ready file object instead. I also added an extra safety in case an unknown FD is returned (which should never happen).

    @vstinner
    Copy link
    Member Author

    vstinner commented Nov 5, 2013

    subprocess_selectors-3.diff looks good to me (this patch does not
    remove any test :-))

    I created the issue bpo-19506 for the memoryview optimization.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 8, 2013

    New changeset 71b618f0c8e9 by Charles-François Natali in branch 'default':
    Issue bpo-18923: Update subprocess to use the new selectors module.
    http://hg.python.org/cpython/rev/71b618f0c8e9

    @neologix neologix mannequin closed this as completed Nov 9, 2013
    @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
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants