Add values_as_slice() to DenseSlotMap - #81
Conversation
|
After some thought, another option would be to publicly expose the |
|
Sorry for the late reply, this kind of slipped. I took a look at your pull request, which contains two separate things (which really should be separate):
I don't think I want to do (1) because it can't be done for I'm a bit hesitant to add (2) because it might lead to bad assumptions, but I do recognize the usefulness. If I were to add it, I think I would add two methods, |
I'm opening this PR not because it's the final proposal, but I wanted to start a discussion at least.
I have some performance sensitive code that currently takes a slice of data to ensure that the iteration it's doing is linear in memory. I don't want that code to accidentally receive an iterator that's not linear in memory, so taking an argument along the lines of
impl Iterator<Item = X>is a bit of a no-go.On top of that, the performance sensitive code is behind a dyn trait, so even if I wanted to, I think it'd be impossible to enforce linearity through a trait there. What do the project maintainers think of exposing the values list of a
DenseSlotMapas a slice directly? Obviously this should come with the caveat that one can't depend on the order of items in that slice, and that one shouldn't store indices into the slice anywhere (I'd be OK for example with marking this asunsafe).Additionally, I found some odd code where the Values iterator also seems to require a) the Key type and b) not forward directly into std::slice::Iter but through the other iterator and
map'ing out the key values, seems like we're leaving some performance on the table there potentially though I didn't do those measurements yet. If desirable I can move those changes into a different PR.