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

[do not merge] fmt::Arguments experiment #84823

Closed
wants to merge 2 commits into from
Closed

Conversation

m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented May 2, 2021

Just testing some incomplete idea.

@m-ou-se m-ou-se added A-fmt Area: `std::fmt` S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. labels May 2, 2021
@m-ou-se m-ou-se self-assigned this May 2, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 2, 2021
@m-ou-se
Copy link
Member Author

m-ou-se commented May 2, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 2, 2021
@bors
Copy link
Contributor

bors commented May 2, 2021

⌛ Trying commit 3259ba1 with merge f585107e5e534511ae54ad991ac429aef3b99ebd...

@m-ou-se m-ou-se removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 2, 2021
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented May 2, 2021

💔 Test failed - checks-actions

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 2, 2021
@rust-log-analyzer

This comment has been minimized.

@m-ou-se
Copy link
Member Author

m-ou-se commented May 2, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@bors
Copy link
Contributor

bors commented May 2, 2021

⌛ Trying commit e89626f with merge 9d649c160c564fa8820680f3132796f90912466b...

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
configure: rust.channel         := nightly
configure: rust.debug-assertions := True
configure: llvm.assertions      := True
configure: dist.missing-tools   := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
configure: 
---
tidy error: /checkout/library/core/src/fmt/mod.rs:1266: undocumented unsafe
Found 515 error codes
Found 0 error codes with no tests
Done!
tidy error: /checkout/compiler/rustc_builtin_macros/src/format.rs:63: XXX is deprecated; use FIXME
tidy error: /checkout/compiler/rustc_builtin_macros/src/format.rs:365: XXX is deprecated; use FIXME
tidy error: /checkout/compiler/rustc_builtin_macros/src/format.rs:428: XXX is deprecated; use FIXME
some tidy checks failed



command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/rust-tidy" "/checkout" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "/checkout/obj/build" "16"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test --stage 2 src/tools/tidy
Build completed unsuccessfully in 0:00:12

@bors
Copy link
Contributor

bors commented May 2, 2021

☀️ Try build successful - checks-actions
Build commit: 9d649c160c564fa8820680f3132796f90912466b (9d649c160c564fa8820680f3132796f90912466b)

@rust-timer
Copy link
Collaborator

Queued 9d649c160c564fa8820680f3132796f90912466b with parent 89ebad5, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (9d649c160c564fa8820680f3132796f90912466b): 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 rollup- to bors.

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
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels May 2, 2021
@bjorn3
Copy link
Member

bjorn3 commented May 3, 2021

Up to 9.3% improvement for helloworld-opt incr-patched and up to 8.4% regression for webrender-wrench-check incr-patched. For full builds many regressions up to 6.8% and some improvements up to 2.0%.

@m-ou-se
Copy link
Member Author

m-ou-se commented May 4, 2021

Yeah these results are all over the place. Not sure why.

fmt::Arguments was reduced from 6 pointers to 2 pointers in size, and the output of format_args!() didn't really change much in complexity.

@m-ou-se m-ou-se removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 5, 2021
@Mark-Simulacrum
Copy link
Member

Going to go ahead and close this since it seems like it's not likely to receive much further attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-fmt Area: `std::fmt` S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants