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

Add 'copy to array' method #52

Closed
pulquero opened this issue Sep 25, 2021 · 8 comments
Closed

Add 'copy to array' method #52

pulquero opened this issue Sep 25, 2021 · 8 comments

Comments

@pulquero
Copy link

I'm using the cyclicbuffer to maintain a fixed size history/lookback of samples. Every so often I need to process the contents of the buffer. Using the buffer directly is too slow, so I first copy out the contents to an array via [] operator. A dedicated copy-to-array method would be more optimal as for starters, the if statement to check index bound could be skipped. I think it would just need a loop to copy everything from head to end of buffer, and another loop to copy from start of buffer to tail. This would also avoid the expensive of having to do % operation.

@rlogiacco
Copy link
Owner

Feel free to submit a pull request along one or two examples/test programs

@rlogiacco
Copy link
Owner

On second thought I don't think I get your request, so can you please provide a code example of what's your usage?
Please consider the copy-to-array operation is going to double up your buffer memory consumption and starters will still have to loop and check they are not getting out of array boundary...

@pulquero
Copy link
Author

pulquero commented Oct 4, 2021

I have a polynomial operation, say O(N^3). Running directly on the buffer introduces alot of overhead. Yes, copying to an array increases memory consumption, but it is worth it for the reduction in time.

template<typename T, size_t S, typename IT>
void inline CircularBuffer<T,S,IT>::copyTo(T* out) const {
    const T* bufEnd = buffer + capacity;
    const T* outEnd = out + count;
    for (const T* t = head; t < bufEnd && out < outEnd; t++, out++) {
        *out = *t;
    }
    for (const T* t = buffer; t <= tail && out < outEnd; t++, out++) {
        *out = *t;
    }
}

template<typename T, size_t S, typename IT>
template<typename R>
void inline CircularBuffer<T,S,IT>::copyTo(R* out, R (&convert)(const T&)) const {
    const T* bufEnd = buffer + capacity;
    const R* outEnd = out + count;
    for (const T* t = head; t < bufEnd && out < outEnd; t++, out++) {
        *out = convert(*t);
    }
    for (const T* t = buffer; t <= tail && out < outEnd; t++, out++) {
        *out = convert(*t);
    }
}

@rlogiacco
Copy link
Owner

Do you want to submit a pull request so your contribution gets traced and you get credited?
Also a usage example and some documentation for the README would be nice.

@pulquero
Copy link
Author

pulquero commented Oct 5, 2021

No, it's fine. Probably quicker if you just do it. You can probably fix-up the 2nd method to accept lambdas too.

@rlogiacco
Copy link
Owner

rlogiacco commented Oct 11, 2021

Ok. Thanks for your contribution.

Quick question: are you 100% sure the two separate loops are faster than the modulus? I'm not assembly guy...

@pulquero
Copy link
Author

Yes, modulus is going to be implemented in software - most MCUs dont have an instruction for it.

@rlogiacco
Copy link
Owner

resolved in release 1.4.0

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

No branches or pull requests

2 participants