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

Array ring buffer wastes capacity #320

Open
franz1981 opened this issue Nov 18, 2024 · 4 comments
Open

Array ring buffer wastes capacity #320

franz1981 opened this issue Nov 18, 2024 · 4 comments

Comments

@franz1981
Copy link
Collaborator

After eons I wanted to reuse this structure for http 2 stream handling in Netty and..

Doesn't seem to account for gaps 😕

i.e.

  1. Add 3 elements
  2. Remove with index the first 2 inserted: head sequence is still there but we have null out two slots
  3. Remove the last and observe that we won't get back anymore the full capacity although all 4 elements are gone

Remove should keep on checking bubbles of nulls to move the head as much as it can, till the tail, if necessary (or stopping at the first non null).

@jabolina
Copy link
Member

Hello, @franz1981. We've also included it on ISPN recently (infinispan/infinispan#13016). I think Will has also added a few modifications.

@franz1981
Copy link
Collaborator Author

Nice one, but the issue is valid? I have read it on the phone since today I couldn't use the computer, so I could be very wrong

@wburns
Copy link

wburns commented Nov 18, 2024

Yeah, I implemented this in our version of the ArrayRingBuffer as I didn't want to deal with the empty space. Check the catchUpHeadPointer method on the PR above.

Maybe there is a more efficient way of doing it, instead of having the while loop, but it seems to work as it is.

@franz1981
Copy link
Collaborator Author

franz1981 commented Nov 20, 2024

@wburns
I have this version I implemented for Netty, simplified: https://gist.github.com/franz1981/6277af990ccfe0a5da8542a75d66ce5e

let me know if it can be of any help

and thanks for infinispan/infinispan#13016: has made my day, seeing this little one been used elsewhere ❤️

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

No branches or pull requests

3 participants