Slightly optimise CString

[ollie27]

Avoid a reallocation in CString::from and CStr::to_owned.

[rust-highfive]

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

Thanks for the PR! Are you worried about any particular function for #[inline]? I'd prefer to avoid adding the tag to basically all of them as it can have serious effects on downstream consumers, but adding a few select ones should be fine.

[alexcrichton]

Also, do you have benchmarks to back up the unsafe additions of eliding bounds checking?

[ollie27]

Are you worried about any particular function for #[inline]? I'd prefer to avoid adding the tag to basically all of them as it can have serious effects on downstream consumers, but adding a few select ones should be fine.

Not really, I just noticed that calling as_ptr on a CString made two function calls (one to deref and one to CStr::as_ptr), now it's just a pointer copy as expected even without lto. I've only added #[inline] to trivial functions and functions which basically just make a library call. Are there any special rules about what should be marked #[inline] in std?

Also, do you have benchmarks to back up the unsafe additions of eliding bounds checking?

I unfortunately haven't been able to see any difference from removing the bounds checks, only a possible tiny improvement from inlining. It probably doesn't matter much so I can remove it if you think it's too risky, I just noticed it in the assembly and thought we may as well remove some unnecessary instructions.

[alexcrichton]

We tend to avoid #[inline] wherever possible as it tends to lead to bloated compile times (more for LLVM to work with), so we prefer to see numeric improvements when adding the tags. It's true most of these functions are trivial, thought.

We also tend to prefer only using unsafe code for optimizations if we can show the improvement we're getting in perf, but if that's not the case here perhaps that could be backed out for now?

[ollie27]

I've removed the inlining and bounds check removal. They weren't very important anyway. The use of #[inline] in std does seem to be very inconsistent.

[alexcrichton]

@bors: r+

Yes #[inline] application has historically been very liberal in the past, but recently we've had renewed interest in cracking down

[bors]

📌 Commit 18f5f99 has been approved by alexcrichton