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

Reduce duplicative instantiation of logic in SeqAccess and MapAccess #1205

Merged
merged 1 commit into from
Oct 19, 2024

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Oct 19, 2024

Measured in https://github.com/serde-rs/json-benchmark using cargo llvm-lines --bin json-benchmark --no-default-features --features parse-struct,lib-serde,all-files --filter 'next_element_seed|next_key_seed', this PR reduces LLVM lines by 53% (compared to 37% for #1102). This is the part of the logic that needs to be instantiated separately per seq/map type, as opposed to once per Read type.

Across the whole json-benchmark program, this PR decreases total LLVM lines by 8.8% (compared to 5.6% for #1102) and decreases function instantiations by 3.4%.

Benchmarked using cargo build --release --bin json-benchmark --no-default-features --features parse-struct,lib-serde,all-files, there is no statistically significant performance difference at runtime. (But I notice some opportunities to improve performance of the code in these functions. Leaving for a future PR.)

Also benchmarked using the following microbenchmark that targets this code specifically, which likewise shows no regression.

#![feature(test)]

extern crate test;

#[bench]
fn bench(t: &mut test::Bencher) {
    use serde::de::{Deserialize, Deserializer, SeqAccess, Visitor};
    use std::fmt;

    struct Arrays;

    impl<'de> Deserialize<'de> for Arrays {
        fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
        where
            D: Deserializer<'de>,
        {
            deserializer.deserialize_any(Arrays)
        }
    }

    impl<'de> Visitor<'de> for Arrays {
        type Value = Arrays;

        fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
            formatter.write_str("arrays")
        }

        fn visit_u64<E>(self, _: u64) -> Result<Self::Value, E> {
            Ok(Arrays)
        }

        fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
        where
            A: SeqAccess<'de>,
        {
            while seq.next_element::<Arrays>()?.is_some() {}
            Ok(Arrays)
        }
    }

    let mut json = "0".to_owned();
    for _ in 0..16 {
        json = format!("[{json},{json}]");
    }
    t.bytes = json.len() as u64;
    t.iter(|| serde_json::from_reader::<_, Arrays>(&mut json.as_bytes()));
}

Closes #1102.

@dtolnay dtolnay merged commit 4cb90ce into serde-rs:master Oct 19, 2024
16 checks passed
@dtolnay dtolnay deleted the hasnext branch October 19, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant