search

mgeier / rtrb

A realtime-safe single-producer single-consumer (SPSC) ring buffer
Apache License 2.0
202 stars 14 forks source link

Weird allocation method #67

Open RamType0 opened 3 years ago

RamType0 commented 3 years ago

RingBuffer was allocated by allocating Vec, then extract pointer, and forget Vec. Just using alloc::alloc::alloc(Layout::array::<T>(capacity).unwrap()) seems to be more natural. Why is this done?

mgeier commented 3 years ago

That's a good question.

The simple answer is that I have just taken it from https://github.com/crossbeam-rs/crossbeam/pull/338.

But why was it done there?

I could imagine that at the time this was written, some functions were just missing. For example, the Layout::array() function was added in Rust 1.44, while this crate still supports 1.36. It would still have been possible to implement it with alloc::alloc(), but it would probably have been quite a bit more complicated.

I'm open to changing this at some point, but right now I don't think it's worth it.

I'm considering to implement the RingBuffer as a dynamically sized type at some point (not very soon though), which would get rid of an indirection (but I'm not sure if that would improve performance). This would probably also allow to (optionally) pass in an existing piece of memory (instead of automatically allocating). For this it would probably also make sense to avoid the additional allocation of the Arc. And I guess for all this, the Vec-based thing wouldn't work anymore and would have to be replaced anyway.

What do you think about this?

RamType0 commented 3 years ago

I'm considering to implement the RingBuffer as a dynamically sized type

Seems to be good. !Sized trait for RingBuffer will not pain because we will access RingBuffer through Arc in most cases.

Also, unsized locals will solve many issues around !Sized.

mgeier commented 3 years ago

we will access RingBuffer through Arc in most cases

Exactly, that's one of the reasons why I removed the .split() method in #57.

Right now, the RingBuffer cannot be owned by anyone, only borrowed. So it should be possible to change it to !Sized (but this might still strictly speaking be a breaking change?).

RamType0 commented 3 years ago

So it should be possible to change it to !Sized (but this might still strictly speaking be a breaking change?).

What users could see in RingBuffer is just capacity. IMHO, almost no one cares about the structure RingBuffer itself.

mgeier commented 2 years ago

I've created a possible implementation of RingBuffer as DST: #75.