classification
Title: performance regression in socket getsockaddrarg()
Type: performance Stage:
Components: Library (Lib) Versions: Python 3.5, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: gvanrossum, haypo, loewis, ncoghlan, neologix, pitrou, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2014-08-03 08:17 by neologix, last changed 2014-08-05 14:23 by loewis. This issue is now closed.

Files
File name Uploaded Description Edit
skip_idna.diff loewis, 2014-08-04 15:02 review
skip_idna_alt.patch serhiy.storchaka, 2014-08-04 16:10 review
skip_idna.diff loewis, 2014-08-05 11:02 review
Messages (24)
msg224618 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-08-03 08:17
I noticed that socket.sendto() got noticably slower since 3.4 (at least), compared to 2.7:

2.7:
$ ./python -m timeit -s "import socket; s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM); DATA = b'hello'; TARGET=('127.0.0.1', 4242)" "s.sendto(DATA, TARGET)"
100000 loops, best of 3: 15.8 usec per loop

3.4:
$ ./python -m timeit -s "import socket; s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM); DATA = b'hello'; TARGET=('127.0.0.1', 4242)" "s.sendto(DATA, TARGET)"
10000 loops, best of 3: 25.9 usec per loop

A profile reveals this:
2.7:
         100065 function calls in 2.268 seconds

   Ordered by: cumulative time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1    0.361    0.361    2.268    2.268 test_send.py:1(<module>)
   100000    1.895    0.000    1.895    0.000 {method 'sendto' of '_socket.socket' objects}


3.4:
         906015 function calls (905975 primitive calls) in 6.132 seconds

   Ordered by: cumulative time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
      5/1    0.000    0.000    6.132    6.132 {built-in method exec}
        1    0.334    0.334    6.132    6.132 test_send.py:1(<module>)
   100000    2.347    0.000    5.769    0.000 {method 'sendto' of '_socket.socket' objects}
   100000    1.991    0.000    3.411    0.000 idna.py:147(encode)
   500086    0.894    0.000    0.894    0.000 {built-in method len}
   100000    0.269    0.000    0.269    0.000 {method 'encode' of 'str' objects}
   100000    0.257    0.000    0.257    0.000 {method 'split' of 'bytes' objects}


As can be seen, it's the IDNA encoding which takes a long time, and doesn't appear in the 2.7 profile.
The parsing code (including idna codec) is present in both versions though:
"""
static int
getsockaddrarg(PySocketSockObject *s, PyObject *args,
               struct sockaddr *addr_ret, int *len_ret)
[...]
        if (!PyArg_ParseTuple(args, "eti:getsockaddrarg",
                              "idna", &host, &port))
            return 0;
"""

I'm currently bisecting the commit, but I'm not familiar with encoding stuff.
Is it expected that the IDNA encoding be called when passed an ascii string?
msg224634 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-08-03 13:22
IDNA encoding is quite slow (see 6e1071ed4c66). I'm surprised we accept general hosnames in sendto(), though (rather than plain IP addresses). 25 µs per call is a lot for such a function.
msg224658 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-08-03 20:37
For Python, the encoder is only used when you pass a Unicode string.
msg224665 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-08-03 22:11
> For Python, the encoder is only used when you pass a Unicode string.

Hm...
I'm passing ('127.0.0.1', 4242)as destination, and you can see in the
above profile that the idna encode function is called.
This doesn't occur with 2.7.
msg224666 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-08-03 22:22
OK, I think I see what you mean:

$ ./python -m timeit -s "import socket; s =
socket.socket(socket.AF_INET, socket.SOCK_DGRAM)" "s.sendto(b'hello',
('127.0.0.1', 4242))"10000 loops, best of 3: 44.7 usec per loop
$ ./python -m timeit -s "import socket; s =
socket.socket(socket.AF_INET, socket.SOCK_DGRAM)" "s.sendto(b'hello',
(b'127.0.0.1', 4242))"
10000 loops, best of 3: 23.7 usec per loop

That's really surprising, especially since gethostbyname() and
getaddrinfo() seem to return strings:
$ ./python -m timeit -s "import socket; s =
socket.socket(socket.AF_INET, socket.SOCK_DGRAM);
addr=socket.gethostbyname('127.0.0.1')" "s.sendto(b'hello', (addr,
4242))"

$ ./python -c "import socket; print(type(socket.gethostbyname('127.0.0.1')))"
<class 'str'>
msg224674 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-08-04 02:31
Note that even the bytes version is still quite slow. UDP is used for light-weight protocols where you may send thousands or more messages per second. I'd be curious what the sendto() performance is in raw C.
msg224694 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-08-04 07:22
"Abc" is a bytes string in Python 2 and an Unicode string in Python 3.
msg224696 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-08-04 07:50
> Note that even the bytes version is still quite slow. UDP is used for light-weight protocols where you may send thousands or more messages per second. I'd be curious what the sendto() performance is in raw C.

Ah, I wouldn't rely on the absolyte values, my computer is *slow*.

On a more recent machine, I get this:
100000 loops, best of 3: 8.82 usec per loop

Whereas a C loop gives a 4usec per loop.

> "Abc" is a bytes string in Python 2 and an Unicode string in Python 3.

Sure, but why do getaddrinfo() and gethostbyname() return strings then?

This means that someone using:

addr = getaddrinfo(...)
sendto(DATA, addr)

Will pay the idna encoding upon every call to sendto().
msg224709 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-04 11:28
Perhaps it is time to add support of ipaddress objects in socket functions. Then we could avoid address parsing in tight loop not only for Unicode strings, but for bytes strings too.

s = socket.socket(...)
addr = ipaddress.ip_address(ipaddress.getaddrinfo(...))
for ...:
    s.sendto(DATA, (addr, port))
msg224715 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-08-04 13:14
> Perhaps it is time to add support of ipaddress objects in socket functions.

What I was thinking too :-)
However, beware the parsing cost of ipaddress objects themselves.

One common pattern when doing UDP networking is the following:

  def datagram_received(self, remote_addr, data):
      # process data
      ...
      self.send_to(remote_addr, response_data)

If you want to pass an ipaddress object to send_to, you have to make it so that datagram_received() gives you an ipaddress object too.

Perhaps we need a more low-level solution, e.g. a parsing cache integrated in the C socket module.
msg224717 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-08-04 13:54
Parsing a bytes object i.e. b'127.0.0.1' is done by inet_pton(), so
it's probably cheap (compared to a syscall).

If we had getaddrinfo() and gethostbyname() return bytes instead of
strings, it would be a huge gain.
msg224721 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-08-04 14:02
Charles-François: you get the idna overhead in 2.7, too, by specifying u'127.0.0.1' as the address.

The idna overhead could be bypassed fairly easily in C by:
1. checking that the string is an ASCII string (this is possible in constant time, in 3.x)
2. directly passing the ASCII string to setipaddr (leaving any error detection to this routine)

Before adding caching, I'd check whether a cache lookup is actually faster than calling inet_pton.
msg224730 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-08-04 15:02
The attached patch makes the difference between Unicode and bytes strings for host names negligible, plus it slightly speeds up the bytes case as well.
msg224735 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-08-04 15:30
> Charles-François: you get the idna overhead in 2.7, too, by specifying u'127.0.0.1' as the address.

I don't see it in a profile output, and the timing doesn't change
whether I pass '127.0.0.1' or b'127.0.0.1' in 2.7.
msg224737 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-08-04 15:44
Please understand that Victor and I were asking you to pass a *unicode* object, with a *u* prefix. For me, the time more-than-doubles, on OSX, with the system python.

mvl:~ loewis$ /usr/bin/python -m timeit -s "import socket; s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)" "s.sendto(b'hello', ('127.0.0.1', 4242))"
100000 loops, best of 3: 8.15 usec per loop
mvl:~ loewis$ /usr/bin/python -m timeit -s "import socket; s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)" "s.sendto(b'hello', (u'127.0.0.1', 4242))"
10000 loops, best of 3: 19.5 usec per loop
mvl:~ loewis$ /usr/bin/python -V
Python 2.7.5
msg224739 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-08-04 15:52
> Please understand that Victor and I were asking you to pass a *unicode* object, with a *u* prefix. For me, the time more-than-doubles, on OSX, with the system python.

Sorry, I misread 'b'.
it's a day without...
msg224740 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-04 16:10
> 2. directly passing the ASCII string to setipaddr (leaving any error detection to this routine)

This will change the type of exception. If this is acceptable and modulo Antoine's and my nitpicks on Rietveld, the patch LGTM.

