Open vdods opened 1 year ago
I agree that the current implementation is not great, I would actually put the client in DIDWeb
Ok -- perhaps there can be two methods: (1) an optional initialize method, in case you want to supply your own reqwest::Client ahead of time, and (2) a getter for the reqwest::Client, which creates the thing if it doesn't exist already. That way, we still get the lazy behavior, but it can be made non-redundant if you care to.
If that sounds good, I'll make a PR.
I don't know if this is the right place to add laziness, I think it's cleaner to have a fn new(client: Option<Client>) -> Self
which either use the provided client or create a new one. Then depending on the use case or environment the DIDWeb
can be constructed lazily, either directly or through DIDMethods
(which would require changes)
In optimizing my verification codepaths, I've noticed that a major bottleneck was the creation of a new
reqwest::Client
for each did:web DID doc resolution. It's mostly because loading up all the TLS certs is a pretty involved operation. Thereqwest
crate documentation ( https://docs.rs/reqwest/latest/reqwest/struct.Client.html ) saysI fixed this issue in my own fork by creating a
reqwest::Client
singleton vialazy_static
(example below), and then cloning that to make HTTP requests. In my particular verification-heavy codepath, this led to reducing the run time by over 60%!This
lazy_static
approach is the easiest and laziest solution. However, if there are other crates that also do similar and have their ownlazy_static
reqwest::Client
singletons, having a redundant one be less than ideal. I don't know a good approach for how to handle that. Maybe there could be an initialization function that is optionally called to set thereqwest::Client
singleton to be a clone of the one supplied to that initialization function.Anyway, because of how severe a bottleneck this is, I'd suggest we address this somehow in the did-web crate.
I put the following in
did-web/src/lib.rs
: