[issue43332] Make http.client._tunnel send one byte string over the network

[Zveinn]

Zveinn  added the comment:

No problem, 

Hopefully this will improve the performance on some network devices and proxy 
services.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[Senthil Kumaran]

Change by Senthil Kumaran :


--
stage: commit review -> resolved

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[Gregory P. Smith]

Gregory P. Smith  added the comment:

I only ported this back to 3.9 as it is a bit late in 3.8's release cycle for a 
pure performance fix of an issue that has been around for ages.

Thanks for raising the issue.  The main http code already did this, the tunnel 
proxy code path clearly hadn't gotten much love.

--
resolution:  -> fixed
stage: patch review -> commit review
status: open -> closed
versions:  -Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[miss-islington]

miss-islington  added the comment:


New changeset c6e7cf1ee09c88d35e6703c33a61eca7b9db54f3 by Miss Islington (bot) 
in branch '3.9':
bpo-43332: Buffer proxy connection setup packets before sending. (GH-24780)
https://github.com/python/cpython/commit/c6e7cf1ee09c88d35e6703c33a61eca7b9db54f3


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 5.0 -> 6.0
pull_requests: +23549
pull_request: https://github.com/python/cpython/pull/24783

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[Gregory P. Smith]

Gregory P. Smith  added the comment:


New changeset c25910a135c2245accadb324b40dd6453015e056 by Gregory P. Smith in 
branch 'master':
bpo-43332: Buffer proxy connection setup packets before sending. (GH-24780)
https://github.com/python/cpython/commit/c25910a135c2245accadb324b40dd6453015e056


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[Senthil Kumaran]

Change by Senthil Kumaran :


--
versions: +Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[Gregory P. Smith]

Change by Gregory P. Smith :


--
assignee:  -> gregory.p.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[Gregory P. Smith]

Change by Gregory P. Smith :


--
keywords: +patch
pull_requests: +23545
stage: needs patch -> patch review
pull_request: https://github.com/python/cpython/pull/24780

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[Zveinn]

Zveinn  added the comment:

Hey! 

First of all, thank you for not shitting all over me <3 

I have never really used python and the only reason I found this is because I 
was developing a tool that accepted a LOT of CONNECT requests for python and I 
just happened to stumble upon this little nugget.

Regarding a PR or a local path, it would have been too much work for me since 
I'm already swamped and I don't know anything about python or it's ecosystem :S

Anyways, I leave this in your hands now. 

Regards, Zveinn

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[Gregory P. Smith]

Gregory P. Smith  added the comment:

FYI another common socket idiom for this, specifically added for use in old 
HTTP 1.x servers building up responses, is setting and clearing the TCP_CORK 
(Linux) or TCP_NOPUSH (FreeBSD, and maybe macos? [buggy?]) socket option before 
and after the set of send()s you want to be optimally buffered into packets.

A more modern approach (often used with TCP_CORK) is not to build up a single 
buffer at all.  Instead use writev().  os.writev() exists in Python these days. 
 It limits userspace data shuffling in the normal case of everything getting 
written.

Unfortunately... this old http client code and API isn't really setup for that. 
 Everything is required to go through the HTTPConnection.send method.  And the 
send method is not defined to take a list of bytes objects as writev() would 
require.  

So for this code, the patch we want is just the joined bytes or BytesIO. It is 
the simple better change that works everywhere without getting into platform 
specific idioms.

For modern best practices I'd look to the async frameworks instead of this 
older library.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[Gregory P. Smith]

Change by Gregory P. Smith :


--
stage:  -> needs patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[Gregory P. Smith]

Gregory P. Smith  added the comment:

yep, that'd be a worthwhile improvement.  note that the send method in that 
code also accepts BytesIO objects so rather than doing our own sequence of 
bytes and join we could just buffer in one of those.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43332] Make http.client._tunnel send one byte string over the network

[Terry J. Reedy]

Terry J. Reedy  added the comment:

I changed the title to what a PR/commit title should look like.  Your 
justification is that "Multiple writes possibly cause excessive network usage 
and increased implementation complexity on the other end."

I see no problem with the formatting of your first post.

I presume the proposal is to make a list of bytes and then b''.join(the_list).  
This is now a standard idiom.  Have you tested a patch locally?  Can you make a 
PR?

I don't know if there was a particular reason to not join before sending.  
Perhaps because successive sends effectively do the same thing, though with the 
possible  downsides you note.  I am not an expert on network usage.

The _connect method was added by Senthil Kumaran in 2009 in #1424152 and 
revised since.  There is no current http maintainer, so I added as nosy  
Senthil and others who have worked on the module.

--
nosy: +berker.peksag, gregory.p.smith, orsenthil, terry.reedy
title: http/client.py: - uses multiple network writes, possibly causing 
excessive network usage and increased implementation complexity on the other 
end -> Make http.client._tunnel send one byte string over the network

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com