Remove shrink_to_fit from default ToString::to_string implementation. #77997
Conversation
rust-highfive
added
the
S-waiting-on-review
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 1a7c78319414969dfbcc67c4afa2b2081f3a3f66 with merge a9f657d64df859414475bb3329cda65b5445193e... |
|
☀️ Try build successful - checks-actions, checks-azure |
|
Queued a9f657d64df859414475bb3329cda65b5445193e with parent dd7fc54, future comparison URL. |
|
Finished benchmarking try commit (a9f657d64df859414475bb3329cda65b5445193e): 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 |
|
That is looking good! |
Co-authored-by: scottmcm <scottmcm@users.noreply.github.com>
m-ou-se
force-pushed
the
to-string-no-shrink
branch
from
1a7c783
to
0b06288
Compare
|
This is a win across the board for performance, and in many cases it's a win in max memory usage, too. A few tests do have higher memory usage, but they don't look unreasonable in absolute terms. I would propose that we merge this as-is, and that we open an issue to do some memory profiling to see where the memory is going in keccak-debug incr-full and inflate-opt full, which could lead us to add back a few targeted |
|
I strongly suspect that the max-rss <15-20% changes are just noise; if you look at the graphs for the last month (https://perf.rust-lang.org/?start=&end=&absolute=true&stat=max-rss) and scroll down to keccak, you'll see that the data is really noisy. I am on board with merging this though. |
|
@bors r+ |
|
📌 Commit 0b06288 has been approved by |
bors
added
S-waiting-on-bors
|
Wow! I never would have guessed it would make that much of a difference! |
|
☀️ Test successful - checks-actions, checks-azure |
joshtriplett
added
the
relnotes
|
I'm adding relnotes to this (not relnotes-perf), because it is technically a visible semantic change. If you have a |
Pkgsrc changes:
* Adjust patches, convert tabs to spaces so that tests pass.
* Remove patches which are no longer needed (upstream changed)
* Minor adjustments for SunOS, e.g. disable stack probes.
* Adjust cargo checksum patching accordingly.
* Remove commented-out use of PATCHELF on NetBSD, which doesn't work anyway...
Upstream changes:
Version 1.49.0 (2020-12-31)
============================
Language
-----------------------
- [Unions can now implement `Drop`, and you can now have a field in a union
with `ManuallyDrop<T>`.][77547]
- [You can now cast uninhabited enums to integers.][76199]
- [You can now bind by reference and by move in patterns.][76119] This
allows you to selectively borrow individual components of a type. E.g.
```rust
#[derive(Debug)]
struct Person {
name: String,
age: u8,
}
let person = Person {
name: String::from("Alice"),
age: 20,
};
// `name` is moved out of person, but `age` is referenced.
let Person { name, ref age } = person;
println!("{} {}", name, age);
```
Compiler
-----------------------
- [Added tier 1\* support for `aarch64-unknown-linux-gnu`.][78228]
- [Added tier 2 support for `aarch64-apple-darwin`.][75991]
- [Added tier 2 support for `aarch64-pc-windows-msvc`.][75914]
- [Added tier 3 support for `mipsel-unknown-none`.][78676]
- [Raised the minimum supported LLVM version to LLVM 9.][78848]
- [Output from threads spawned in tests is now captured.][78227]
- [Change os and vendor values to "none" and "unknown" for some targets][78951]
\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.
Libraries
-----------------------
- [`RangeInclusive` now checks for exhaustion when calling `contains`
and indexing.][78109]
- [`ToString::to_string` now no longer shrinks the internal buffer
in the default implementation.][77997]
- [`ops::{Index, IndexMut}` are now implemented for fixed sized
arrays of any length.][74989]
Stabilized APIs
---------------
- [`slice::select_nth_unstable`]
- [`slice::select_nth_unstable_by`]
- [`slice::select_nth_unstable_by_key`]
The following previously stable methods are now `const`.
- [`Poll::is_ready`]
- [`Poll::is_pending`]
Cargo
-----------------------
- [Building a crate with `cargo-package` should now be independently
reproducible.][cargo/8864]
- [`cargo-tree` now marks proc-macro crates.][cargo/8765]
- [Added `CARGO_PRIMARY_PACKAGE` build-time environment
variable.] [cargo/8758] This variable will be set if the crate
being built is one the user selected to build, either with `-p`
or through defaults.
- [You can now use glob patterns when specifying packages &
targets.][cargo/8752]
Compatibility Notes
-------------------
- [Demoted `i686-unknown-freebsd` from host tier 2 to target tier
2 support.][78746]
- [Macros that end with a semi-colon are now treated as statements
even if they expand to nothing.][78376]
- [Rustc will now check for the validity of some built-in attributes
on enum variants.][77015] Previously such invalid or unused
attributes could be ignored.
- Leading whitespace is stripped more uniformly in documentation
comments, which may change behavior. You read [this post about
the changes][rustdoc-ws-post] for more details.
- [Trait bounds are no longer inferred for associated types.][79904]
Internal Only
-------------
These changes provide no direct user facing benefits, but represent
significant improvements to the internals and overall performance
of rustc and related tools.
- [rustc's internal crates are now compiled using the `initial-exec` Thread
Local Storage model.][78201]
- [Calculate visibilities once in resolve.][78077]
- [Added `system` to the `llvm-libunwind` bootstrap config option.][77703]
- [Added `--color` for configuring terminal color support to bootstrap.][79004]
[75991]: rust-lang/rust#75991
[78951]: rust-lang/rust#78951
[78848]: rust-lang/rust#78848
[78746]: rust-lang/rust#78746
[78376]: rust-lang/rust#78376
[78228]: rust-lang/rust#78228
[78227]: rust-lang/rust#78227
[78201]: rust-lang/rust#78201
[78109]: rust-lang/rust#78109
[78077]: rust-lang/rust#78077
[77997]: rust-lang/rust#77997
[77703]: rust-lang/rust#77703
[77547]: rust-lang/rust#77547
[77015]: rust-lang/rust#77015
[76199]: rust-lang/rust#76199
[76119]: rust-lang/rust#76119
[75914]: rust-lang/rust#75914
[74989]: rust-lang/rust#74989
[79004]: rust-lang/rust#79004
[78676]: rust-lang/rust#78676
[79904]: rust-lang/rust#79904
[cargo/8864]: rust-lang/cargo#8864
[cargo/8765]: rust-lang/cargo#8765
[cargo/8758]: rust-lang/cargo#8758
[cargo/8752]: rust-lang/cargo#8752
[`slice::select_nth_unstable`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable
[`slice::select_nth_unstable_by`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by
[`slice::select_nth_unstable_by_key`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by_key
[`hint::spin_loop`]: https://doc.rust-lang.org/stable/std/hint/fn.spin_loop.html
[`Poll::is_ready`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_ready
[`Poll::is_pending`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_pending
[rustdoc-ws-post]: https://blog.guillaume-gomez.fr/articles/2020-11-11+New+doc+comment+handling+in+rustdoc
As suggested by @scottmcm on Zulip. shrink_to_fit() seems like the wrong thing to do here in most use cases of to_string(). Would be intereseting to see if it makes any difference in a timer run.
r? @joshtriplett