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

Jump to bottom

Add the SIO_LOOPBACK_FAST_PATH option to socket.ioctl #70723

Closed
DanielStokes mannequin opened this issue Mar 11, 2016 · 16 comments
Closed

Add the SIO_LOOPBACK_FAST_PATH option to socket.ioctl #70723

DanielStokes mannequin opened this issue Mar 11, 2016 · 16 comments
Assignees
@zooba
Labels
extension-modules C modules in the Modules dir OS-windows type-feature A feature request or enhancement

Comments

@DanielStokes
Copy link
Mannequin

DanielStokes mannequin commented Mar 11, 2016

BPO 26536
Nosy @pfmoore, @pitrou, @tjguk, @berkerpeksag, @vadmium, @zware, @zooba
Files
  • loopback_fast_path.patch
  • loopback_fast_path_v2.patch
  • loopback_fast_path_test.zip
  • loopback_fast_path_v3.patch
    		•

    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 = 'https://github.com/zooba'
closed_at = <Date 2016-06-18.13:40:54.454>
created_at = <Date 2016-03-11.03:15:26.080>
labels = ['extension-modules', 'type-feature', 'OS-windows']
title = 'Add the SIO_LOOPBACK_FAST_PATH option to socket.ioctl'
updated_at = <Date 2016-06-18.17:17:47.218>
user = 'https://bugs.python.org/DanielStokes'

    bugs.python.org fields:

    activity = <Date 2016-06-18.17:17:47.218>
actor = 'steve.dower'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2016-06-18.13:40:54.454>
closer = 'berker.peksag'
components = ['Extension Modules', 'Windows']
creation = <Date 2016-03-11.03:15:26.080>
creator = 'Daniel Stokes'
dependencies = []
files = ['42124', '42888', '42895', '42908']
hgrepos = []
issue_num = 26536
keywords = ['patch']
message_count = 16.0
messages = ['261539', '264326', '264335', '265805', '265845', '265847', '265906', '268738', '268739', '268777', '268801', '268802', '268803', '268805', '268806', '268816']
nosy_count = 9.0
nosy_names = ['paul.moore', 'pitrou', 'tim.golden', 'python-dev', 'berker.peksag', 'martin.panter', 'zach.ware', 'steve.dower', 'Daniel Stokes']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue26536'
versions = ['Python 3.6']
    @DanielStokes
    Copy link
    Mannequin Author

    DanielStokes mannequin commented Mar 11, 2016

    Adding the SIO_LOOPBACK_FAST_PATH option to socket.ioctl on Windows allows Windows users to enable the loopback fast path option available on Windows 8+. This allows for much better TCP loopback performance on Windows.

    For more information on TCP Loopback Fast Path, see:

    @DanielStokes DanielStokes mannequin added topic-IO type-feature A feature request or enhancement labels Mar 11, 2016
    @SilentGhost SilentGhost mannequin added the OS-windows label Mar 11, 2016
    @DanielStokes
    Copy link
    Mannequin Author

    DanielStokes mannequin commented Apr 26, 2016

    The "Lifecycle of a Patch" document recommends pinging an issue after a month of no review. I do not see any special ping option, so I am assuming that means to post a comment.

    To help with code review I tried to keep this change very similar to a previous similar change: http://bugs.python.org/issue6971

    @berkerpeksag
    Copy link
    Member

    Thanks for pinging, Daniel. I'm not a Windows user so I can't test the patch, but I left some review comments on Rietveld: http://bugs.python.org/review/26536/

    @berkerpeksag berkerpeksag added extension-modules C modules in the Modules dir OS-windows and removed OS-windows topic-IO labels Apr 27, 2016
    @DanielStokes
    Copy link
    Mannequin Author

    DanielStokes mannequin commented May 18, 2016

    Thank you for the review. I have made the changes suggested in the code and I have uploaded a new patch.

    @DanielStokes
    Copy link
    Mannequin Author

    DanielStokes mannequin commented May 19, 2016

    I have added a zip containing a simple server and client to test the patch with. With loopback fast path enabled, I get 3-5x more throughput on my Windows 10 computer with an AMD 900 series chipset.

    @berkerpeksag
    Copy link
    Member

    Thanks for the test script. I left some minor comments about loopback_fast_path_v2.patch on Rietveld.

    @DanielStokes
    Copy link
    Mannequin Author

    DanielStokes mannequin commented May 20, 2016

    I have uploaded a new patch with the suggested changes, thanks for the review.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 17, 2016

    New changeset f8957c755c7a by Steve Dower in branch 'default':
    Issue bpo-26536: socket.ioctl now supports SIO_LOOPBACK_FAST_PATH. Patch by Daniel Stokes.
    https://hg.python.org/cpython/rev/f8957c755c7a

    @zooba
    Copy link
    Member

    zooba commented Jun 17, 2016

    All looked good to me, and as far as I could see all of Berker's feedback was addressed, so consider it in!

    @zooba zooba closed this as completed Jun 17, 2016
    @zooba zooba self-assigned this Jun 17, 2016
    @vadmium
    Copy link
    Member

    vadmium commented Jun 18, 2016

    This is failing on Windows 7 buildbots:

    http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/7849/steps/test/logs/stdio
    ======================================================================
    ERROR: test_sio_loopback_fast_path (test.test_socket.GeneralModuleTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_socket.py", line 1226, in test_sio_loopback_fast_path
    s.ioctl(socket.SIO_LOOPBACK_FAST_PATH, True)
OSError: [WinError 10045] The attempted operation is not supported for the type of object referenced

    @vadmium vadmium reopened this Jun 18, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 18, 2016

    New changeset e8b10ed64e63 by Berker Peksag in branch 'default':
    Issue bpo-26536: Skip test_sio_loopback_fast_path under Windows 7
    https://hg.python.org/cpython/rev/e8b10ed64e63

    @berkerpeksag
    Copy link
    Member

    It looks like SIO_LOOPBACK_FAST_PATH is also defined in older Windows versions. I updated the test to skip if SIO_LOOPBACK_FAST_PATH is defined and exc.winerror is WSAEOPNOTSUPP. Buildbot is now green: http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/7855

    Please reopen if I did something wrong.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 18, 2016

    New changeset 213c240cce0a by Berker Peksag in branch 'default':
    Issue bpo-26536: Use spaces instead of tabs
    https://hg.python.org/cpython/rev/213c240cce0a

    @zooba
    Copy link
    Member

    zooba commented Jun 18, 2016

    That'll handle the test fine, though I wonder whether we should try and conditionally define the constant at runtime?

    Probably at least want to add a note to the docs about Win 8 being the minimum, especially since so many people are still on 7.

    @berkerpeksag
    Copy link
    Member

    That'll handle the test fine, though I wonder whether we should try and conditionally define the constant at runtime?

    +1. Is it safe to use IsWindows8OrGreater()?

    @zooba
    Copy link
    Member

    zooba commented Jun 18, 2016

    That's the best function to use.

    @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

    @zooba zooba

    Labels
    extension-modules C modules in the Modules dir OS-windows type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @berkerpeksag @vadmium @zooba