http parser implementation based on http-parser #1626

fafhrd91 · 2017-02-14T21:25:23Z

added c-based http parser, cython code is based on https://github.com/MagicStack/httptools

it gives ~15-25% increase

kxepal · 2017-02-14T21:33:36Z

aiohttp/_parser.pyx

+            self._upgraded = True
+
+        encoding = None
+        enc = headers.get(hdrs.CONTENT_ENCODING)


Is there any need to have two variables to reference on content encoding?

kxepal · 2017-02-14T21:34:45Z

aiohttp/_parser.pyx

+                return HttpVersion10
+            elif parser.http_minor == 1:
+                return HttpVersion11
+


Should we crash if version will be something weird?

kxepal · 2017-02-14T21:35:52Z

aiohttp/_parser.pyx

+            messages = self._messages
+            self._messages = []
+        else:
+            messages = ()


self._messages are list, but here messages becomes a tuple.

i do not want to convert messages into tuple. doesnt really matter

kxepal · 2017-02-14T21:36:14Z

aiohttp/_parser.pyx

+                 size_t max_field_size=8190):
+         self._init(cparser.HTTP_REQUEST, protocol, loop,
+                    max_line_size, max_headers, max_field_size)
+        #self._proto_on_url = getattr(protocol, 'on_url', None)


May be drop this?

i've not decided on url, http-parser provides url parsing, should give some more performance, but it requires yarl integration

kxepal · 2017-02-14T21:36:57Z

aiohttp/_parser.pyx

+                   const char *at, size_t length) except -1:
+    cdef HttpParser pyparser = <HttpParser>parser.data
+    try:
+        #pyparser.url = _parse_url(at[:length], length)


May be remove this?

same as previous

kxepal · 2017-02-14T21:39:27Z

aiohttp/_parser.pyx

+cdef int cb_on_chunk_complete(cparser.http_parser* parser) except -1:
+    cdef HttpParser pyparser = <HttpParser>parser.data
+    try:
+        pass


Something TBD here?

kxepal · 2017-02-14T21:40:13Z

aiohttp/_parser.pyx

+cdef class URL:
+    cdef readonly str schema
+    cdef readonly str host
+    cdef readonly object port


Why port is an object?

i have no idea :)
url handling needs some more work

kxepal · 2017-02-14T21:41:32Z

aiohttp/server.py

+                        loop=self._loop))
+                return
+            except Exception as exc:
+                print('exc')


Debug print.

kxepal · 2017-02-14T21:42:04Z

aiohttp/server.py

+
+        # feed payload
+        else:
+            if data:


elif data?

kxepal · 2017-02-14T21:43:05Z

tests/test_proxy_functional.py

@@ -267,9 +275,9 @@ def request(pid):
    yield from sess.close()


-@pytest.mark.xfail
+# @pytest.mark.xfail


May be remove all these # pytest.mark.xfail?

i do not understand those tests, i need andrew help

codecov-io · 2017-02-15T00:34:08Z

Codecov Report

❗ No coverage uploaded for pull request base (master@6a3c16a). Click here to learn what that means.
The diff coverage is 84.14%.

@@            Coverage Diff            @@
##             master    #1626   +/-   ##
=========================================
  Coverage          ?   94.61%           
=========================================
  Files             ?       31           
  Lines             ?     7395           
  Branches          ?     1281           
=========================================
  Hits              ?     6997           
  Misses            ?      264           
  Partials          ?      134

Impacted Files	Coverage Δ
aiohttp/connector.py	`97.5% <ø> (ø)`
aiohttp/client_reqrep.py	`96.28% <100%> (ø)`
aiohttp/web_ws.py	`92.18% <100%> (ø)`
aiohttp/web_server.py	`100% <100%> (ø)`
aiohttp/web_urldispatcher.py	`98.83% <100%> (ø)`
aiohttp/signals.py	`100% <100%> (ø)`
aiohttp/client.py	`88.82% <100%> (ø)`
aiohttp/web_middlewares.py	`100% <100%> (ø)`
aiohttp/web.py	`98.74% <100%> (ø)`
aiohttp/errors.py	`100% <100%> (ø)`
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a3c16a...e49b59b. Read the comment docs.

fafhrd91 · 2017-02-15T00:35:33Z

added http-parser's url parsing functionality.

on my mac i get ~14.3k rps, ~7k was on 1.3

fafhrd91 · 2017-02-15T00:36:03Z

1.2 is slightly faster than 1.3, ~7.3k rps

fafhrd91 · 2017-02-15T07:03:12Z

with all latest changes i could get ~16.2rps

interestingly, __new__ method adds huge overhead.

fafhrd91 · 2017-02-15T14:09:30Z

cool, i didn't know about purest_addoption

fafhrd91 · 2017-02-15T14:29:06Z

@samuelcolvin added --fast option

samuelcolvin · 2017-02-15T14:34:21Z

great!

samuelcolvin · 2017-02-15T14:36:41Z

aiohttp/test_utils.py

    """Teardown and cleanup an event_loop created
    by setup_test_loop.

    """
+    if fast is None:


I think this should be

fast |= TESTS_FAST

that way if you're testing with pytest and have AIOHTTP_TESTS_FAST=True but don't supply the --fast argument you get fast tests.

With current setup AIOHTTP_TESTS_FAST has no effect with pytest.

I think AIOHTTP_TESTS_FAST is not needed

fine by me.

…during tests

fafhrd91 · 2017-02-15T19:25:35Z

i modified gunicorn worker to disable access log if access log is not configured

