Skip to content

Conversation

knsd
Copy link
Contributor

@knsd knsd commented Jul 3, 2016

So it's possible to preallocate things in Deserialize implementations.

@coveralls
Copy link

coveralls commented Jul 3, 2016

Coverage Status

Coverage increased (+0.02%) to 73.927% when pulling 31e7727 on knsd:master into 2fe3ed9 on 3Hren:master.

@3Hren
Copy link
Owner

3Hren commented Jul 3, 2016

Hi!

Looks good!

This is some kind of new feature of serde I suppose? Can you make a simple benchmark to see performance benefit before and after this PR?

@knsd
Copy link
Contributor Author

knsd commented Jul 3, 2016

Hi!

Looks like it a quite old feature from serde 0.2.0 :)

Of course, for example for this benchmark:

#![feature(test)]

extern crate test;

extern crate serde;
extern crate rmp_serde;

use std::collections::{HashMap};
use serde::{Serialize, Deserialize};

use test::{Bencher};

#[bench]
fn bench_map_1000(bencher: &mut Bencher) {
    bench_map(bencher, 1000)
}

#[bench]
fn bench_map_5000(bencher: &mut Bencher) {
    bench_map(bencher, 5000)
}

#[bench]
fn bench_map_10000(bencher: &mut Bencher) {
    bench_map(bencher, 10000)
}

fn bench_map(bencher: &mut Bencher, size: usize) {
    let mut map: HashMap<usize, usize> = HashMap::new();
    for item in 0..size {
        map.insert(item, item);
    }

    let mut buf = Vec::new();
    {
        let mut serializer = rmp_serde::Serializer::new(&mut buf);
        map.serialize(&mut serializer).unwrap();
    }
    bencher.iter(|| {
        <HashMap<usize, usize>>::deserialize(&mut rmp_serde::Deserializer::new(&buf[..])).unwrap();
    })
}

Without size hint:

test bench_map_1000  ... bench:     112,692 ns/iter (+/- 3,586)
test bench_map_5000  ... bench:   3,679,561 ns/iter (+/- 775,236)
test bench_map_10000 ... bench:  13,976,963 ns/iter (+/- 1,625,630)

And with size hint:

test bench_map_1000  ... bench:      58,376 ns/iter (+/- 9,696)
test bench_map_5000  ... bench:     297,913 ns/iter (+/- 21,654)
test bench_map_10000 ... bench:     602,640 ns/iter (+/- 119,802)

Should I include this benchmark into the repository?

@3Hren 3Hren merged commit c742c5b into 3Hren:master Jul 3, 2016
@3Hren
Copy link
Owner

3Hren commented Jul 3, 2016

Great results!

Should I include this benchmark into the repository?

I think it's not necessary.

Thanks!

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