Proposal: Make `EcoVec` single-threaded by default for better performance

[pedrodesu]

Context

Currently, EcoVec uses an AtomicUsize in its Header to track reference counts unconditionally. This makes EcoVec thread-safe (Send/Sync), but it incurs a performance overhead due to atomic instructions (fetch_add/fetch_sub) and cache synchronization, even when used in entirely single-threaded contexts.

With the eventual support for custom Allocators - assuming we will be mirroring Vec's decision - EcoVec will be able to be used with stateful local allocators (like bump or arena allocators) that are themselves also single-threaded (!Send/!Sync). Forcing an atomic reference count on the collection in these scenarios is wasteful.

Proposal

I propose making EcoVec single-threaded by default to maximize performance for standard workloads, while still supporting multi-threaded contexts. We could achieve this by, for example, gating the atomic features via a Cargo feature (such as sync) or by splitting into two distinct types. I'd suggest the former.

Impact

For instances such as single-threaded layout engines, parsers, or single-threaded compiler stages, replacing atomic operations with raw Cell mutations will remove memory synchronization overhead and unlock significant performance improvements during cloning and dropping.

I'd love to hear the maintainers' thoughts on which architecture fits the project's direction best.

[laurmaedje]

My feelings go a bit in a similar direction as rust-analyzer/smol_str#37 (comment), though I'm not 100% fixed on this. This also came up on Reddit before.

[pedrodesu]

I think there's a relatively elegant way to achieve this without complicating the codebase or introducing a sprawling hierarchy of different structs. We can handle this purely via a Cargo feature flag + conditional compilation.

By default, we could use a raw Cell<usize> for single-threaded performance, and if the feature is enabled, conditionally type-alias it with AtomicUsize and using conditional compilation wherever necessary (for instance for the unsafe impls of Sync and Send). This keeps the API completely uniform and leaves the core code virtually unchanged, avoiding duplication.

To me, squeezing out this extra performance feels like a no-brainer. Unnecessary atomic reference counting incurs a notable cache-synchronization penalty. Implementing this simple change means potentially removing a bit of the "a bit of overhead for mutation" (in the uniqueness checking part) that the README talks about. Since this library is meant to be highly reusable and generic, we shouldn't assume that a single-threaded, zero-sync use case isn't valid or worth optimizing for - in fact, one could make a similar case about how much more complex (and rightfully so!) the code becomes just to avoid single indirection. Providing a simple compilation toggle lets users opt into that performance boost effortlessly. It's seamless to use and should be just as easy to maintain.

These are my 2 cents on the matter. I can whip up some benchmarks to reveal how big of a difference it might or not make. If you still feel like it's better to keep the thread safety hard-coded, that's fair. If not, I can draft a quick PR.