Skip to content

bpo-35380: Enable TCP_NODELAY for proactor event loop #10867

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
Merged
asvetlov merged 2 commits into from
Dec 3, 2018
Merged

bpo-35380: Enable TCP_NODELAY for proactor event loop #10867

asvetlov merged 2 commits into from
Dec 3, 2018

Conversation

@asvetlov
Copy link
Contributor

@asvetlov asvetlov commented Dec 3, 2018 •
edited
Loading

Restore a consitensy with selector based event loops.
TCP_NODELAY is enabled starting from Python 3.6

https://bugs.python.org/issue35380

@asvetlov asvetlov requested a review from 1st1 as a code owner December 3, 2018 12:43
1st1
1st1 approved these changes Dec 3, 2018
View reviewed changes
Copy link
Member

@1st1 1st1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit cautious (i.e. I'm +0) with backporting this to 3.7. Are you sure it's 100% safe?

Lib/asyncio/base_events.py
futures._get_loop(fut).stop()


if hasattr(socket, 'TCP_NODELAY'):
Copy link
Member

@1st1 1st1 Dec 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a new asyncio.sockutils module for this kind of stuff.

Copy link
Contributor Author

@asvetlov asvetlov Dec 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sockutils.py sounds great but in the case I doubt if the PR can be backported.

Copy link
Member

@1st1 1st1 Dec 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I suggest to do that in a follow up PR specifically for the master branch.

Copy link
Contributor Author

@asvetlov asvetlov Dec 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created https://bugs.python.org/issue35392 for it.

@asvetlov
Copy link
Contributor Author

asvetlov commented Dec 3, 2018

Yes, the change is safe: Windows supports TCP_NODELAY very well for any version, at least I used it on Win 2000 and XP.

Without backport third parties like aiohttp should check for event loop type and enable the mode for proactor only, which looks very odd.

@1st1
Copy link
Member

1st1 commented Dec 3, 2018

Without backport third parties like aiohttp should check for event loop type and enable the mode for proactor only, which looks very odd.

SGTM

@asvetlov asvetlov merged commit 3bc0eba into python:master Dec 3, 2018
@miss-islington
Copy link
Contributor

miss-islington commented Dec 3, 2018

Thanks @asvetlov for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

@bedevere-bot
Copy link

bedevere-bot commented Dec 3, 2018

@asvetlov: Please replace # with GH- in the commit message next time. Thanks!

@asvetlov asvetlov deleted the tcp-nodelay-windows branch December 3, 2018 19:08
@bedevere-bot
Copy link

bedevere-bot commented Dec 3, 2018

GH-10872 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 3, 2018
@asvetlov @miss-islington
bpo-35380: Enable TCP_NODELAY for proactor event loop (pythonGH-10867)
a6f24bd 
(cherry picked from commit 3bc0eba)

Co-authored-by: Andrew Svetlov <[email protected]>
@miss-islington
Copy link
Contributor

miss-islington commented Dec 3, 2018

Sorry, @asvetlov, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 3bc0ebab17bf5a2c29d2214743c82034f82e6573 3.6

@bedevere-bot
Copy link

bedevere-bot commented Dec 3, 2018

GH-10874 is a backport of this pull request to the 3.6 branch.

asvetlov added a commit to asvetlov/cpython that referenced this pull request Dec 3, 2018
@asvetlov
[3.6] bpo-35380: Enable TCP_NODELAY for proactor event loop (pythonGH…
6b9d758 
…-10867).

(cherry picked from commit 3bc0eba)

Co-authored-by: Andrew Svetlov <[email protected]>
asvetlov added a commit that referenced this pull request Dec 3, 2018
@miss-islington @asvetlov
[3.7] bpo-35380: Enable TCP_NODELAY for proactor event loop (GH-10867) (
fe91e9b 
GH-10872)

* bpo-35380: Enable TCP_NODELAY for proactor event loop (GH-10867)
(cherry picked from commit 3bc0eba)

Co-authored-by: Andrew Svetlov <[email protected]>
asvetlov added a commit that referenced this pull request Dec 5, 2018
@asvetlov
[3.6] bpo-35380: Enable TCP_NODELAY for proactor event loop (GH-10867)…
bfb8818 
…. (GH-10874)

* [3.6] bpo-35380: Enable TCP_NODELAY for proactor event loop (GH-10867).
(cherry picked from commit 3bc0eba)

Co-authored-by: Andrew Svetlov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@1st1 1st1 1st1 approved these changes

Assignees

@asvetlov asvetlov

Labels

topic-asyncio

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@asvetlov @1st1 @miss-islington @bedevere-bot @the-knights-who-say-ni