Visitar URL original
asyncio: proactor read transport: use recv_into instead of recv · Issue #85445 · python/cpython · GitHub
Skip to content

asyncio: proactor read transport: use recv_into instead of recv #85445

Closed
Closed
asyncio: proactor read transport: use recv_into instead of recv#85445
Labels
performancePerformance or resource usagetopic-asyncio

Description

@tontinton
tontinton
mannequin
opened on Jul 10, 2020
BPO 41273
Nosy @db3l, @scoder, @vstinner, @1st1, @tontinton
PRs
  • bpo-41273: asyncio's proactor read transport's better performance by using recv_into instead of recv #21439
  • bpo-41273: asyncio's proactor read transport's better performance by using recv_into instead of recv #21442
  • gh-85451: Add StreamReaderBufferedProtocol #21446
    		•

    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 2020-07-10.21:08:56.958>
labels = []
title = 'asyncio: proactor read transport: use recv_into instead of recv'
updated_at = <Date 2020-08-06.09:48:07.805>
user = 'https://github.com/tontinton'

    bugs.python.org fields:

    activity = <Date 2020-08-06.09:48:07.805>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2020-07-10.21:08:56.958>
creator = 'tontinton'
dependencies = []
files = []
hgrepos = []
issue_num = 41273
keywords = ['patch']
message_count = 10.0
messages = ['373483', '373655', '373935', '373969', '373970', '373974', '373981', '373996', '374896', '374924']
nosy_count = 5.0
nosy_names = ['db3l', 'scoder', 'vstinner', 'yselivanov', 'tontinton']
pr_nums = ['21439', '21442', '21446']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue41273'
versions = []

    Activity

    1. tontinton commented on Jul 10, 2020

      @tontinton
      tontintonmannequin
      on Jul 10, 2020
      MannequinAuthor

      Using recv_into instead of recv in the transport _loop_reading will speed up the process.

      From what I checked it's about 120% performance increase.

      This is only because there should not be a new buffer allocated each time we call recv, it's really wasteful.

    2. 1st1 commented on Jul 14, 2020

      @1st1
      1st1
      on Jul 14, 2020
      Member

      New changeset 568fb0f by Tony Solomonik in branch 'master':
      bpo-41273: asyncio's proactor read transport's better performance by using recv_into instead of recv (bpo-21442)
      568fb0f

    3. scoder commented on Jul 19, 2020

      @scoder
      scoder
      on Jul 19, 2020
      Contributor

      There have been sporadic buildbot failures in "test_asyncio" since this change, message being "1 test altered the execution environment", e.g.

      https://buildbot.python.org/all/#/builders/129/builds/1443

      Could someone please check if they are related?

    4. db3l commented on Jul 19, 2020

      @db3l
      db3l
      on Jul 19, 2020
      Contributor

      I can at least confirm that this commit is the source of the issue on the Windows 10 buildbot. Interactively, it fails every time with it in place (unlike the buildbot which has had the occasional success) and passes reliably with it reverted.

      The error arises during the SubprocessProactorTests test suite, but seemingly not with a specific test, just something that shows up when the entire suite is run. At least I haven't been able to get it to occur by running individual tests.

      Since the buildbot test output hides the underlying exception, here's the details of the two unraisable exceptions in question from test_asyncio.test_subprocess.SubprocessProactorTests. I enabled tracemalloc but the original allocation is from the common utils module so probably not that much extra help:

      D:\test\cpython\lib\asyncio\windows_utils.py:112: ResourceWarning: unclosed <PipeHandle handle=424>
  _warn(f"unclosed {self!r}", ResourceWarning, source=self)
Object allocated at (most recent call last):
  File "D:\test\cpython\lib\asyncio\windows_utils.py", lineno 164
    self.stdout = PipeHandle(stdout_rh)
D:\test\cpython\lib\asyncio\windows_utils.py:112: ResourceWarning: unclosed <PipeHandle handle=476>
  _warn(f"unclosed {self!r}", ResourceWarning, source=self)
