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

Optimize compile time of Deserializer trait methods #687

Closed
wants to merge 1 commit into from

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Jun 28, 2020

We can instantiate the control flow in these methods once per Read type rather than once per Visitor type. This reduces compile time of https://github.com/serde-rs/json-benchmark by 44% as measured by cargo clean && RUSTFLAGS='-C codegen-units=1' cargo build --release --no-default-features --features parse-struct,lib-serde,all-files -Ztimings, with 0-13% impact on performance.

Before

   Completed serde_json v1.0.55 in 3.8s
   Completed json-benchmark v0.0.1 bin "json-benchmark" in 18.1s
                                DOM                  STRUCT
======= serde_json ======= parse|stringify ===== parse|stringify ====
data/canada.json         270 MB/s              620 MB/s
data/citm_catalog.json   390 MB/s              950 MB/s
data/twitter.json        270 MB/s              560 MB/s

After

   Completed serde_json v1.0.55 in 3.7s
   Completed json-benchmark v0.0.1 bin "json-benchmark" in 10.1s
                                DOM                  STRUCT
======= serde_json ======= parse|stringify ===== parse|stringify ====
data/canada.json         250 MB/s              540 MB/s
data/citm_catalog.json   380 MB/s              850 MB/s
data/twitter.json        260 MB/s              560 MB/s

@dtolnay dtolnay added the wip label Jun 28, 2020
Marwes pushed a commit to Marwes/json that referenced this pull request Jun 30, 2020
Marwes pushed a commit to Marwes/json that referenced this pull request Jun 30, 2020
@Marwes
Copy link

Marwes commented Jun 30, 2020

This could be done in a safe manner though I guess there would be some additional branches unless the unreachable!() branches are made unreachable_unchecked. https://github.com/Marwes/json/tree/safe_once_dispatch

It is only a POC but I'd expect the vtable from the trait to end up the same as this unsafe impl.

@Marwes
Copy link

Marwes commented Jun 30, 2020

cc #313

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants