Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add flags to allow multiple spaces in request and status lines #110

Merged
merged 1 commit into from
Mar 31, 2022
Merged

Add flags to allow multiple spaces in request and status lines #110

merged 1 commit into from
Mar 31, 2022

Commits on Mar 29, 2022

  1. Add flags to allow multiple spaces in request and status lines 

    These new flags set whether multiple spaces are allowed as delimiters in request lines and response
status lines.

The latest version of the HTTP/1.1 spec ([request lines][spec-req], [response status
lines][spec-resp]) allows implementations to parse multiple whitespace characters in place of the
`SP` delimiter in the response status line, including:

> SP, HTAB, VT (%x0B), FF (%x0C), or bare CR

This option relaxes the parsers to allow for multiple spaces, but does *not* allow the delimiters to
contain the other mentioned whitespace characters. We'd rather wait for someone to have a concrete
use case before deciding to support that, as allowing chars like `\r` raises serious security
questions as described by the spec.

Happily this seems to be a performance _improvement_ rather than a regression even when the new
flags are disabled (results vs 6f6ff10):

```
req/req                 time:   [292.13 ns 295.81 ns 301.23 ns]
                        thrpt:  [2.2261 GiB/s 2.2668 GiB/s 2.2954 GiB/s]
                 change:
                        time:   [-13.390% -12.468% -11.159%] (p = 0.00 < 0.05)
                        thrpt:  [+12.561% +14.244% +15.460%]
                        Performance has improved.

req_short/req_short     time:   [62.834 ns 62.992 ns 63.188 ns]
                        thrpt:  [1.0023 GiB/s 1.0054 GiB/s 1.0079 GiB/s]
                 change:
                        time:   [-11.112% -9.9009% -8.9416%] (p = 0.00 < 0.05)
                        thrpt:  [+9.8196% +10.989% +12.501%]
                        Performance has improved.

resp/resp               time:   [303.51 ns 304.23 ns 305.34 ns]
                        thrpt:  [2.1320 GiB/s 2.1398 GiB/s 2.1449 GiB/s]
                 change:
                        time:   [-12.059% -11.512% -11.016%] (p = 0.00 < 0.05)
                        thrpt:  [+12.379% +13.010% +13.713%]
                        Performance has improved.

resp_short/resp_short   time:   [67.521 ns 67.686 ns 67.929 ns]
                        thrpt:  [1.2476 GiB/s 1.2521 GiB/s 1.2552 GiB/s]
                 change:
                        time:   [-9.1562% -8.7366% -8.3331%] (p = 0.00 < 0.05)
                        thrpt:  [+9.0906% +9.5730% +10.079%]
                        Performance has improved.
```

I haven't thrown it into godbolt or anything yet, but I suspect something about this change triggers
a different inlining behavior compared to the baseline.

[spec-req]: https://httpwg.org/http-core/draft-ietf-httpbis-messaging-latest.html#rfc.section.3.p.3
[spec-resp]: https://httpwg.org/http-core/draft-ietf-httpbis-messaging-latest.html#rfc.section.4.p.3
    @acfoltzer
    acfoltzer committed Mar 29, 2022
    Configuration menu
    Copy the full SHA
    36871cc View commit details
    Browse the repository at this point in the history