Object allocated at (most recent call last):
  File "D:\test\cpython\lib\asyncio\windows_utils.py", lineno 166
    self.stderr = PipeHandle(stderr_rh)
Exception ignored in: <function _ProactorBasePipeTransport.__del__ at 0x000001DEB55D14B0>
Traceback (most recent call last):
  File "D:\test\cpython\lib\asyncio\proactor_events.py", line 115, in __del__
    _warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
  File "D:\test\cpython\lib\asyncio\proactor_events.py", line 79, in __repr__
    info.append(f'fd={self._sock.fileno()}')
  File "D:\test\cpython\lib\asyncio\windows_utils.py", line 102, in fileno
    raise ValueError("I/O operation on closed pipe")
ValueError: I/O operation on closed pipe
Exception ignored in: <function _ProactorBasePipeTransport.__del__ at 0x000001DEB55D14B0>
Traceback (most recent call last):
  File "D:\test\cpython\lib\asyncio\proactor_events.py", line 115, in __del__
    _warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
  File "D:\test\cpython\lib\asyncio\proactor_events.py", line 79, in __repr__
    info.append(f'fd={self._sock.fileno()}')
  File "D:\test\cpython\lib\asyncio\windows_utils.py", line 102, in fileno
    raise ValueError("I/O operation on closed pipe")
ValueError: I/O operation on closed pipe

    5. tontinton commented on Jul 19, 2020

      @tontinton
      tontintonmannequin
      on Jul 19, 2020
      MannequinAuthor

      I see, I'll start working on a fix soon

    6. tontinton commented on Jul 19, 2020

      @tontinton
      tontintonmannequin
      on Jul 19, 2020
      MannequinAuthor

      Ok so I checked and the PR I am currently having a CR on fixes this issue: #21446

      Do you want me to make a different PR tomorrow that fixes this specific issue to get it faster to master or is it ok to wait a bit?

    7. db3l commented on Jul 20, 2020

      @db3l
      db3l
      on Jul 20, 2020
      Contributor

      First, I also no longer see the error with a local PR 21446 build on the Win10 buildbot.

      As for timing, I believe policy is to revert, but in my view it can probably depend on how short "a bit" is for the fix. We've been in this state for 4-5 days already, so one more day, for example, probably won't matter that much.

      If it looks like it'll be longer, or the timing is uncertain, then I'd probably suggest reverting until the new PR is ready.

    8. tontinton commented on Jul 20, 2020

      @tontinton
      tontintonmannequin
      on Jul 20, 2020
      MannequinAuthor

      If the error is not resolved yet, I would prefer if we revert this change then.

      The new PR is kinda big I don't know when it will be merged.

    9. db3l commented on Aug 5, 2020

      @db3l
      db3l
      on Aug 5, 2020
      Contributor

      It looks like there was an underlying asyncio.recv_into bug that was the likely root issue here. It's recently been fixed in bpo-41467, and test_asyncio is passing on at least the Win10 buildbot.

    10. vstinner commented on Aug 6, 2020

      @vstinner
      vstinner
      on Aug 6, 2020
      Member

      Tony: "asyncio: proactor read transport: use recv_into instead of recv" seems to be implemented, but PR 21446 "bpo-41279: Add StreamReaderBufferedProtocol" is a new feature. I suggest to open a new issue for this feature, and close this one.

    11. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    12. AlexWaygood
      added
      performancePerformance or resource usage
      topic-asyncio
      on Apr 10, 2022
    13. ezio-melotti
      added this to asyncioon Jun 20, 2022
    14. kumaraditya303
      closed this as completedon Jul 7, 2022
    15. kumaraditya303
      moved this to Done in asyncioon Jul 7, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      performancePerformance or resource usagetopic-asyncio

      Projects

      • asyncio
        Status
        Done

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions