collections: Move optimized String::from_str to String::from #24517
Conversation
|
r? @pcwalton (rust_highfive has picked a reviewer for you, use r? to override) |
| @@ -1015,7 +1015,7 @@ impl AsRef<str> for String { | |||
| impl<'a> From<&'a str> for String { | |||
| #[inline] | |||
| fn from(s: &'a str) -> String { | |||
| s.to_string() | |||
| String { vec: <[_]>::to_vec(string.as_bytes()) } | |||
There was a problem hiding this comment.
As an onlooker, I'm a bit confused here. Where is the string variable coming from, or do you mean to have s?
There was a problem hiding this comment.
shame on me! you're right. Let me fix that.
| let s = "Hello there, the quick brown fox jumped over the lazy dog! \ | ||
| Lorem ipsum dolor sit amet, consectetur. "; | ||
| b.iter(|| { | ||
| test::black_box(String::from_str(s)); |
There was a problem hiding this comment.
It's ok to actually omit these calls to test::black_box, whatever is returned from the closure will be passed to black_box automatically (so these could all just return the string)
|
Nice! r=me with the benchmark tweaks |
|
@bors: r=alexcrichton c8841d2 |
|
Is it possible to |
|
Nevermind, I don't think my previous idea works. |
|
⌛ Testing commit c8841d2 with merge fb01e4b... |
|
💔 Test failed - auto-linux-64-nopt-t |
This implementation is currently about 3-4 times faster than using the `.to_string()` based approach.
This implementation is currently about 3-4 times faster than using the `.to_string()` based approach. I would also suggest we deprecate `String::from_str` since it's redundant with the stable `String::from` method, but I'll leave that for a future PR.
Deprecation: rust-lang/rust#24517 nightly gives a warning, depr. status in 1.0.0 release notes: https://github.com/rust-lang/rust/blob/master/RELEASES.md
This implementation is currently about 3-4 times faster than using the
.to_string()based approach.I would also suggest we deprecate
String::from_strsince it's redundant with the stableString::frommethod, but I'll leave that for a future PR.