michaeltyson / TPCircularBuffer

A simple, fast circular buffer implementation
http://atastypixel.com/blog/a-simple-fast-circular-buffer-implementation-for-audio-processing/
839 stars 141 forks source link

No need for the atomic operations. #13

Closed al-skobelev closed 7 years ago

al-skobelev commented 7 years ago

Just a hint: you can get rid of fillCount and atomic operations if you keep head and tail as they are and reduce them to the buffer size only when returning pointers.

clobber commented 7 years ago

@al-skobelev Do you have an implementation of this? Might be a good pull request.

aleksandr-skobelev-epam commented 7 years ago

@clobber I didn't use TPCircularBuffer but I looked into its code when implemented my own version of a circular buffer.

michaeltyson commented 7 years ago

Yup, you're correct, but I made the decision to keep the full count as a utility. There are a number of cases where it's useful to know how many bytes are currently stored, and for that you need atomic operations.

michaeltyson commented 7 years ago

(Uh, fill count, not full count! Damn you, Siri!)

aleksandr-skobelev-epam commented 7 years ago

Just wondering, is returning (write position - read positon) not enough in that case?

michaeltyson commented 7 years ago

Ah! Yes I think you're right, although I'd need to be a lot better slept to be certain that's safe. No, I remember now, the reason I went with a fill count is because if you just have head and tail indices you can only use bufferSize - 1 bytes to avoid full/empty ambiguity. So, more convenient to avoid that complexity. At least I think that's why - been many years!

pietrasm commented 7 years ago

I think that wouldn't be correct since both head and tail are going to be accessed from both threads, so you would have to at least use atomic_load and atomic_store. However, you could do it without using atomic_fetch_add and atomic_fetch_sub since both variables are going to be modified from a single thread only. I am not sure if it's going to be any faster to perform atomic_load, increment or decrement the variable, and perform atomic_store instead of just using atomic_fetch_add or atomic_fetch_sub, I guess that depends on the platform. Moreover, you would have to deal with the full/empty ambiguity mentioned above by @michaeltyson.

aleksandr-skobelev-epam commented 7 years ago

While both head and tail are accessed from both threads, they are changed only from only its own dedicated thread: head is changed in Produce thread and tail is changed in Consume thread. So there is no need in atomic operations at all. If the use case for TPCircularBuffer supposes the use of TPCircularBufferProduceBytes which checks if there is enough space in the buffer, then there never be the full/empty ambiguity, as the buffer is empty only when head == tail. If the buffer is not empty then always head > tail.

Update: This seems to be correct if we read/write operations are atomic. If we use 64-bit indices it is correct for x86_64 and, perhaps, not for ARM?