Skip to content

Add builtin derive benchmarks #791

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

Merged
merged 2 commits into from
Nov 9, 2020
Merged

Conversation

rylev
Copy link
Member

@rylev rylev commented Nov 5, 2020

tl;dr Using #[derive] for built in traits incurs significant overhead at scale

I've created a benchmark which has 5,000 structs with one i32 field. I then #[derive(Copy, Clone, Default, Eq, PartialEq, Debug)] for all the structs. This builds using the current nightly compiler (f2bbdd0a3 2020-11-04) in ~5 seconds. When I manually implement these traits, the code compiles in ~2.5 seconds.

This issue is being discussed on Zulip.

@Mark-Simulacrum
Copy link
Member

It may make sense to delete the Copy derive (at least in say half the cases); Copy as a derive makes the Clone impl much simpler and likely much cheaper too.

@joshtriplett
Copy link
Member

for the sake of making it easier to see optimizations or pessimizations that affect a series of derives on the same type, could you:

  1. Add Ord, PartialOrd, and Hash, which rounds out the complete set of derivable traits.
  2. Change half of these to write the derives separately (#[derive(Debug)] #[derive(Default)] #[derive(PartialEq)] ...)?

(This is in addition to @Mark-Simulacrum's suggestion.)

@rylev
Copy link
Member Author

rylev commented Nov 6, 2020

@joshtriplett rustsmt collapses separate derives into one line. Obviously not everyone uses rustfmt, but maybe this makes it a bit less compelling to test that case? Also, it does not seem to have any performance impact to keep them separate.

By the way, I am using the this script to generate the benchmark.

@Mark-Simulacrum Mark-Simulacrum merged commit 4c136f0 into rust-lang:master Nov 9, 2020
@rylev rylev deleted the derive branch November 9, 2020 14:07
@joshtriplett
Copy link
Member

@rylev My only concern was catching potential regressions, but it isn't critical, and as you said, rustfmt makes it unlikely anyone will hit that case.

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

Successfully merging this pull request may close these issues.

3 participants