python-hyper / h11

A pure-Python, bring-your-own-I/O implementation of HTTP/1.1
https://h11.readthedocs.io/
MIT License
490 stars 62 forks source link

Preserve header casing. Take two. 🎬 #104

Closed tomchristie closed 4 years ago

tomchristie commented 4 years ago

Closes #31

Based on #103, focusing on minimising benchmark regressions.

Three benchmark runs here against each case.

Running against current master...

7330.1 requests/sec 7357.9 requests/sec 7363.4 requests/sec 7310.5 requests/sec 7363.2 requests/sec 7354.3 requests/sec 7363.3 requests/sec

7321.4 requests/sec 7355.2 requests/sec 7353.7 requests/sec 7375.7 requests/sec 7352.6 requests/sec 7381.0 requests/sec 7410.0 requests/sec

7234.4 requests/sec 7274.0 requests/sec 7231.8 requests/sec 7272.5 requests/sec 7263.7 requests/sec 7268.9 requests/sec 7273.7 requests/sec

Running against this PR...

7022.7 requests/sec 7041.2 requests/sec 7052.2 requests/sec 7038.3 requests/sec 7038.5 requests/sec 7042.8 requests/sec 7053.2 requests/sec

7129.9 requests/sec 7145.6 requests/sec 7152.4 requests/sec 7158.3 requests/sec 7154.1 requests/sec 7116.6 requests/sec 7146.2 requests/sec

7080.6 requests/sec 7077.9 requests/sec 7100.3 requests/sec 7051.9 requests/sec 7091.7 requests/sec 7072.6 requests/sec 7056.3 requests/sec

codecov[bot] commented 4 years ago

Codecov Report

Merging #104 into master will increase coverage by 0.01%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #104      +/-   ##
==========================================
+ Coverage   99.14%   99.15%   +0.01%     
==========================================
  Files          21       21              
  Lines         937      952      +15     
  Branches      173      175       +2     
==========================================
+ Hits          929      944      +15     
  Misses          7        7              
  Partials        1        1              
Impacted Files Coverage Δ
h11/tests/test_connection.py 100.00% <ø> (ø)
h11/tests/test_events.py 100.00% <ø> (ø)
h11/tests/test_headers.py 100.00% <ø> (ø)
h11/_connection.py 100.00% <100.00%> (ø)
h11/_headers.py 100.00% <100.00%> (ø)
h11/_readers.py 100.00% <100.00%> (ø)
h11/_writers.py 88.73% <100.00%> (+0.16%) :arrow_up:
h11/tests/test_io.py 100.00% <100.00%> (ø)
h11/tests/helpers.py 100.00% <0.00%> (ø)
... and 1 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 1540843...5f77670. Read the comment docs.

tomchristie commented 4 years ago

Okay, I think this is about the best we can do here. I've updated the docs to include the one extra point of API that's now exposed.

Unavoidably it does benchmark marginally slower that the current master branch.

Before...

7211.1 requests/sec
7300.6 requests/sec
7298.3 requests/sec
7271.7 requests/sec
7342.2 requests/sec
7310.1 requests/sec
7334.3 requests/sec

After...

7025.6 requests/sec
7094.9 requests/sec
7095.7 requests/sec
7094.9 requests/sec
7115.5 requests/sec
7053.5 requests/sec
7104.2 requests/sec

Averaging to: 7295 requests/sec -> 7083 requests/sec

What do we think about this folks? @njsmith, @pgjones?

(From my POV I'd really like to get this resolved - we're currently treating it as one of our HTTPX 1.0 blockers.)

🙏💚😇

pquentin commented 4 years ago

@StephenBrown2 @pgjones Would one of you be able to review this fine work? I'm not qualified (and not part of python-hyper anyways) and @njsmith isn't currently available for OSS reviews (even on Trio)

pgjones commented 4 years ago

Minded to merge - happy to be corrected - will merge and release over the weekend if not.

tomchristie commented 4 years ago

@pgjones Fantastic - pretty excited to get this one resolved. 🤗 Presumably we'll bump to 0.11 for this release?