But it is too complicated. Here is alternative. It has many flaws (less extensible, incompatible with Argument Clinic, can produce inaccurate error message, etc), but it is much simpler. And preserve the type of exception.
msg224818 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-08-05 11:02
I have updated my patch per the review.
msg224819 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-08-05 11:12
Serhiy: your patch still changes the type of exception, for 

s.sendto(b'hello',(u'thisisaverylongstringthisisaverylongstringthisisaverylongstringthisisaverylongstring', 4242))

You get a UnicodeError now, but a socket.gaierror then. This is because the name encodes fine as ascii, but still violates the IDNA requirement on label length. My patch does the same. I don't see where your and my patch differ in behavior.

But I agree that your patch is certainly much simpler, while mine might be slightly faster (for not creating copies of the host name).

I'm fine with either being applied. Antoine?
msg224824 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-05 13:07
> Serhiy: your patch still changes the type of exception, for

Oh, really.

> I'm fine with either being applied. Antoine?

May be apply your Argument Clinic friendly patch to 3.5 and simple patch to 
earlier versions?
msg224828 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-08-05 13:38
Martin's approach looks better to me; also, it could be exported for other modules (for example, the ssl module also requests idna encoding at one place).

I don't know if this should be fixed in 3.4. It's a performance improvement, not really a bug fix.
msg224830 - (view) Author: Roundup Robot (python-dev) Date: 2014-08-05 14:11
New changeset bc991d4f9ce7 by Martin v. Löwis in branch 'default':
Issue #22127: Bypass IDNA for pure-ASCII host names (in particular for numeric IPs).
http://hg.python.org/cpython/rev/bc991d4f9ce7

New changeset 0b477934e0a1 by Martin v. Löwis in branch 'default':
Issue #22127: Bypass IDNA for pure-ASCII host names (in particular for numeric IPs).
http://hg.python.org/cpython/rev/0b477934e0a1
msg224831 - (view) Author: Roundup Robot (python-dev) Date: 2014-08-05 14:14
New changeset 49085b746029 by Martin v. Löwis in branch 'default':
Issue #22127: fix typo.
http://hg.python.org/cpython/rev/49085b746029
msg224832 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-08-05 14:22
I agree that this doesn't need to be back ported to 3.4, in particular as there is a minor semantic change (for invalid labels, it might perform a DNS lookup, instead of rejecting them right away).
History
Date User Action Args
2014-08-05 14:23:02loewissetstatus: open -> closed
resolution: fixed
2014-08-05 14:22:53loewissetmessages: + msg224832
2014-08-05 14:14:02python-devsetmessages: + msg224831
2014-08-05 14:11:07python-devsetnosy: + python-dev
messages: + msg224830
2014-08-05 13:38:48pitrousetmessages: + msg224828
2014-08-05 13:08:00serhiy.storchakasetmessages: + msg224824
2014-08-05 11:12:58loewissetmessages: + msg224819
2014-08-05 11:02:21loewissetfiles: + skip_idna.diff

messages: + msg224818
2014-08-04 16:11:00serhiy.storchakasetfiles: + skip_idna_alt.patch

messages: + msg224740
2014-08-04 15:52:56neologixsetmessages: + msg224739
2014-08-04 15:44:02loewissetmessages: + msg224737
2014-08-04 15:30:04neologixsetmessages: + msg224735
2014-08-04 15:02:27loewissetfiles: + skip_idna.diff
keywords: + patch
messages: + msg224730
2014-08-04 14:02:56loewissetmessages: + msg224721
2014-08-04 13:54:19neologixsetmessages: + msg224717
2014-08-04 13:14:33pitrousetnosy: + gvanrossum, ncoghlan
messages: + msg224715
2014-08-04 11:28:41serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg224709
2014-08-04 07:50:33neologixsetmessages: + msg224696
2014-08-04 07:22:49hayposetmessages: + msg224694
2014-08-04 02:31:47pitrousetmessages: + msg224674
2014-08-03 22:22:19neologixsetmessages: + msg224666
2014-08-03 22:11:19neologixsetmessages: + msg224665
2014-08-03 20:37:30hayposetmessages: + msg224658
2014-08-03 13:22:36pitrousetmessages: + msg224634
2014-08-03 08:18:10neologixsettitle: performance regression in socket.getsockaddr() -> performance regression in socket getsockaddrarg()
2014-08-03 08:17:29neologixcreate