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

Add experiment to produce precompiled builds of serde_derive #2514

Merged
merged 1 commit into from
Jul 18, 2023

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Jul 18, 2023

# Cargo.toml

[dependencies]
serde = "1"  # no features=["derive"]
serde_derive-x86_64-unknown-linux-gnu = "1.0.171-alpha.3"
use serde_derive::{Deserialize, Serialize};

#[derive(Deserialize, Serialize)]
pub struct MyStruct {
    /* ... */
}

If this works, it needs to be productionized further using any of the following techniques:

  • Cosmopolitan Libc / αcτµαlly pδrταblε εxεcµταblε
  • [target.x86_64-unknown-linux-gnu.dependencies] to make the macro forward to a different crate on different targets
  • Precompile multiple different targets into the same crate, and detect the current platform during macro expansion time

Screenshot from 2023-07-18 13-44-10

@dtolnay dtolnay merged commit eb3f232 into serde-rs:master Jul 18, 2023
@dtolnay dtolnay deleted the precompiled branch July 18, 2023 20:48
@dtolnay
Copy link
Member Author

dtolnay commented Jul 19, 2023

Expansion time benchmark

I tested against https://github.com/dtolnay/clang-ast/blob/0.1.19/tests/exhaustive.rs to quantify the overhead of process spawning and serialization. That integration test contains 286 calls to derive(Deserialize).

 [dev-dependencies]
-serde_derive = "1.0.171"
+serde_derive-x86_64-unknown-linux-gnu = "1.0.171-alpha.1"

Here is the command I measured:

$ time cargo expand --ugly --test exhaustive >expand.rs

On my machine, the macro expansion takes 1.06 seconds with serde_derive and 1.91 seconds with serde_derive-x86_64-unknown-linux-gnu. (They produce the same code.)

That is about 3 milliseconds per macro call.

@Mingun
Copy link
Contributor

Mingun commented Jul 20, 2023

Interesting. Could you explain, what is improved, because in you last comment there is slowdown!? Some statistics also would be interesting.

@dtolnay
Copy link
Member Author

dtolnay commented Jul 20, 2023

That specifically only measures macro expansion. Overall build time for serde_derive goes from 9.2 seconds to 0.8 seconds (at -j1) or 4.9 seconds to 0.48 seconds (at -j8) which is 10× improvement.

@safinaskar
Copy link

safinaskar commented Aug 21, 2023

Cosmopolitan Libc

@dtolnay, if this feature will be ever re-enabled, then, please, don't use Cosmopolitan. Cosmopolitan is just a hack. It has a lot of problems:

  • Despite advertised as universal, it seems to work on x86 only. On some limited amount of platforms. So it is no better than, say, 3 separate statically linked binaries for very same platforms
  • It relies on Thompson shell hack, i. e. shell scripts without shebang. So if you run the binary using execve you are probably out of lack
  • Sometimes these binaries are flagged by Windows antiviruses

Overall, this is very hacky solution for a non-problem (i. e. what is wrong with just creating some amount of separate binaries?)


let mut trees = Vec::new();
while !buf.is_empty() {
match match buf.read_u8() {

This comment was marked as resolved.

@serde-rs serde-rs locked as resolved and limited conversation to collaborators Aug 21, 2023
@oli-obk
Copy link
Member

oli-obk commented Aug 21, 2023

I'm preemptively locking this PR. Please open new issues instead of commenting on merged PRs

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants