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 option to HttpConnector to enable or disable HttpInfo #2833

Open
ho-229 opened this issue May 15, 2022 · 4 comments · May be fixed by #2837
Open

Add option to HttpConnector to enable or disable HttpInfo #2833

ho-229 opened this issue May 15, 2022 · 4 comments · May be fixed by #2837
Labels
A-client Area: client. C-feature Category: feature. This is adding a new feature. E-easy Effort: easy. A task that would be a great starting point for a new contributor.

Comments

@ho-229
Copy link

ho-229 commented May 15, 2022

Is your feature request related to a problem? Please describe.
By default hyper will try to get the HttpInfo for each Response even if they are not needed in most cases. The bottom line is that getpeername and getsockname are actually very slow and they can become a performance bottleneck when making a lot of requests

Describe the solution you'd like
Just provide an API like client::Builder::set_host
@ho-229 ho-229 added the C-feature Category: feature. This is adding a new feature. label May 15, 2022
@seanmonstar
Copy link
Member

It could be an option on HttpConnector.

@seanmonstar seanmonstar added A-client Area: client. E-easy Effort: easy. A task that would be a great starting point for a new contributor. labels May 15, 2022
@seanmonstar seanmonstar changed the title How to turn off getting HttpInfo for each Response Add option to HttpConnector to enable or disable HttpInfo May 17, 2022
@mnpw
Copy link

mnpw commented May 19, 2022

hey @seanmonstar, I would like to take this up. Please assign this to me 😄

@mnpw mnpw linked a pull request May 20, 2022 that will close this issue
Open
@RajivTS
Copy link
Contributor

RajivTS commented Sep 3, 2022

@seanmonstar If no one is currently working on this and if this is still needed, I would like to take it up. Also, I think the relevant code is no longer part of the hyper crate and has instead moved into hyper-util.

I noticed your comment on the linked PR of adding the configuration without introducing a new type. However, based on the below snippet, the Connection trait is implemented for tokio::TcpStream which has no access to Config where the new flag will be stored. Do you have any pointers on getting access to this flag without an outer wrapper object?

https://github.com/hyperium/hyper-util/blob/85aade41a080fc4b40a9a53c818680feed14f138/src/client/connect/http.rs#L342-L366

@ho-229
Copy link
Author

ho-229 commented Jun 11, 2023

I think the calls to peer_addr() and local_addr() should be lazy-loaded, then the connection object would cache the results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-client Area: client. C-feature Category: feature. This is adding a new feature. E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(client): add option to HttpConnector to enable or disable mnpw/hyper
4 participants
@seanmonstar @mnpw @ho-229 @RajivTS