icu4x
icu4x copied to clipboard
Add LiteMap to ZeroMap[2d] conversion functions
In #1058, I'm introducing code that converts from LiteMap to ZeroMap.
We should make this a impl From<LiteMap> for ZeroMap
in the zeromap crate, and we may as well do the other direction while we are at it.
Question: the conversion is infallible and fairly efficient if we use .try_append()
. How can we tell the compiler that, based on the invariants of LiteMap and ZeroMap, .try_append()
is infallible? We can have a .expect()
but that pulls in Debug
impls for the error and other stuff I'd like to avoid if possible.
Needs input from:
- [x] @Manishearth
@sffc ZeroVec and VarZeroVec have From
impls from slices, they are written to be efficient for this kind of use case. We should use them, they may require an additional method on the container traits.
We should not use try_append()
here.
@jwebbed is interested in this.
Which direction should the dependency go: zerovec to litemap, or litemap to zerovec? I think I prefer zerovec to litemap because litemap is a smaller, simpler crate. I can also see us eventually moving some of the ZeroMap-specific code and algorithms into the litemap crate since the two are very similar data structures.
I'd say the other way around (litemap depends on zeromap), as a feature. Litemap is smaller and simpler but it's specifically about maps and I think it makes sense for it to depend on zeromap.
I'd rather not have zeromap unconditionally depending on litemap; I'm really happy with it having a very small dependency footprint.
I don't think direct conversions add any value over just going through .iter().collect()
. I don't think there are any optimisation possibilities, because LiteMap
stores the tuples together, and ZeroMap
breaks them up, so we have to reconstruct all collections anyway.
- @echeran This seems reasonable
- @nordzilla We don't have good ways to construct ZeroMap
- @Manishearth We can say that you start with LiteMap and make a ZeroMap from that
- @echeran Sounds great
- @robertbastian Why not just go through an iterator
- @sffc LiteMap guarantees the values are sorted, so we can convert more efficiently
I think this is obsolete now. Datagen doesn't use LiteMap
s anymore, and we're tracking BTreeMap
to ZeroMap
in #2826
The BTreeMap construction path is great, and it would be nice if we could make it work for LiteMap, too. I don't think this issue is "obsolete" because it is still a big omission that the two custom map impls we maintain don't have an ergonomic conversion routine.