search

ParkMyCar / compact_str

A memory efficient string type that can store up to 24* bytes on the stack
MIT License
557 stars 41 forks source link

Add support for holding Arc<str> as a new repr? #355

Open NobodyXu opened 5 months ago

NobodyXu commented 5 months ago

I think it makes sense to support Arc<str> in CompactString as a new repr, it would then enable O(1) cloning, which is very handy for parser/configuration.

dragazo commented 4 months ago

Just noting that if this gets added, there would need to be a fallback to Rc<str> on systems without #[cfg(target_has_atomic = "ptr")] (docs). Or maybe better, support Rc<str> on all platforms and just conditionally also support Arc<str> on platforms that allow it.

NobodyXu commented 4 months ago

Thx, I forgot existence of Rc<str> and problems of atomics not present for a while.

While I agree it'd be a good idea, supporting Rc<str> means CompactString would no longer be Send.

We can conditionally support Arc<str> on platforms that support it, and for other platforms, IMO it's Arc<T>'s job to fallback to Rc<T> implementation, because we don't know if it's safe to fallback to it (only when the program is single-threaded).

dragazo commented 4 months ago

Ah, that's true, and a fallback to Rc<str> would also break Send. In that case, it might be best to just support Arc<str> when it exists on the arch and just not allow that (with no fallback) otherwise.