build(deps): Update lru to 0.8.0
Description
This is a replacement for https://github.com/libp2p/rust-libp2p/pull/2889. With lru version 0.8.0, we can no longer pass a capacity of 0 to it which is why we write our own wrapper that can disable the cache internally.
Links to any relevant issues
- https://github.com/libp2p/rust-libp2p/pull/2889
Open Questions
Change checklist
- [x] I have performed a self-review of my own code
- [x] I have made corresponding changes to the documentation
- ~[ ] I have added tests that prove my fix is effective or that my feature works~
- [x] A changelog entry has been made in the appropriate crates
How would one disable the caching logic entirely with the
NonZeroUsize? I could imagine a use-case where someone does not wantlibp2p-identifyto be involved in the address selection, but does want anotherNetworkBehaviourto contribute to the list of addresses.
Haven't thought of that. Can the caching actually be harmful? We try all sorts of addresses for a peer when we dial them, a few more or less don't seem to be a problem?
We could make the cache optional or wrap it in our own type so that it can still act as a no-op in case it is disabled.
Regarding a default value of 10, what is the rational here? I would say most of our default values are optimized for laptops and servers. With a maximum identify payload of 4096 byte I would be fine with caching >= 100 peers, which would still keep the upper bound of the cache below a mega byte. What do you think?
No rationale other than just picking a number but you are right, it will probably be a lot more useful if we up the cache size. Can also add a comment that explains that even with 100 peers, this should be well under a megabytes of memory usage.
Would you like it to be possible for the cache to be disabled?
Would you like it to be possible for the cache to be disabled?
I would be in favor of it. Though more as an intuition and the expectation that there are some crazy implementations of Networkbehaviour out there.
@mxinden I ended up not doing to the change to the default cache size in here so that this is a clean dependency update without a functional change. This allows us to consider the cache size change individually.
I opened an issue here: https://github.com/libp2p/rust-libp2p/issues/2933