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

clock-bound-c: use array instead of vec for performance #3

Merged
merged 1 commit into from Dec 23, 2021

Conversation

siddontang
Copy link
Contributor

@siddontang siddontang commented Dec 14, 2021

Issue #, if available:

None

Description of changes:

I found that you here use vector to get timestamps, IMO, these functions are highly frequently called, so it is better to use array instead of vector.

I wrote a simple benchmark below:

#![feature(test)]
extern crate test;
use test::{black_box, Bencher};
#[bench]
fn bench_vec(b: &mut Bencher) {
    b.iter(|| {
        for _ in 0..16 {
            let mut request: Vec<u8> = Vec::new();
            // Inner closure, the actual test
            for i in 0..4 {
                request.push(i);
            }
            black_box(request);
        }
    });
}
#[bench]
fn bench_vec_with_capacity(b: &mut Bencher) {
    b.iter(|| {
        for _ in 0..16 {
            let mut request: Vec<u8> = Vec::with_capacity(4);
            // Inner closure, the actual test
            for i in 0..4 {
                request.push(i);
            }
            black_box(request);
        }
    });
}
#[bench]
fn bench_array(b: &mut Bencher) {
    b.iter(|| {
        for _ in 0..16 {
            let mut request: [u8; 4] = [0; 4];
            // Inner closure, the actual test
            for i in 0..4 {
                request[i] = i as u8;
            }
            black_box(request);
        }
    });
}

The benchmark result is:

test bench_array            ... bench:           4 ns/iter (+/- 0)
test bench_vec               ... bench:       1,014 ns/iter (+/- 143)
test bench_vec_with_capacity ... bench:         977 ns/iter (+/- 70)

As you can see, using array gains a better performance.

Because this repo doesn't have any unit tests, I have to test it by myself, seem it can work well, like:

cargo run --example now /run/clockboundd/clockboundd.sock
The UTC timestamp 2021-12-14 14:24:20.048961000 has the following error bounds.
In nanoseconds since the Unix epoch: (1639491860040783810,1639491860057138190)
In UTC in date/time format: (2021-12-14 14:24:20.040783810, 2021-12-14 14:24:20.057138190)

cargo run --example before /run/clockboundd/clockboundd.sock
1639491872937419000 nanoseconds since the Unix Epoch is not before the current time's error bounds.
Waiting 1 second...
1639491872937419000 nanoseconds since the Unix Epoch is before the current time's error bounds.

cargo run --example after /run/clockboundd/clockboundd.sock
1639491883489757000 nanoseconds since the Unix Epoch is after the current time's error bounds.
Waiting 2 seconds...
1639491883489757000 nanoseconds since the Unix Epoch is not after the current time's error bounds.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@bestgopher
Copy link

bestgopher commented Dec 14, 2021

Hi, maybe you mean array instead of vector.

@siddontang siddontang changed the title clock-bound-c: use slice instead of vec for performance clock-bound-c: use array instead of vec for performance Dec 15, 2021
@siddontang
Copy link
Contributor Author

yes, array. sometimes I use slice by myself. I have changed it.

@siddontang
Copy link
Contributor Author

PTLA @jacoblwisniewski

@jacoblwisniewski jacoblwisniewski merged commit dc7754f into aws:main Dec 23, 2021
@jacoblwisniewski
Copy link
Contributor

Tested it locally myself and confirmed it's working. Thank you for the optimization!

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.

None yet

3 participants