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.

Sign up for GitHub

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

Jump to bottom

Add os.preadv() and os.pwritev() #75549

Closed
YoSTEALTH mannequin opened this issue Sep 6, 2017 · 18 comments
Closed

Add os.preadv() and os.pwritev() #75549

YoSTEALTH mannequin opened this issue Sep 6, 2017 · 18 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@YoSTEALTH
Copy link
Mannequin

YoSTEALTH mannequin commented Sep 6, 2017

BPO 31368
Nosy @pitrou, @vstinner, @ned-deily, @njsmith, @YoSTEALTH, @nitishch, @miss-islington
PRs
  • bpo-31368: Expose preadv (preadv2) and pwritev (pwritev2) in the os module #5239
  • bpo-31368: Enhance os.preadv() documentation #5415
  • bpo-31368: Enhance os.preadv() documentation #5467
  • bpo-31368: Enhance os.preadv() documentation #7254
  • [3.7] bpo-31368: Enhance os.preadv() documentation (GH-7254) #7258
    		•

    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 2018-05-30.23:26:01.392>
created_at = <Date 2017-09-06.19:57:29.982>
labels = ['3.7', '3.8', 'type-feature', 'library']
title = 'Add os.preadv() and os.pwritev()'
updated_at = <Date 2018-05-30.23:26:01.390>
user = 'https://github.com/YoSTEALTH'

    bugs.python.org fields:

    activity = <Date 2018-05-30.23:26:01.390>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2018-05-30.23:26:01.392>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2017-09-06.19:57:29.982>
creator = 'YoSTEALTH'
dependencies = []
files = []
hgrepos = []
issue_num = 31368
keywords = ['patch']
message_count = 18.0
messages = ['301508', '309898', '310003', '310030', '310033', '310089', '310454', '310828', '310829', '310831', '310869', '310881', '311020', '317536', '318158', '318218', '318221', '318223']
nosy_count = 7.0
nosy_names = ['pitrou', 'vstinner', 'ned.deily', 'njs', 'YoSTEALTH', 'nitishch', 'miss-islington']
pr_nums = ['5239', '5415', '5467', '7254', '7258']
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue31368'
versions = ['Python 3.7', 'Python 3.8']
    @YoSTEALTH
    Copy link
    Mannequin Author

    YoSTEALTH mannequin commented Sep 6, 2017

    Asynchronous, Non-blocking Buffered File Read

    "RWF_NONBLOCK" flag for os.open()

    Link:
    https://lwn.net/Articles/612483/
    https://lwn.net/Articles/613068/
    https://lwn.net/Articles/636967/

    @YoSTEALTH YoSTEALTH mannequin added 3.7 (EOL) end of life type-feature A feature request or enhancement labels Sep 6, 2017
    @YoSTEALTH
    Copy link
    Mannequin Author

    YoSTEALTH mannequin commented Jan 13, 2018

    preadv2(2) syscall with RWF_NONBLOCK feature is now released starting linux4.14
    https://kernelnewbies.org/Linux_4.14#Asynchronous_buffered_I.2FO_support

    @ned-deily
    Copy link
    Member

    It seems to me there are two issues here:

    1. What are the pros and cons of adding support for this new mode, e.g. are there any potential gotchas?

    2. Assuming the result of 1 is we should go ahead, then omeone needs to generate a pull request including configure checks as necessary, doc changes and tests.

    It's getting late in the game for inclusion in 3.7 with only 2 weeks left until feature freeze.

    @ned-deily ned-deily added the stdlib Python modules in the Lib dir label Jan 15, 2018
    @ned-deily ned-deily changed the title RWF_NONBLOCK Support asynchronous, non-blocking buffered reads (RWF_NONBLOCK) Jan 15, 2018
    @pitrou
    Copy link
    Member

    pitrou commented Jan 16, 2018

    In any case, I think this would take the form of new low-level calls in the posix module, i.e. add thin wrappers around the new system / libc calls.

    @njsmith
    Copy link
    Contributor

    njsmith commented Jan 16, 2018

    Whoa, cool.

    bpo-32561 is complementary to this. (They both make sense on their own, but they're even better together.)

    @YoSTEALTH
    Copy link
    Mannequin Author

    YoSTEALTH mannequin commented Jan 16, 2018

    According to this nginx test https://www.nginx.com/blog/thread-pools-boost-performance-9x/ there is a huge boost in performance when using thread poll for File IO. It is postulated that when preadv2() & RWF_NONBLOCK feature are available it would further benefit (latency, thread sync, ...)

    I am not a C coder so i can't just jump into this and code this feature (if i could i would have done it long ago). As Antoine mentioned, it needs to written at C (+ctypes) level.

    I am willing to help, hit me up on #python user: stealth_

    @vstinner
    Copy link
    Member

    Explanation for myself.

    https://kernelnewbies.org/Linux_4.14#Asynchronous_buffered_I.2FO_support

    "In this release, the preadv2(2) syscall with RWF_NONBLOCK will let userspace applications bypass enqueuing operation in the threadpool if it's already available in the pagecache."

    For applications using a thread pool, like the aiofiles does for asyncio, preadv2() allows to bypass the thread pool which is obviously faster. If the read block, just uses thread pool as currently done.

    Same rationale for pwritev2().

    @vstinner
    Copy link
    Member

    I set the priority to release blocker to make sure that I don't forget to merge the PR before Python 3.7b1 (next monday?). The PR is *almost* ready, remaining issues are mostly related to documentation and coding style.

    @vstinner
    Copy link
    Member

    Antoine Pitrou: "In any case, I think this would take the form of new low-level calls in the posix module, i.e. add thin wrappers around the new system / libc calls."

    "new system / libc calls"... well, preadv() is available on FreeBSD since FreeBSD 6 :-) And preadv() and pwritev() first appeared in Linux 2.6.30; library support was added in glibc 2.10. It's not really something "new".

    It seems like the two functions became more interesting for Python since Linux implemented the new RWF_NONBLOCK flag. aiofiles can be used to bypass its thread-pool, for better performances.

    @vstinner vstinner changed the title Support asynchronous, non-blocking buffered reads (RWF_NONBLOCK) Add os.preadv() and os.pwritev() Jan 27, 2018
    @vstinner
    Copy link
    Member

    FYI in 2013 I proposed to use writev() in the io module, but it was inefficient: bpo-17655.

    @vstinner
    Copy link
    Member

    New changeset 4defba3 by Victor Stinner (Pablo Galindo) in branch 'master':
    bpo-31368: Expose preadv and pwritev in the os module (bpo-5239)
    4defba3

    @YoSTEALTH
    Copy link
    Mannequin Author

    YoSTEALTH mannequin commented Jan 27, 2018

    Thank you Pablo Galindo for your amazingly hard work. Also Victor Stinner for your guidance, thanks you :)

    @ned-deily
    Copy link
    Member

    If I understand the discussion on the PR properly, Victor would like to revisit the documentation for this PR but is OK with the code as merged. I'm therefore downgrading this to "deferred blocker". Please try to get the issue closed one way or another prior to 3.7.0b2, thanks!

    @ned-deily ned-deily added 3.8 only security fixes deferred-blocker and removed release-blocker labels Jan 28, 2018
    @ned-deily
    Copy link
    Member

    As far as I can tell, this issue is still open only because of the original desire to expand the original documentation. YoSTEALTH produced PR 5447 based on Victor's original doc update PR. In the meantime, the current 3.7 docs have been updated somewhat. YoSTEALTH, if you would be willing to update PR 5467 perhaps we can get it reviewed and merged and close this issue. In any case, I'm lowering the priority from "deferred blocker".

    @YoSTEALTH
    Copy link
    Mannequin Author

    YoSTEALTH mannequin commented May 30, 2018

    I can't at the moment, i am out of country for couple more months.

    @vstinner
    Copy link
    Member

    New changeset 02e2a08 by Victor Stinner (Pablo Galindo) in branch 'master':
    bpo-31368: Enhance os.preadv() documentation (GH-7254)
    02e2a08

    @miss-islington
    Copy link
    Contributor

    New changeset 0e823c6 by Miss Islington (bot) in branch '3.7':
    bpo-31368: Enhance os.preadv() documentation (GH-7254)
    0e823c6

    @vstinner
    Copy link
    Member

    Thanks Pablo Galindo for the new better documentation! (And thanks myself since I wrote the original documentation PR ;-))

    @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
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    5 participants
    @vstinner @njsmith @pitrou @ned-deily @miss-islington