fix: Share reqwest HTTP clients

[tmathern]

Changes in this pull request

• Share reqwest clients (avoid client recreation and related memory consumption).

Checklist

• This PR represents a single feature, fix, or change.
• All applicable changes have been documented.
• Any TO DO items (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.

[codspeed-hq]

Merging this PR will not alter performance

✅ 30 untouched benchmarks
⏩ 64 skipped benchmarks¹

---

_{Comparing mathern/pool-reqwest (98ddb7d) with main (350e138)}

Footnotes

1. 64 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[ok-nick]

IMO I prefer to not by default store global state like this. I'd rather the caller of the library chooses to store the HTTP client as global state if they'd like. I'm not sure if it has meaningful impact on memory/perf. I defer to @gpeacock on the final decision.

[tmathern]

Normally, the client itself is stateless (it is not state that changes behavior). You are reusing HTTP plumbing, so you do not recreate it, with all the underlying plumbing and needed memory, on the flight, on each call you need it.

A new reqwest client goes through reqwest::new, which spawns a tokio runtime current thread on this thread, with its conenction pool.

Now imagine this in a server under load. N concurrent requests, each on its own worker thread, each going through this (and a few fetches), rebuilding on each call. Without sharing, every worker thread spawns its own runtime thread, and undergoes init again. (I don't think it impacts perf yet, but memory: threads do not come for free).

The caller could, choose, yes, but we also should have our default part reuse this for better defaults. The two statics only govern the default built-in resolver, where the library owns the lifecycle, not the caller. And the OnceLock is lazy, so if the network path is never touched, nothing is allocated.

[gpeacock]

I think this is ok, but I'm nervous about it after the problems we had with settings that were held in a thread local