http parser implementation based on http-parser by fafhrd91 · Pull Request #1626 · aio-libs/aiohttp

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

+            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

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


Should we crash if version will be something weird?

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

+            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

+                 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

+                   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

+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

+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

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


Debug print.

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

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


elif data?

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



-@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

    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

Uh oh!

Conversation

fafhrd91 commented Feb 14, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Feb 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fafhrd91 commented Feb 15, 2017

Uh oh!

fafhrd91 commented Feb 15, 2017

Uh oh!

fafhrd91 commented Feb 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fafhrd91 commented Feb 15, 2017

Uh oh!

fafhrd91 commented Feb 15, 2017

Uh oh!

samuelcolvin commented Feb 15, 2017

Uh oh!

samuelcolvin Feb 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fafhrd91 commented Feb 15, 2017

Uh oh!

nhumrich left a comment

Choose a reason for hiding this comment

Uh oh!

fafhrd91 commented Feb 16, 2017

Uh oh!

lock Bot commented Oct 29, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-io commented Feb 15, 2017 •

edited

Loading

fafhrd91 commented Feb 15, 2017 •

edited

Loading

samuelcolvin Feb 15, 2017 •

edited

Loading