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

core/benches: Add rudimentary benchmark for PeerId::from_bytes and clone #1875

Merged
merged 3 commits into from
Dec 7, 2020

Conversation

mxinden
Copy link
Member

@mxinden mxinden commented Dec 7, 2020

I hope these somewhat simple benchmarks can back up discussions in #1874 and #1865 with data.

$ cd core/
$ cargo bench
[...]
from_bytes              time:   [187.15 ns 187.88 ns 188.79 ns]                                                       
                        change: [-11.163% -10.359% -8.9828%] (p = 0.00 < 0.05)                                        
                        Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
  1 (1.00%) low mild
  5 (5.00%) high mild
  3 (3.00%) high severe

clone                   time:   [14.811 ns 14.868 ns 14.940 ns]                                                       
                        change: [-14.744% -14.274% -13.812%] (p = 0.00 < 0.05)                                        
                        Performance has improved.
Found 20 outliers among 100 measurements (20.00%)
  1 (1.00%) low severe
  1 (1.00%) low mild
  6 (6.00%) high mild
  12 (12.00%) high severe


$ cat /proc/cpuinfo | grep "model name" | head -1
model name      : Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz

In addition this pull request adds a CI step to ensure benchmarks continue to compile and run.

To ensure changes through pull requests won't make benchmarks fail to
compile or run, run them as part of CI.
@dvc94ch
Copy link
Contributor

dvc94ch commented Dec 7, 2020

Yes, clone is slower as it needs to copy about double the amount of bytes compared to the Arc, but faster if you put Arc<PeerId>, compared to just using Bytes (by about 14% may vary from run to run slightly).

However you can easily concoct a benchmark depending on what it is you want to prove. I concocted one here:

fn sort_vec(c: &mut Criterion) {
    let mut peer_ids = Vec::with_capacity(100);
    for _ in 0..peer_ids.capacity() {
        peer_ids.push(identity::Keypair::generate_ed25519()
            .public()
            .into_peer_id());
    }

    c.bench_function("clone", |b| {
        b.iter(|| {
            let mut peer_ids = peer_ids.clone();
            peer_ids.sort_unstable();
            black_box(peer_ids);
        })
    });
}

@dvc94ch
Copy link
Contributor

dvc94ch commented Dec 7, 2020

Let's merge this then?

@mxinden mxinden merged commit e665a81 into libp2p:master Dec 7, 2020
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants