Featue: making len atomic

[cehteh]

I would like to know if you are interested to merge a rather big slightly breaking change:

The .len field could be atomic with:

• add unsafe fn HeaderVecHeader::len_add_release() incrementing the length being a atomic op
  This is unsafe because one has to hold a promised contract, see below
• making HeaderVecHeader::len(&mut self) -> usize (note the mut, this is a breaking change)
• adding unsafe fn HeaderVecHeader::len_acquire(&self) -> usize this is what .len() currently does, but using atomics

Alternative without breaking changes:

• using the atomic op for .len(&self) under the hood, keeping the compatible API
• adding a HeaderVecHeader::len_noatomic(&mut self) -> usize // or other name
  Disadvantage is that the .len() API which is what users most likely want to use/expect would have to pay the atomics cost on some platforms.

Rationale:
One can add elements within capacity and then when done increment the len atomically as long the vec stays within capacity. But adding elements needs some synchronization otherwise this will be racy. The len_add_release() is unsafe because of this. The actual synchronization is deliberately not part of the implementation here and has to be done somehow else.

Currently I roll my own allocation very similar to what this crate does in https://crates.io/crates/cowstr I would like to externalize this to another crate and yours fits my needs except for the above changes. When you like the idea I will make the proposed changes and send a PR.

[vadixidav]

I agree that it is good to have a single implementation in the ecosystem where possible. It almost seems like two different use cases appear when attempting to separately update the length. I understand that any case where this crate is used is almost certainly going to be highly performance sensitive, so being able to update that length with utmost performance is always important. I believe the right solution is likely to have two separate types, one for each of your proposed solutions. The user will choose the one that is more appropriate for their use case. I can address this soon.

If you already have the changes available or wish to contribute them, I would be happy to accept them. It is okay to make a breaking change so that each implementation has an appropriate name as needed.

[cehteh]

dunno if you really need 2 implementations because AtomicBool has .get_mut() which allows you to mutate the the atomic in a optimized non atomic way that is pretty well optimized. Only for getting the len() it needs a atomic and a non atomic variant, the actual naming might be bikeshedding. Your decision if that qualifies to make a complete new variant. IMO it would benefit to have both in once because under certain conditions (like one has a &mut self) non atomic access is always possible while for the same kind of object one has a shared variant where the length needs to be accessed atomically. I also deliberately left out a atomic decrement of the length. as this definitely will be racy.

[vadixidav]

I might actually need to see what the changes would look like then. I am willing to merge regardless, but if we need to create two variants I can do that myself if you check the checkbox in the PR that says "allow maintainers to make edits."

[cehteh]

On Mon, 16 Sep 2024 21:02:34 -0700 Geordon Worley ***@***.***> wrote: I might actually need to see what the changes would look like then. I am willing to merge regardless, but if we need to create two variants I can do that myself if you check the checkbox in the PR that says "allow maintainers to make edits."

When one doesn't use the atomic-append parts then the normal operations have Relaxed ordering which is free or cheap at least AFAIK on all platforms supporting atomics. Even with using the atomic append parts one can still use these relaxed operation because in many cases the race here, that a append isn't immediately visible in rare cases won't cause any (memory-safety) problems. The way I am doing it now is that the API and performance beehavior stays *mostly* the same. atomic append is a opt-in and still very cheap, but it will come with some unsafe API/contract. This is important because it leaves the actual synchronization to the user. In cowstr this is a single AtomicBool in the header that flags that some thread has a 'Guard' which can be used for appending, that much cheaper than a Mutex for example. In other cases one may put a mutex or a RwLock there. I can (actually will becasue it's easy to do) put the atomic append stuff behind a feature flag that is disabled by default. Since this crate is no_std and there are some embedded platforms that do not support atomics this is prolly the best way to go.