-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Box Item::Attributes #80802
Box Item::Attributes #80802
Conversation
This comment has been minimized.
This comment has been minimized.
The job Click to see the possible cause of the failure (guessed by this bot)
|
Looks good for now. I'll re-review once #80339 has been merged. Also, might be worth it to have a perf run too. |
☔ The latest upstream changes (presumably #80867) made this pull request unmergeable. Please resolve the merge conflicts. |
6d21dd3
to
030e1c9
Compare
This reduces the size of Item from 136 to 48 bytes.
030e1c9
to
690aeaf
Compare
@bors try @rust-timer queue This didn't actually depend on #80339, I was just too lazy to change it. Since that isn't getting merged any time soon I dropped those changes. |
Awaiting bors try build completion. |
⌛ Trying commit 690aeaf with merge 2420bbf76cea896b7592e5edbbf9be549d1c8f3e... |
☀️ Try build successful - checks-actions |
Queued 2420bbf76cea896b7592e5edbbf9be549d1c8f3e with parent fd34606, future comparison URL. @rustbot label: +S-waiting-on-perf |
Finished benchmarking try commit (2420bbf76cea896b7592e5edbbf9be549d1c8f3e): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
Interesting. Up to -.6% on serde-doc (which is probably the closest we have to a realistic rustdoc benchmark), but up to +.2% on other several other benches, including hello world and await-call-tree. Max-rss is all over the place. One reason this might be slow is that attributes are accessed a lot, here's
|
@bors r+ |
📌 Commit 690aeaf has been approved by |
⌛ Testing commit 690aeaf with merge ab80a582cfb3c1ab6117d842406e32bbe036e509... |
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
Looks spurious:
@bors retry |
☀️ Test successful - checks-actions |
This reduces the size of Item from 128 to 40 bytes. I think this is as small as it needs to get 🎉
Builds on #80339 and should not be merged before.
r? @GuillaumeGomez