Stack allocated PeerId #1874
Merged
Stack allocated PeerId #1874
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
romanb
reviewed
Dec 7, 2020
There was a problem hiding this comment.
The direction looks good to me. Contrary to the PR description, I don't think this closes #1810, as we use bytes in a few other places throughout libp2p. It is also better not to address #1852 here, as that is unrelated. The build should be fixed separately either by upgrading rustls (#1852) or by async-tls-0.10.2 being yanked. See async-rs/async-tls#42.
dvc94ch
force-pushed
the
peer-id-stack
branch
from
69bd277
to
e7750b4
Compare
romanb
approved these changes
Dec 7, 2020
There was a problem hiding this comment.
Looks good to me. I'd just like to leave this PR open for a bit longer as the next libp2p release is just to update libp2p-mdns and libp2p-request-response, and this PR will essentially trigger new releases of most crates. It may also give more people the chance to comment here. If there are no objections we can probably merge and release this next week.
mxinden
reviewed
Dec 8, 2020
There was a problem hiding this comment.
I am in favor of this patch. Thanks @dvc94ch.
Given that with this pull request PeerId implements Copy I would suggest removing all unnecessary clone() calls. What do you think?
I have run the benchmarks added in #1875 plus the additional sort_vec benchmark. See the result below.
$ git checkout master
$ cargo bench --bench peer_id -- --save-baseline master
$ git checkout peer-id-stack
$ cargo bench --bench peer_id -- --save-baseline peer-id-stack
$ critcmp master peer-id-stack
group master peer-id-stack
----- ------ -------------
clone 1.00 15.6±0.27ns ? B/sec 3.07 47.8±1.08ns ? B/sec
from_bytes 1.59 179.1±5.96ns ? B/sec 1.00 112.3±6.03ns ? B/sec
sort_vec 1.07 7.5±0.27µs ? B/sec 1.00 7.0±0.31µs ? B/sec
|
I plan to merge this later today. @mxinden Would a follow-up getting-started ticket for removing redundant |
added 2 commits
December 15, 2020 14:20
AgeManning
added a commit
to sigp/rust-libp2p
that referenced
this pull request
Dec 16, 2020
* Update rustls requirement from 0.18.0 to 0.19.0 (libp2p#1852) Updates the requirements on [rustls](https://github.com/ctz/rustls) to permit the latest version. - [Release notes](https://github.com/ctz/rustls/releases) - [Changelog](https://github.com/ctz/rustls/blob/main/OLDCHANGES.md) - [Commits](rustls/rustls@v/0.18.0...v/0.19.0) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Prepare libp2p-websocket-0.26.1 * Update top-level libp2p-websocket patch version. * [request-response] Refine success & error reporting for inbound requests. (libp2p#1867) * Refine error reporting for inbound request handling. At the moment one can neither get confirmation when a response has been sent on the underlying transport, nor is one aware of response omissions. The latter was originally intended as a feature for support of one-way protocols, which seems like a bad idea in hindsight. The lack of notification for sent responses may prohibit implementation of some request-response protocols that need to ensure a happens-before relation between sending a response and a subsequent request, besides uses for collecting statistics. Even with these changes, there is no active notification for failed inbound requests as a result of connections unexpectedly closing, as is the case for outbound requests. Instead, for pending inbound requests this scenario can be identified if necessary by the absense of both `InboundFailure` and `ResponseSent` events for a particular previously received request. Interest in this situation is not expected to be common and would otherwise require explicitly tracking all inbound requests in the `RequestResponse` behaviour, which would be a pity. `RequestResponse::send_response` now also synchronously returns an error if the inbound upgrade handling the request has been aborted, due to timeout or closing of the connection, giving more options for graceful error handling for inbound requests. As an aside, the `Throttled` wrapper now no longer emits inbound or outbound error events occurring in the context of sending credit requests or responses. This is in addition to not emitting `ResponseSent` events for ACK responses of credit grants. * Update protocols/request-response/src/lib.rs Co-authored-by: Max Inden <mail@max-inden.de> * Address some minor clippy warnings. (libp2p#1868) * Track pending credit request IDs. In order to avoid emitting events relating to credit grants or acks on the public API. The public API should only emit events relating to the actual requests and responses sent by client code. * Small cleanup * Cleanup * Update versions and changelogs. * Unreleased Co-authored-by: Max Inden <mail@max-inden.de> * core/benches: Add rudimentary benchmark for PeerId::from_bytes and clone (libp2p#1875) * core: Add rudimentary benchmark for PeerId::from_bytes and clone * .github/workflow: Include benchmarks To ensure changes through pull requests won't make benchmarks fail to compile or run, run them as part of CI. * [mdns] Split response packets if necessary. (libp2p#1877) * [mdns] Split response packets. Prevent MDNS response packets becoming too large by creating multi-packet responses. Also skip addresses that don't fit into a TXT record or contain invalid characters. * Update protocols/mdns/src/dns.rs Co-authored-by: Max Inden <mail@max-inden.de> * Refactor response packet construction. * Update mdns changelog. Co-authored-by: Max Inden <mail@max-inden.de> * core/benches: Add PeerId sort_vec benchmark (libp2p#1878) * Prepare v0.32 (libp2p#1879) * [websocket] Update minimum async-tls patch version. (libp2p#1881) * Update minimum async-tls patch version. After the upgrade to rustls 0.19, this is the minimum version required to build. * Prepare libp2p patch. * Update async-tls requirement from 0.10.2 to 0.11.0 (libp2p#1884) * Update async-tls requirement from 0.10.2 to 0.11.0 Updates the requirements on [async-tls](https://github.com/async-std/async-tls) to permit the latest version. - [Release notes](https://github.com/async-std/async-tls/releases) - [Commits](async-rs/async-tls@v0.10.2...v0.11.0) Signed-off-by: dependabot[bot] <support@github.com> * *: Prepare release Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Max Inden <mail@max-inden.de> * rename generic types + add default types + update documentation links * fix broken intra links * fix tests * type annotation in example * fix doc example * Add compression to gossipsub * Add snappy compression to the smoke tests * Stack allocated PeerId (libp2p#1874) * Stack allocate PeerId. * Update stuff. * Upgrade rusttls to fix build. * Remove unnecessary manual implementations. * Remove PeerId::into_bytes. * Remove bytes dependency. * Perform some cleanup. * Use Into<kbucket::Key<K>>. * Update versions and changelogs. * Fix PR link. * Fix benchmarks. Co-authored-by: Roman S. Borschel <roman@parity.io> * Remove copy and adjust validation ordering Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Roman S. Borschel <roman@parity.io> Co-authored-by: Roman Borschel <romanb@users.noreply.github.com> Co-authored-by: Max Inden <mail@max-inden.de> Co-authored-by: blacktemplar <blacktemplar@a1.net> Co-authored-by: David Craven <david@craven.ch>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allocates PeerId's on the stack, incidentally also making clippy happy. For more info see #1865. Superceeds #1865.