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

SWT speed up... #58

Open
wants to merge 7 commits into
base: unstable
Choose a base branch
from

Conversation

mledford
Copy link

@mledford mledford commented Mar 6, 2013

I removed a subtract and a multiply from calculating the memory location by using a pre-calculated lookup table. When testing I found this decreased the processing time of up to 35%.

@mledford
Copy link
Author

I have found some bugs in the implementation. I am looking to see if it's something that can be corrected.

@mledford
Copy link
Author

I made some bad assumptions about the algorithm and so most everything in this pull request is invalid. The speed ups I saw were due to short circuiting the algorithm. One of the commits should still be valid where I reordered the memory accesses but is rather small on whole.

@mledford mledford closed this Mar 10, 2013
… memory location by using a pre-calculated lookup table."

This reverts commit c1c258e.
… location by using a pre-calculated lookup table."

This reverts commit 10ed690.
@mledford
Copy link
Author

After my embarrassing last try I looked at it again and managed to improve performance while faithfully maintaining the algorithm's integrity. Hopefully I got it right this time. I have reopened the pull request and hopefully the new commits will show up.

Basically there are two tight loops in ccv_swt() that do memory location calculations. It so happens that these loops can be modified in such a way that part of the memory location can be pre-calculated and then use a lookup table with some simple addition to gain some speed.

@mledford mledford reopened this Mar 11, 2013
…d a memory access violation. Accidentally used the wrong variable as the base of the offset. Also renamed an offset variable to properly reflect its origin.
@asinteg-daehn
Copy link

+1

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

Successfully merging this pull request may close these issues.

2 participants