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

Code optimization: 30% speedup #6

Merged
merged 20 commits into from Aug 13, 2011
Merged

Code optimization: 30% speedup #6

merged 20 commits into from Aug 13, 2011

Conversation

gioele
Copy link
Contributor

@gioele gioele commented Aug 13, 2011

Hello,

I refactored a bit of code of RQRCode code in order to make it faster. The refactored code pass all the test in test suite.

I have an additional commit (f4de8484122ecd1271945b159f22ab99e09d53ce) that would bring another 20% speed up, but it fails two test. I'm investigating whether the older test code was wrong or the new refactored code is to blame.

Please pull these changes, there are also some little bug fix inside (for example, the original code did not take level 4 demerit score into account).

Code such as

	case mask_pattern
	when QRMASKPATTERN[:pattern000]
		(i + j) % 2 == 0
	when QRMASKPATTERN[:pattern001]
		i % 2 == 0
	when QRMASKPATTERN[:pattern010]
		j % 3 == 0

is very slow because it requires an `Hash#[]` lookup and a `Kernel#===`
call for every branch of the `case` construct, 3.5 * 2 function calls on
average.

We exploit the fact that `mask_pattern` is a Fixnum and pre-populate an
array of possible computations. This takes exactly one `Array#[]`
lookup and `Proc#call` execution per mask pattern.
@gioele
Copy link
Contributor Author

gioele commented Aug 13, 2011

An additional note: the last commit 52c9caf is there so that profilers can understand which part of the code is taking longer to compute. Please keep it as other code builds on top of it.

@whomwah
Copy link
Owner

whomwah commented Aug 13, 2011

This is awesome work, thankyou! When I get time, I'll pull your changes and double check the tests. All being well I'll merge.

Ref your extra commit. Investigation would be good. As you may know this code is a port of a JavaScript encoding lib and I have never seen the real QRCode spec. The only way I have been able to check things is whether the QRCodes read, so I'm not sure how much the error correction is masking problems.

whomwah added a commit that referenced this pull request Aug 13, 2011
Code optimization: 30% speedup
@whomwah whomwah merged commit 4d7c01e into whomwah:master Aug 13, 2011
@whomwah
Copy link
Owner

whomwah commented Aug 15, 2011

Thanks again for your work. I have merged your initial patch and released a new version of the Gem. If you see any problems, do please let me know.

Best,
Duncan

On 13 Aug 2011, at 18:19, gioele wrote:

Hello,

I refactored a bit of code of RQRCode code in order to make it faster. The refactored code pass all the test in test suite.

I have an additional commit (f4de8484122ecd1271945b159f22ab99e09d53ce) that would bring another 20% speed up, but it fails two test. I'm investigating whether the older test code was wrong or the new refactored code is to blame.

Please pull these changes, there are also some little bug fix inside (for example, the original code did not take level 4 demerit score into account).

Reply to this email directly or view it on GitHub:
#6

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.

None yet

2 participants