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

Performance regression v0.2.13 #320

Closed
AzHicham opened this issue Feb 11, 2025 · 3 comments · Fixed by #324
Closed

Performance regression v0.2.13 #320

AzHicham opened this issue Feb 11, 2025 · 3 comments · Fixed by #324

Comments

@AzHicham
Copy link

Hello,

OS: Debian 12 & Ubuntu 24.04
Cargo: 1.81.0

We noticed something really weird in a app we are developing in my team.
The app we are developing is a simple backend that provide a HTTP interface (actix) and do some mongodb request to query data.
We do have some benchmark to monitor the time to process different kind of query. Benchmark consists mainly on doing a lot of http request with the library reqwest (that depends on native-tls)

We noticed on a renovate PR (cargo update deps) that some benchmarks were diverging by a factor 2.
We did reproduce the factor 2 locally between the develop branch and the PR branch.

After some analysis we found out that using our develop branch (native-tls 0.2.12) then updating only native-tls with this command: cargo update --package native-tls --precise 0.2.13 results in the benchmark degradation of a factor 2.

We did check the cargo.lock and the only deps that depends on native-tls is reqwest.

Is it really possible that our issue is due to native-tls ?

Thank a lot

@sfackler
Copy link
Owner

If you're creating and throwing away reqwest clients #316 could have impacted that by searching for trust stores every time but I wouldn't think that would take a significant amount of time.

@AzHicham
Copy link
Author

AzHicham commented Feb 12, 2025

Hello,

Thank you for your answer !!!
Indeed this is the case thanks for spotting that (without code!!).
Still I'm surprised a small bump of native-tls will incur such a big performance regression in my case, but to be fair in my benchmark the retrieval of data is really fast meaning that the cost of creating a new reqwest client is more visible.

I don't know if you can do something to improve that if not I will close this issue.

EDIT: Some data about our benchmarks (cargo bench release mode):
bench1 with reqwest with native-tls 0.2.12 -> 6ms/iter
bench1 with reqwest with native-tls 0.2.13 -> 12ms/iter
bench1 with reqwest with rust-tls -> 2ms/iter

Thank you!!

@sfackler
Copy link
Owner

We can shift this lookup into a Once so it doesn't run on every client creation: https://github.com/sfackler/rust-native-tls/blob/master/src/imp/openssl.rs#L271. That should more closely match the behavior before the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants