Inline some serialize impls#71208
Conversation
Some trivial methods in impls of the `Serialize::Encoder` trait are not marked `#[inline]` but are not generic and are (I believe) used cross-crate.
At present, the macro only adds `#[inline]` to the first method.
| macro_rules! encoder_methods { | ||
| ($($name:ident($ty:ty);)*) => { | ||
| #[inline] | ||
| $(fn $name(&mut self, value: $ty) -> Result<(), Self::Error> { |
There was a problem hiding this comment.
Haha, this one is great!
There was a problem hiding this comment.
Oh dear, I just realized what's going on now.
|
Presuming this will ultimately be approved, do we want to just rollup=never this or do a manual perf run now? rollup=never saves one perf run if this does have an impact, and the results from #69281 indicate that it will probably have a small one. |
|
I always do a perf run ahead of time for any change that is intended to improve performance, because sometimes things don't work the way you'd expect. @bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 99cca04 with merge 52d7e4551d123138e20df7d2fd74e1998ed3e815... |
|
r=me after perf run |
|
Oh and might as well: @bors rollup=never |
|
☀️ Try build successful - checks-azure |
|
Queued 52d7e4551d123138e20df7d2fd74e1998ed3e815 with parent 7f3df57, future comparison URL. |
|
Finished benchmarking try commit 52d7e4551d123138e20df7d2fd74e1998ed3e815, comparison URL. |
|
This caused both instruction count and task-clock measurements to regress slightly. I might try a run with only |
|
Looks like a wash to me, tbh. I wonder if none of these actually matter in practice. I recall @nnethercote did some work with encoding/decoding, maybe they found everything hot enough. |
|
So I think we can close this. I'm gonna try making I think we should do something about the dangling |
|
Closing this based on the above comment |
Some trivial methods in impls of the
Serialize::Encodertrait are not marked#[inline]but are not generic and are (I believe) used cross-crate.r? @eddyb