Possible niche optimization

[fgardt]

It's possible to get Option<ColdString> to be the same size as ColdString by informing the compiler about the available niche of 0 (unless I missed something and 0 is a valid value?).

Can be achieved just by changing

pub struct ColdString([u8; WIDTH]);

to

pub struct ColdString {
    ptr: NonNull<usize>,
}

And then updating all other areas accordingly.

[dtolnay]

0 is a valid value for "\0\0\0\0\0\0\0\0".

[tomtomwombat]

Thanks, for the suggestion. Unfortunately, this would cause the alignment of ColdString to be 8, since NonNull's alignment is 8.

[dtolnay]

The alignment can be reduced easily with repr(packed).

[tomtomwombat]

The alignment can be reduced easily with repr(packed).

Ah cool. I guess that means we can use *mut u8 to leverage the strict provenance APIs, and repr(packed) to keep the alignment at 1. And as already stated, NonNull won't help with niche optimizations since "\0\0\0\0\0\0\0\0" is valid UTF-8 (A valid in-line representation ColdString).

[dlight]

What about finding another representation for "\0\0\0\0\0\0\0\0"? Since there's a lot of unused bits patterns due to #1

In an ideal world, Rust would allow you to explictly mark which byte ranges are invalid, rather than forcing the use of NonNull / NonZero

[tomtomwombat]

@dlight in the current implementation, "\0\0\0\0\0\0\0\0" would be the representation for 8 in-lined bytes, and needs to remain as such so that the as_str(&self) -> &str implementation is possible (I don't think you can decode the UTF-8 representation and then return a borrow of the decoded...?).

Before #1 in the old representation where only 7 bytes were inlinable, this would be possible. So the trade-off here is a niche optimization for Option vs 1 less inlinable byte.

BTW, mem::size_of::<Option<ColdString>>() == 9, due alignment, so an Option is not such a huge overhead, but it's still a shame it's not currently niche optimizable.

[boomshroom]

A string of 8 nulls is likely a lot less common than a None Option variety. It might make sense to just special-case it to always allocate with this specific string. (and "\0\0\0\0" on 32-bit platforms). Whether accommodation this specific case would be worth the code-size increase, I'm not sure. (Performance should barely be affected considering it's extremely unlikely for the path to get taken.)

BTW, mem::size_of::<Option>() == 9, due alignment, so an Option is not such a huge overhead, but it's still a shame it's not currently niche optimizable.

This probably would have a higher performance penalty than checking for a string of nulls.

[tomtomwombat]

A string of 8 nulls is likely a lot less common than a None Option variety.

ColdString should, by default, work with all UTF-8, otherwise this would be a huge usability pain.

It might make sense to just special-case it to always allocate with this specific string.

Behind a feature, maybe. Is the compiler guaranteed to see this as niche-optimizable though?

If not, then users of ColdString could just use their own sentinel value of ColdString (instead cold string natively having it's own).

This probably would have a higher performance penalty than checking for a string of nulls.

ColdString is prioritizing size and usability over speed. The point here is that Option is just 1 byte of overhead vs the expected 8.

[luksan]

Heh, I came up with #8, and then I found this issue. :)

[tomtomwombat]

#8 adds a niche optimization. @dlight @boomshroom I realize now what your were suggesting was pretty simple and didn't complicate things much, it's what @luksan added in his PR. Thank you!