Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add const-erased versions of the various containers #371

Open
sosthene-nitrokey opened this issue Jul 12, 2023 · 2 comments
Open

Add const-erased versions of the various containers #371

sosthene-nitrokey opened this issue Jul 12, 2023 · 2 comments

Comments

@sosthene-nitrokey
Copy link
Contributor

Making code flexible around heapless structures can be a bit verbose because it requires being generic over the capacity of each collection. This could be helped by providing versions of the structures with the const erased. This would allow removing the generics in some cases.

For example, heapless::Vec currently is :

pub struct Vec<T, const N: usize> {
    len: usize,
    buffer: [MaybeUninit<T>; N],
}

It should be possible to provide a struct that goes hand in hand with it:

pub struct VecView<'a, T> {
    len: &'a mut len,
    buffer: &mut [MaybeUninit<T>],
}

impl<T, const N: usize> Vec<T, N> {
    fn as_view(&mut self) -> VecView<'_, T> {
        VecView { len: &mut self.len, buffer: &mut self.buffer }
    }
}

With VecView implementing most of the API that Vec currently implements. This would allow for example methods that write data to an outbuffer of type &mut Vec<T, N> to instead take a parameter VecView<'_, T>.

Something like:

fn do_something<const N: usize>(response_buffer: &mut Vec<u8, N>)

could become

fn do_something(response_buffer: VecView<'_, u8>)

In a trait for example this would also have the advantage of making it object-safe.

I believe this pattern:

  1. Improve ergonomics
  2. Might reduce compilation time and binary size because of monomorphisation (unverified)
@sosthene-nitrokey
Copy link
Contributor Author

sosthene-nitrokey commented Jun 26, 2024

Types:

IndexMap and IndexSet

IndexSet depends on the View type for IndexMap being available.

IndexMap is currently implemented with a CoreMaptype that looks like:

struct CoreMap<K, V, const N: usize> {
    entries: Vec<Bucket<K, V>, N>,
    indices: [Option<Pos>; N],
}

It is not possible to construct a View for this type like for Vec since the N would have to be erased twice.

We could solve that issue by "inlining" the indices into the entries:

struct CoreMap<K, V, const N: usize> {
    entries_indices: [(MaybeUninit<Bucket<K, V>>, Option<Pos>); N],
    len: usize,
}

Then only one N needs to be erased, and this can be implemented. However this requires changing the implementation significantly.

It might even be possible to get rid of the len field (I think an entry is initialized if and only if a index points to it, and only one index can point to it at a given time?).

@sosthene-nitrokey sosthene-nitrokey changed the title Add const-erased versions of the various Add const-erased versions of the various containers Jun 27, 2024
@sosthene-nitrokey
Copy link
Contributor Author

There are cases where some adjacent structures have the const generic that could be removed thanks to this implementation. Would it be worth making breaking changes to remove this const generic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants