Conversation
Yep! The partial decoder for the sharding codec retrieves and decodes the shard index only once. However, the partial decoder cache should only live as long as the batch of requests in
So my recommendation is to avoid locks and:
|
|
Separately there seems to be an error about |
|
Yup, https://github.com/zarr-developers/numcodecs/blob/58bc9c4da74e06fbec985d4235f912b31d4d31f6/numcodecs/abc.py#L79-L95 means it is now part of the configuration. So we can probably ignore it, will open a separate PR |
|
Seems like I did 1. by constructing a |
| let mut item_map: HashMap<StoreKey, &WithSubset> = HashMap::new().into(); | ||
| chunk_descriptions | ||
| .iter() | ||
| .filter(|item| !(is_whole_chunk(item))) | ||
| .for_each(|item| { | ||
| item_map.insert(item.key().clone(), item); | ||
| }); |
There was a problem hiding this comment.
I think there is a cleaner way to do this...
LDeakin
left a comment
There was a problem hiding this comment.
Nice! Just a minor nit on Option -> Result + ensuring that the chunk concurrent limit is used.
Co-authored-by: Lachlan Deakin <[email protected]>
Co-authored-by: Lachlan Deakin <[email protected]>
Co-authored-by: Lachlan Deakin <[email protected]>
Fixes #90 (hopefully) - I benchmarked and performance (at least relatively speaking) did not go down, but also did not go up, although our benchmarks are probably not really checking for this behavior.
I went with
HashMapbecause I couldn't see a reason to useBTreeMap(I see it's used elsewhere and that people in rust seem to use it as a drop-in replacement forHashMapbecause it's faster sometimes? Not really familiar with this TBH).I am also not sure the
keyis the right thing to hash on - the "key" here would be a shard, and we want to share the decoder per shard, right?