on my mac i got ~60k rps

t_srv.py:

async def intro(request):
    resp = Response(status=200)
    return resp

def init():
    app = Application()
    app.router.add_get('/', intro)
    return app

app = init()

gunicorn command:

>>> python -OO -m gunicorn.app.wsgiapp --worker-class aiohttp.worker.GunicornUVLoopWebWorker t_srv:app --workers 6 -b 127.0.0.1:8080

and wrk command:

wrk -t12 -c400 -d10s http://127.0.0.1:8080/ -s pipeline.lua --latency -- / 16

nhumrich

🖌

fafhrd91 · 2017-02-16T00:55:23Z

merged to master

@kxepal if you want to modify cython code, just modify on master

lock · 2019-10-29T04:04:30Z

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

fafhrd91 force-pushed the parser branch from 0a36ded to afdec09 Compare February 14, 2017 21:31

kxepal reviewed Feb 14, 2017

View reviewed changes

fafhrd91 mentioned this pull request Feb 15, 2017

replace http parser? #858

Closed

fafhrd91 force-pushed the parser branch from b8fcff3 to 3458cd7 Compare February 15, 2017 04:36

samuelcolvin reviewed Feb 15, 2017

View reviewed changes

fafhrd91 force-pushed the parser branch from 2c15a3f to bf582a8 Compare February 15, 2017 15:52

Nikolay Kim added 9 commits February 15, 2017 08:00

initial implementation

adf00e7

cleanup various code paths

0824cff

clear raw_path usage

cf9a088

use url parsing functionality from http-parser

344a7cd

fix ws handshake tests

0c553ce

added environ variable for disabling gc.collect and disabling uvloop …

67cb65e

…during tests

unify http message parser

6301d6b

use new versin of yarl

48f4e66

added --fast tests option

2a42377

fafhrd91 force-pushed the parser branch from bf582a8 to 2a42377 Compare February 15, 2017 16:01

Nikolay Kim added 4 commits February 15, 2017 08:53

drop AIOHTTP_TESTS_FAST env var

f21e997

allow uvloop configuration with pytests

f087856

do not alter header case; drop separate cparser tests

6fc71b3

disable access log if gunicron is not configured

884890a

fafhrd91 force-pushed the parser branch 2 times, most recently from ba056e3 to ea68872 Compare February 15, 2017 21:29

fafhrd91 force-pushed the parser branch 4 times, most recently from 1f2d965 to a92549e Compare February 15, 2017 23:16

nhumrich approved these changes Feb 15, 2017

View reviewed changes

Nikolay Kim added 6 commits February 15, 2017 16:18

better checks in test

06bad90

added --enable-loop-debug pytest option

f39845d

fix chunk size deprecation warning

a0363a2

fix CONNECT support in _parser

a156f3d

do not call set_debug if it is not enabled

44cde0b

cleanups

e49b59b

fafhrd91 force-pushed the parser branch from a92549e to e49b59b Compare February 16, 2017 00:28

fafhrd91 merged commit e49b59b into master Feb 16, 2017

fafhrd91 deleted the parser branch February 16, 2017 00:54

lock bot added the outdated label Oct 29, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

http parser implementation based on http-parser #1626

http parser implementation based on http-parser #1626

fafhrd91 commented Feb 14, 2017

kxepal Feb 14, 2017

kxepal Feb 14, 2017

kxepal Feb 14, 2017

fafhrd91 Feb 14, 2017

kxepal Feb 14, 2017

fafhrd91 Feb 14, 2017

kxepal Feb 14, 2017

fafhrd91 Feb 14, 2017

kxepal Feb 14, 2017

fafhrd91 Feb 14, 2017

kxepal Feb 14, 2017

fafhrd91 Feb 14, 2017

kxepal Feb 14, 2017

kxepal Feb 14, 2017

fafhrd91 Feb 14, 2017

kxepal Feb 14, 2017

fafhrd91 Feb 14, 2017

codecov-io commented Feb 15, 2017 •

edited

Loading

fafhrd91 commented Feb 15, 2017

fafhrd91 commented Feb 15, 2017

fafhrd91 commented Feb 15, 2017 •

edited

Loading

fafhrd91 commented Feb 15, 2017

fafhrd91 commented Feb 15, 2017

samuelcolvin commented Feb 15, 2017

samuelcolvin Feb 15, 2017 •

edited

Loading

fafhrd91 Feb 15, 2017

samuelcolvin Feb 15, 2017

fafhrd91 commented Feb 15, 2017

nhumrich left a comment

fafhrd91 commented Feb 16, 2017

lock bot commented Oct 29, 2019

http parser implementation based on http-parser #1626

http parser implementation based on http-parser #1626

Conversation

fafhrd91 commented Feb 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Feb 15, 2017 • edited Loading

Codecov Report

fafhrd91 commented Feb 15, 2017

fafhrd91 commented Feb 15, 2017

fafhrd91 commented Feb 15, 2017 • edited Loading

fafhrd91 commented Feb 15, 2017

fafhrd91 commented Feb 15, 2017

samuelcolvin commented Feb 15, 2017

samuelcolvin Feb 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fafhrd91 commented Feb 15, 2017

nhumrich left a comment

Choose a reason for hiding this comment

fafhrd91 commented Feb 16, 2017

lock bot commented Oct 29, 2019

codecov-io commented Feb 15, 2017 •

edited

Loading

fafhrd91 commented Feb 15, 2017 •

edited

Loading

samuelcolvin Feb 15, 2017 •

edited

Loading