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

Use ? instead of try! internally and in generated code #2366

Open
Lucretiel opened this issue Jan 20, 2023 · 4 comments
Open

Use ? instead of try! internally and in generated code #2366

Lucretiel opened this issue Jan 20, 2023 · 4 comments

Comments

@Lucretiel
Copy link

Much of serde's implementation uses try!, which I assumed was for backwards compatibility reasons. However, while support for Option was only added in Rust 1.22, the ? itself was added in Rust 1.13, a much earlier version than serde's claimed MSRV of 1.19.

I'd need to bench to be sure, but my presumption is that ?, a built-in operator, is faster for the compiler than expanding the try! macro.

@dtolnay
Copy link
Member

dtolnay commented Jan 20, 2023

The last time I checked (~1 year ago) ? still produced larger binaries than try!, compiled slower, and had worse performance at runtime.

@Lucretiel
Copy link
Author

Surprising but fair enough.

@Lucretiel
Copy link
Author

Lucretiel commented Jan 21, 2023

Supposedly this was fixed last August: rust-lang/rust#37939

@dtolnay
Copy link
Member

dtolnay commented Jan 21, 2023

Oh nice. I think that issue was only in regard to runtime performance though? I tried to check compile time using this diff in serde_derive:

         macro_rules! try {
             (#dollar __expr:expr) => {
-                match #dollar __expr {
-                    _serde::__private::Ok(__val) => __val,
-                    _serde::__private::Err(__err) => {
-                        return _serde::__private::Err(__err);
-                    }
-                }
+                #dollar __expr?
             }
         }

and using the following commands:

and these all compile consistently 6.5–7.5% slower on my machine with ?, using current nightly. That's the identical ratio noted 5 years ago in #1121 (comment). ? just generates more inference variables, and function calls / copies / layers of trait abstractions that need to get compiled or inlined.

The gap would be larger upon using ? in serde_json too, since that currently uses its own tri! macro, for the same reason.

Maybe the gap would be slightly smaller with try! eliminated entirely from serde_derive in favor of directly using ? throughout all the quote! blocks, but not by more than 0.25% percentage points I'd guess. A tiny macro_rules like that is effectively instant to expand compared to type checking.

I did confirm the json-benchmark produces an identical binary after strip, so that's great. I could believe it no longer makes sense to use try! at this point considering the deleterious effect on readability of cargo expand output.

@dtolnay dtolnay reopened this Jan 21, 2023
@dtolnay dtolnay changed the title Nitpick: use ? instead of try! internally and in generated code Use ? instead of try! internally and in generated code Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants