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.

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

[feature-request] Replace std::unordered_map with flat map for http headers #142

Closed
itrofimow opened this issue Oct 15, 2022 · 2 comments
Closed
Labels
big A big feature help wanted We would appreciate PR

Comments

@itrofimow
Copy link
Contributor

itrofimow commented Oct 15, 2022

Looking at flamegraphs from #141 (comment) one can see that unordered_map takes ~10-11% off total time, with most of it coming from http internals (and some from TimeStorage).

There is an idea that it could very well be replaced with flat map, that would fall back to unordered_map after some threshold, and chances are it would perform better - not only std::unordered_map is pretty slow and linear search might perform better for small number of keys, but there also is that HashDOS-preventing hash, which ain't the fastest. Also my experience tells me that it's not that common to have, say, 50+ headers in http.

What is also promising is that this abstract HttpHeadersMap could be implemented via on stack buffer containing data in http-format right away! - just add some array of string_views which indexes into that buffer. Not sure whether that would work for requests, but should be possible for responses, and for responses we could just pass it's content in Socket::SendAll as is

@apolukhin apolukhin added the help wanted We would appreciate PR label Oct 25, 2022
@apolukhin
Copy link
Member

Another idea is to move special headers (like Content-Type, Date, Content-Length) into a separate fields inside a structure to totally avoid search for them

@apolukhin apolukhin added help wanted We would appreciate PR big A big feature and removed help wanted We would appreciate PR labels Oct 25, 2022
@itrofimow
Copy link
Contributor Author

Custom map for http headers is merged in c4b1d90

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
big A big feature help wanted We would appreciate PR
Projects
None yet
Development

No branches or pull requests

2 participants