-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Reduce number of allocations done by NLL #51617
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
Conversation
// non-handling of siblings); | ||
// - ~99% of the time the loop isn't reached, and this code hot, so we | ||
// don't want to allocate `todo` unnecessarily. | ||
// the time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"the time" here is a dangling fragment.
And since you'll be updating the comment phrasing anyway, i would like it if you added an explicit note that we do not traverse siblings of the input mpi
, merely its children. (I believe that's what the comment on push_siblings
was trying to do in the previous version in this code.)
this looks fine to me; I left a nit about a typo in the comments and also about a comment that was deleted that I would like preserved. |
r? @pnkfelix |
`has_any_child_of` is hot. It allocates a `Vec` that almost always doesn't exceed a length of 1. This patch peels off the first iteration of the loop, avoiding the need for the `Vec` creation in ~99% of cases.
These vectors are always small, so this avoids lots of allocations.
@pnkfelix: new version is up. I removed the dangling text and tweaked the new comment about siblings. (Note that the old comment about siblings is still present.) |
@bors r+ rollup |
📌 Commit ba0bb02 has been approved by |
Reduce number of allocations done by NLL A couple of easy wins. Here are the NLL speedups that exceed 1%: ``` sentry-cli-check avg: -3.5% min: -3.5% max: -3.5% inflate-check avg: -1.9% min: -1.9% max: -1.9% inflate avg: -1.7% min: -1.7% max: -1.7% clap-rs-check avg: -1.6% min: -1.6% max: -1.6% cargo-check avg: -1.6% min: -1.6% max: -1.6% ripgrep-check avg: -1.4% min: -1.4% max: -1.4% serde-check avg: -1.2% min: -1.2% max: -1.2% regex-check avg: -1.0% min: -1.0% max: -1.0% sentry-cli avg: -1.0% min: -1.0% max: -1.0% ``` r? @nikomatsakis
☀️ Test successful - status-appveyor, status-travis |
A couple of easy wins. Here are the NLL speedups that exceed 1%:
r? @nikomatsakis