djg / chunky-vec

A pin safe, append only vector never moves the backing store for an element.
Apache License 2.0
1 stars 0 forks source link

Removal #2

Open zbraniecki opened 3 years ago

zbraniecki commented 3 years ago

How can one remove/update an element in the ChunkyVec?

I need to implement it for https://searchfox.org/mozilla-central/source/intl/l10n/L10nRegistry.jsm#246-276

djg commented 3 years ago

I feel like for safety aspects I should check with @Manishearth, but the intention is that ChunkyVec is mostly push only. I copied this from the elsa types.

Since the storage for ChunkyVec is chunked, and not a Vec, it should be okay to replace items already allocated as well as supporting pop. removeSources() in L10nRegistry.jsm might be a bit hard to implement given the constraints. This is where the elsa types keep the element boxed so it's address is stable and it's possible to then re-arrange the underlying Vec.

Alternatively, is it possible to store Option<T> in the ChunkyVec? Removal would then None the element and source checking logic would need to skip those.

WDYT, @zbraniecki?

zbraniecki commented 3 years ago

Using Option would mean that for i in 0..10 { registerSource;removeSource;} would result in 10 element ChunkyVec with empty elements. I'd prefer to avoid that if possible.

Do you think that this is the best we can do with ChunkyVec?

djg commented 3 years ago

Do you think that this is the best we can do with ChunkyVec?

At the moment, unfortunately, yes, with out adding logic inside ChunkyVec so that it becomes more like slab.

Would it be possible to add the add/remove functionality into a thin wrapper over ChunkyVec<Option<T>> type in L10nRegistry? (Or can we add impls only for ChunkyVec<Option<T>>? I'd be open to that, unless someone with more Rust soundness experience tells me it's wrong)

Manishearth commented 3 years ago

Yeah append-only collections have a lot of good properties for performance and safety, deletion makes it harder. slab is indeed a good option here

Thin wrapper is another.