Use SmallVec for SmallCStr #53644
Use SmallVec for SmallCStr #53644
Conversation
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| const SIZE: usize = 38; | ||
| use smallvec::SmallVec; | ||
|
|
||
| const SIZE: usize = 36; |
There was a problem hiding this comment.
Any reason for the change?
There was a problem hiding this comment.
Yes, Array isn't implemented for [T; 38], and due to the union optimization, the on-stack version can take more content per byte.
| SmallVec::from_vec(data) | ||
| }; | ||
| if let Err(e) = ffi::CStr::from_bytes_with_nul(&data) { | ||
| panic!("The string \"{}\" cannot be converted into a CStr: {}", s, e); |
There was a problem hiding this comment.
I know you're not introducing this, but should we be returning Result here instead? Probably ok given the usage, but still...
There was a problem hiding this comment.
This was already discussed during the introduction of the SmallCStr type, the consensus was that ergonomics are more important as long as it's possible to get a stack trace.
| data: ffi::CString, | ||
| } | ||
| pub struct SmallCStr { | ||
| data: SmallVec<[u8; SIZE]>, |
There was a problem hiding this comment.
Ah! Now I see what you're doing. Why is this an improvement over the OnHeap version?
There was a problem hiding this comment.
Absent the union optimization, this should generate the same code. However with the union optimization, the type is reduced to (usize, [T; _] / *mut T), which saves the discriminant + padding.
Apart from the memory savings, the code has seen a good deal of perf work already.
|
r=me, but I wan't to know the response for the comments |
| } | ||
|
|
||
| #[inline] | ||
| pub fn new_with_zero(s: &str) -> SmallCStr { |
There was a problem hiding this comment.
This should probably be called new_with_nul , or from_str_with_nul.
llogiq
force-pushed
the
smallvec-for-small-c-str
branch
from
39f1e31 to
25a83e3
Compare
|
@bors r+ rollup |
|
📌 Commit 25a83e3 has been approved by |
bors
added
S-waiting-on-bors
…estebank Use SmallVec for SmallCStr This reuses the awesome optimizations from Servo's `SmallVec` to speed up `SmallCStr`.
Rollup of 16 pull requests Successful merges: - #53311 (Window Mutex: Document that we properly initialize the SRWLock) - #53503 (Discourage overuse of mem::forget) - #53545 (Fix #50865: ICE on impl-trait returning functions reaching private items) - #53559 (add macro check for lint) - #53562 (Lament the invincibility of the Turbofish) - #53563 (use String::new() instead of String::from(""), "".to_string(), "".to_owned() or "".into()) - #53592 (docs: minor stylistic changes to str/string docs) - #53594 (Update RELEASES.md to include clippy-preview) - #53600 (Fix a grammatical mistake in "expected generic arguments" errors) - #53614 (update nomicon and book) - #53617 (tidy: Stop requiring a license header) - #53618 (Add missing fmt examples) - #53636 (Prefer `.nth(n)` over `.skip(n).next()`.) - #53644 (Use SmallVec for SmallCStr) - #53664 (Remove unnecessary closure in rustc_mir/build/mod.rs) - #53666 (Added rustc_codegen_llvm to compiler documentation.)
This reuses the awesome optimizations from Servo's
SmallVecto speed upSmallCStr.