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

bbqueue use on shared memory #67

Open
timvisee opened this issue Jul 17, 2020 · 11 comments
Open

bbqueue use on shared memory #67

timvisee opened this issue Jul 17, 2020 · 11 comments
Labels
question Further information is requested

Comments

@timvisee
Copy link

I'm looking into using this on shared memory for IPC. Do you have any input on this, whether this might work or not?

I couldn't really figure out whether the actual buffer is part of the struct (ConstBBBuffer), or whether it uses a pointer. If it is part of the struct, I think copying/transmuting the buffer onto shared memory would be alright, wouldn't it. Sorry for the possibly vague message, I'm somewhat uneducated on this topic.

Also, if this can easily be done, providing (unsafe) functions to initialize and attach to a bip buffer might be useful. I believe using a bip buffer on shared memory is quite a common use case.

@mattico
Copy link

mattico commented Aug 11, 2020

Yes, the actual buffer is part of the struct, it's the buf field where the A type parameter is for a GenericArray<u8, N> where N is a type-level integer which determines the size of the array.

I'm trying to do a similar thing, making the buffer use a specific SRAM at a specific address. I think I could do this with linker sections, but I don't want to use any of the SRAM memory for the bookkeeping variables.

I'm going to try to hack in the ability to create a bbqueue using a pointer+length. Hopefully we can figure it out and get it upstreamed.

@jamesmunns
Copy link
Owner

I'm not 100% sure this would be a great idea with bbqueue as-is, though it could potentially be possible with the correct cross-process synchronization. I admit to not know what is necessary to achieve this, at least on hosted (Windows/Mac/Linux) platforms.

Currently, yes, the storage is part of the struct.

@jamesmunns jamesmunns added the question Further information is requested label Sep 15, 2020
@timvisee
Copy link
Author

timvisee commented Sep 16, 2020

I did some experimentation, and it seems to work quite well (easily reaching 280Gbps through it across processes if you're wondering). I'm just placing the BBBuffer struct on shared memory, and am dereferencing it in a different process. This involves some unsafe logic of course.

The only real problem I have is accessing the producer and consumer. You can't split twice, even though you end up using the producer and consumer once. This is because of the already_split flag.

Having two separate flags (for the producer and consumer) and an additional two functions would solve this. How do you think about this, can I implement this change? (this is of course something quite specific for shared memory usage)

@jamesmunns
Copy link
Owner

@timvisee sure! I have an issue for that already actually: #40

I'd definitely take a PR for this.

@timvisee
Copy link
Author

@timvisee sure! I have an issue for that already actually: #40

I'd definitely take a PR for this.

I've submitted a draft PR for this, see: #78

Wasn't aware of that issue, thanks for linking.

timvisee added a commit to timvisee/bbqueue that referenced this issue Nov 12, 2020
@jamesmunns
Copy link
Owner

Hey @mattico and @timvisee, I'm currently working on the "Next Gen" version of BBQueue with const generics, and I was wondering if you would be okay with the "bring your own memory" constructor to require &mut [u8; N], instead of &mut [u8]. This would allow me to keep all of the array lengths compile-time known, which I think would reduce some potential overhead.

I wanted to see if this was acceptable for your use case, or whether you would prefer to have a &mut [u8] constructor.

@timvisee
Copy link
Author

timvisee commented Jan 4, 2021

I'm currently not actively working on a project that uses bbqueue, and I'm not sure it that would be acceptable.

I do feel that using a fixed size array can be quite annoying to work with. This next gen version sounds great though!

Would it be an option to support both? Or would that increase complexity by a lot.

@jamesmunns
Copy link
Owner

jamesmunns commented Jan 4, 2021

Right now I'm looking at the trait that would abstract over storage, and it seems that I have two options:

  • Always use *mut [u8; N], which means I can always keep the array length const-knowable
  • Always use my own "cell" that tracks a (*mut u8, usize), which means const-knowable items must always create this intermediary cell

I honestly don't know what kind of overhead this would introduce in practice, but I can't think of a way to "mix and match" these two approaches. The first option would look something like this:

trait BBGetter<const N: usize>: Clone {
    fn get_header(&self) -> &BBHeader;
    fn get_storage(&self) -> NonNull<[u8; N]>;
}

/// A backing structure for a BBQueue. Can be used to create either
/// a BBQueue or a split Producer/Consumer pair
pub struct BBBuffer<const N: usize, STO>
where
    STO: BBGetter<N>,
{
    buf: STO,
    hdr: BBHeader,
}

The only use case I can think of for the latter option would be a case where you need to create a bbqueue of a size that is determined at runtime, e.g. you ask the user to input "how large would you like your bbqueue"?

I think for all use cases where the size should be statically knowable, it would be reasonable to have a Try constructor (or helper function) that checks if (&mut [u8]).len() is >= N, and returns an error in the case it is not. Basically something like this:

fn try_to_array<const N: usize>(buf: &mut [u8]) -> Option<&mut [u8; N]> {
    if buf.len() >= N {
        Some(unsafe { &mut *buf.as_mut_ptr().cast::<[u8; N]>() })
    } else {
        None
    }
}

Or something to that extent, which I think should be sound.

EDIT: It seems like this behavior already exists as part of TryInto?

EDIT2: Yup, it already works.

@jamesmunns
Copy link
Owner

So after talking to @Dirbaio, I realized a second motivating factor for using the "Cell" approach: This would likely allow for reduction in monomorphization bloat, if you use the the "borrowed storage" variant, which might be useful for some folks.

I'll probably plan on doing that for now. Thanks for the feedback @timvisee!

@timvisee
Copy link
Author

timvisee commented Jan 4, 2021

This would likely allow for reduction in monomorphization bloat, if you use the the "borrowed storage" variant, which might be useful for some folks.

Is such bloat really an issue when you might only use a single array type (meaning the same size for all bbqueue's), or am I missing something here? I'm not too familiar with that topic in context of generics.

@mattico
Copy link

mattico commented Jan 4, 2021

Appreciate you working on improving this @jamesmunns! My use case uses only fixed size arrays of known size. You could use an inner function to work around the monomorphization bloat, perhaps. You'll have a clearer idea of the implementation tradeoffs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants