Cyotek.Collections.Generic.CircularBuffer icon indicating copy to clipboard operation
Cyotek.Collections.Generic.CircularBuffer copied to clipboard

Copying Large buffer

Open shauleiz opened this issue 6 years ago • 3 comments

I'm currently using the original CircularBuffer for one of my projects. Since it seemed to be "mostly dead" I searched for a follow-up and found this. Good work! I wonder if for copying large buffers into/out the Circular Buffer it will perhaps be better to call static method Array.Copy, rather than loop on the array members? What is do you think?

shauleiz avatar Oct 10 '17 14:10 shauleiz

Hello,

As Array.Copy ultimately is native code I believe, there probably would be a performance gain, although it may need two different calls to account for the wrapping behaviour. However, it's definitely worth a look. (Someone previously suggested using Buffer.BlockCopy but as that works by copying individual bytes I think it's probably overkill for the complexity that would be involved)

I need to update the package anyway as the very latest version is marked as pre-release which is causing me no end of issues when referencing it in my own packages so it's high time that flag is removed. I can't promise anything timely (I still need to update another project for use with VS2017) but I will definitely add it to the list of things to do. I'll start by writing a benchmark and seeing if it really will help, and if it does then I'll get it implemented properly.

Thank you for the suggestion!

Regards; Richard Moss

cyotek avatar Oct 10 '17 16:10 cyotek

Hi Richard and thanks for the quick reply. Yes, I am aware of the need to call this method twice when crossing the wrapping boundry. As for eficency studies, there is a good discussion here. Search for entry by "prelude". Once I moved to your code I will try these changes but this will take time. I also plan to implement a "peek" method that copies n enntries from the tail side while living the buffer unchanged.

Regards Shaul Eizikovich

shauleiz avatar Oct 10 '17 16:10 shauleiz

Hello,

Thanks for the reference, that was interesting reading.

The class actually does have Peek methods, although these work from the head. The last release did add a PeekLast method to work from the tail but unlike Peek there isn't an overload which allows you to specify the number of elements to return, if you wish to submit a PR for that I'm sure it would be welcome.

If you do migrate to using this library it should be fairly painless, but let me know if you run into any problems.

Regards; Richard Moss

cyotek avatar Oct 10 '17 18:10 cyotek