Small string optimization: remove as_mut_vec

[LukasKalbertodt]

As far as I can tell nobody in this thread was really concerned about the removal (or private) of as_mut_vec. No one has evidence that this method is used often or in important ways. Is that correct? And we also agree that this method can be replaced with other unsafe methods like from_raw_parts without doing much additional work (with the little exception when a SSO string needs to convert to a normal string first)., right? And we agree that keeping this method would limit the ability to change the implementation of the string type which could (or could not) be a problem in the future.

So I don’t really see any reason to keep it.

---

But I read and thought about another idea that was mentioned somewhere. I am really not sure if this is useful, but I think it would be a pretty flexible solution.

We separate the “String” into two parts: Buffer and UTF8-Wrapper. We would have an trait called StringBuf (for example) and a type UTF8Wrapper<B: StringBuf>. The wrapper ensures that everything stays UTF8 by using the methods specified in the StringBuf interface. The string module would consists of the wrapper type, a few special string buffers (like SSO) and this type alias:

type String = UTF8Wrapper<Vec>; // or UTF8Wrapper<SSOBuf>

I really like the idea. But to really use this some things needs to be clear:

• Is there enough time to make such a big change?
• Is it possible to do that without loss in performance?
• Would the trait StringBuf allow the implementation by a immutable string buffer?

---

So: I really think we should at least remove as_mut_vec and maybe think of another even better solution.

[spud_taters]

FWIW, from_utf8() also seems to be strongly coupled with an internal Vec representation, because the docs say:

Returns the vector as a string buffer, if possible, **taking care not to copy it**.

[LukasKalbertodt]

FWIW, from_utf8() also seems to be strongly coupled with an internal Vec representation

It should be possible to implement that without copying the buffer even without using Vec as implementation, I think. And I don’t think that this should be a part of the String type but rather of a special VecString type…

[comex]

It wouldn’t be mandatory under SSO for every String constructor to actually use it, including from_utf8 and from_raw_parts. However, in this still hypothetical world where SSO for String is benchmarked to be a benefit in general, I think from_utf8 might be common enough to want it; I think a copy bounded from above by 23ish bytes (which, since the destination is always exactly that length, can be done using all of three 64-bit load/stores in the common case the source isn’t near the end of a page) isn’t exactly what the documentation was worried about.

[d3zd3z]

There seem to be several mentions of benchmarking SSO to see if it is a benefit. I thought the benefit of SSO was a reduction of memory use, and that it was generally done for that, despite having a performance hit.

[aturon]

As far as I can tell nobody in this thread was really concerned about the removal (or private) of as_mut_vec. No one has evidence that this method is used often or in important ways. Is that correct?And we also agree that this method can be replaced with other unsafe methods like from_raw_parts without doing much additional work (with the little exception when a SSO string needs to convert to a normal string first)., right?

One of the reasons this method exists is to allow direct mutation of strings as byte buffers without UTF8 checking (i.e., unsafe manipulation). There were previously a bunch of distinction unsafe APIs replicating some of the Vec methods in an ad hoc way, but as_mut_vec makes it possible to just compose with the Vec API directly.

And we agree that keeping this method would limit the ability to change the implementation of the string type which could (or could not) be a problem in the future.

It is somewhat limiting, in that it must be possible to acquire a particular kind of mutable reference into the string (it must be “upgradable” to a vector). But it does not imply that the String representation is always a simple Vec wrapper.

[comex]

It is somewhat limiting, in that it must be possible to acquire a particular kind of mutable reference into the string (it must be “upgradable” to a vector). But it does not imply that the String representation is always a simple Vec wrapper.

I think this is likely to be quite limiting in practice in terms of the resulting available encoding space, unless String is changed to actually be larger than Vec, which sounds bad.

[LukasKalbertodt]

I think this is likely to be quite limiting in practice in terms of the resulting available encoding space, unless String is changed to actually be larger than Vec, which sounds bad.

I agree. Sure: It’s possible, but not in a nice way.

One of the reasons this method exists is to allow direct mutation of strings as byte buffers without UTF8 checking (i.e., unsafe manipulation).

Are there really situations in which I need to change the buffer in a safe and unsafe way alternating? Having a big raw buffer and wanting to wrap a String around it is often useful. And having a String and then mutate it in an unsafe way is too. But always swapping between unsafe and safe operations sounds strange…

[mzabaluev]

After all things said, I have actually found a somewhat valid use case for as_mut_vec: in an optimized implementation of std::io::Read, where it’s certain that read_to_end would either return a complete UTF-8 string or an error, read_to_string can simply give the string’s underlying Vec to read_to_end.

[LukasKalbertodt]

I’ve to admit that I don’t quite get it. The only advantage of that is that read_to_string would be a one-liner? You are not talking about performance, right?

[mzabaluev]

It is about performance and avoiding copies: read_to_string appends to a String passed by mutable reference. There is no way to move out of the reference, so it’s either reading into a temporary Vec that is then appended to the string, or passing the string into read_to_end as a Vec.

See how it’s done in std::io.

[LukasKalbertodt]

Ok, that makes sense. My important question is: Is it OK to have a bit overhead here? If we wanted to replace as_mut_vec with something else, we would have a bit additional work. Consider this (kinda pseudocode):

let vec = Vec::from_raw_parts(buf.to_raw_parts());
// do "append to vec" stuff with UTF8 checks
return String::from_raw_parts(vec.to_raw_parts());

Now this would be more work, yes. But could one live with that little additional work or is it a no-go?

• If it’s not too bad, I will quickly try to find a fast API fix in RFC form.
• If it’s a no-go, it is really a valid example that some stuff really just needs the fact that there is a Vec in String. But I would propose that the documentation makes clear that String is really intended to be a UTF8Wrapper around Vec.

[mzabaluev]

I think reassembling the String from raw parts may be a more disciplined approach; as you can see in the std::io module, there is a guard that ensures the string is restored to a correct state even if an unwind happens, and this would look more natural with “phase transitions” between String and Vec. In general, any code temporarily lifting the string invariant for unsafe mutation should be similarly elaborate, so the verbosity of Strong::from_raw_parts is not a big issue.

@LukasKalbertodt, will you submit an RFC PR for “as_mut_vec considered harmful?”

[LukasKalbertodt]

[EDIT: This solution doesn’t make sense, read my next post]

Sorry for the delay! I would love to submit an RFC! I think I have an easy solution actually… What do you think of that:

Remove the method as_mut_vec from String and instead add the method: unsafe fn as_mut_slice(&mut self) -> &mut [u8]. Vec has the same method, though in Vec's case it is not unsafe. In String's case it needs to be unsafe because this method could insert non-UTF8 content into the buffer. Both Vec and String have the unsafe method from_raw_parts. You can obtain those “raw parts” from both classes in a similar way: Both have the method capacity that returns the current capacity. Both will have (!) the method as_mut_slice returning a slice to the buffer (which contains the ptr and len). Additionally both classes have a method to return an immutable slice (Vec::container_as_bytes and String::as_bytes). Furthermore String already has two methods to unsafely convert from and into a Vec without copying the buffer (fn into_bytes(self) -> Vec<u8> and unsafe fn from_utf8_unchecked(bytes: Vec<u8>) -> String).

TL;DR: If I’m right, we just need to replace as_mut_vec with another similar method as_mut_slice to resolve all problems.

Do you (all) agree? If yes, I will try to setup an RFC.

[mzabaluev]

I have been playing with from_raw_parts and it’s not so simple. A library-provided RAII guard would be helpful for safer in-place mutation of String as a Vec. You can take a look at my attempt here.

[mzabaluev]

That, of course, is only helpful in certain scenarios like unsafe appending. In general, in-place-as-Vec is both needed and highly unsafe against potential unwinding.

[LukasKalbertodt]

Ok so I figured that my last “solution” to this problem was pretty useless. Adding as_mut_slice doesn’t really make sense.

@mzabaluev: I agree that from_raw_parts is not the safest thing to use. But I don’t think a RAII guard like the one you proposed does really make sense. Your code still relies on several assumptions about the internal implementation of String, which isn’t optimal. And again: I don’t think that it’s really useful to edit the string buffer in an alternating UTF8-safe and UTF8-unsafe way.

And whenever we want to work on the string buffer without UTF8 checks, we could do:

    let mut v = s.into_bytes();
    // do stuff with v
    s = String::from_utf8_unchecked(v);

And this is not really more work than as_mut_vec, right?

So my new proposal is: Just remove as_mut_vec. You can do everything it could do with other methods anyway. I don’t really like the whole std::string design anyway… But removing the method as_mut_vec is a step in the right direction, I think.

[leonardo]

as_mut_vec

What’s the status of the Small String Optimization? I think it’s a rather important optimization for Rust code. Most strings are quite short.

[oli-obk]

Reading the thread above it seems that the consensus is that due to the existance of &str, you can simply implement your own SmallString-Type that does the optimization and use it where it can help. Since there are few APIs requiring a String, you can just use your SmallString where you need it and create &str to it when interfacing with other APIs

[leonardo]

Since there are few APIs requiring a String, you can just use your SmallString where you need it and create &str to it when interfacing with other APIs

That sounds like a poor workaround. I have not read the whole thread so I don’t know what the problems are (beside from_raw_parts), but I think a basic optimization like this should be automatic. Most people that could gain from this optimization will not use a SmallString. Think about having to import a crate to optimize floating point operations…