capitalone / fpe

A format-preserving encryption implementation in Go
Apache License 2.0
202 stars 41 forks source link

Remove extraneous allocations. #3

Closed cstockton closed 7 years ago

cstockton commented 7 years ago

What's in this PR?

I removed extraneous allocations to illustrate the techniques for use in other areas of the library if the author would like.

TODOs

I saw the CLA after I made the changes, my mistake. I'll look into it tomorrow evening maybe, depending how much information I need to disclose.

benchmark old ns/op new ns/op delta BenchmarkDecrypt/Sample#1-24 45900 30815 -32.86% BenchmarkDecrypt/Sample#2-24 45662 31572 -30.86% BenchmarkDecrypt/Sample#3-24 49453 39011 -21.11% BenchmarkDecrypt/Sample#4-24 44523 37762 -15.19% BenchmarkDecrypt/Sample#5-24 40971 25624 -37.46% BenchmarkDecrypt/Sample#6-24 45525 27824 -38.88% BenchmarkDecrypt/Sample#7-24 42651 30249 -29.08% BenchmarkDecrypt/Sample#8-24 49264 39506 -19.81% BenchmarkDecrypt/Sample#9-24 42568 31181 -26.75% BenchmarkDecrypt/Sample#10-24 42259 25162 -40.46% BenchmarkDecrypt/Sample#11-24 46210 32699 -29.24% BenchmarkDecrypt/Sample#12-24 39739 29340 -26.17% BenchmarkDecrypt/Sample#13-24 50127 34662 -30.85% BenchmarkDecrypt/Sample#14-24 52410 37990 -27.51% BenchmarkDecrypt/Sample_#15-24 44501 30777 -30.84%

benchmark old allocs new allocs delta BenchmarkDecrypt/Sample#1-24 231 102 -55.84% BenchmarkDecrypt/Sample#2-24 237 108 -54.43% BenchmarkDecrypt/Sample#3-24 234 105 -55.13% BenchmarkDecrypt/Sample#4-24 234 105 -55.13% BenchmarkDecrypt/Sample#5-24 228 99 -56.58% BenchmarkDecrypt/Sample#6-24 237 108 -54.43% BenchmarkDecrypt/Sample#7-24 231 102 -55.84% BenchmarkDecrypt/Sample#8-24 234 105 -55.13% BenchmarkDecrypt/Sample#9-24 234 105 -55.13% BenchmarkDecrypt/Sample#10-24 228 99 -56.58% BenchmarkDecrypt/Sample#11-24 231 102 -55.84% BenchmarkDecrypt/Sample#12-24 234 105 -55.13% BenchmarkDecrypt/Sample#13-24 237 108 -54.43% BenchmarkDecrypt/Sample#14-24 237 108 -54.43% BenchmarkDecrypt/Sample_#15-24 228 99 -56.58%

benchmark old bytes new bytes delta BenchmarkDecrypt/Sample#1-24 7728 2912 -62.32% BenchmarkDecrypt/Sample#2-24 7728 2928 -62.11% BenchmarkDecrypt/Sample#3-24 8176 3344 -59.10% BenchmarkDecrypt/Sample#4-24 8176 3344 -59.10% BenchmarkDecrypt/Sample#5-24 7696 2912 -62.16% BenchmarkDecrypt/Sample#6-24 7760 2928 -62.27% BenchmarkDecrypt/Sample#7-24 7728 2912 -62.32% BenchmarkDecrypt/Sample#8-24 8176 3344 -59.10% BenchmarkDecrypt/Sample#9-24 8176 3344 -59.10% BenchmarkDecrypt/Sample#10-24 7696 2912 -62.16% BenchmarkDecrypt/Sample#11-24 7728 2912 -62.32% BenchmarkDecrypt/Sample#12-24 7744 2928 -62.19% BenchmarkDecrypt/Sample#13-24 8192 3360 -58.98% BenchmarkDecrypt/Sample#14-24 8192 3360 -58.98% BenchmarkDecrypt/Sample_#15-24 7696 2912 -62.16%

anitgandhi commented 7 years ago

Thanks for the informative PR!

I do request that you sign the CLA if you'd like this merged. If not, would it be ok if I piggy-backed on this code and applied it to the other package as well?

cstockton commented 7 years ago

I signed the individual CLA, though I didn't (and won't) provide a cell phone number or address. Let me know if you have any questions though I would be glad to help.

cstockton commented 7 years ago

Are you merging this or should I close?

anitgandhi commented 7 years ago

@cstockton unfortunately our open source office rules require phone number and address based on what @jaredsmith told me :(

Unless something's changed on that end I will have to decline this PR

cstockton commented 7 years ago

@jaredsmith What is your privacy policy around this personal information? It's a google doc which makes it very unclear who has access to these details. While sometimes I come across a CLA that may ask for my address, It's unusual to require a personal phone number in a CLA, under what scenario would CapitalOne even need my address- let alone call me?

jaredsmith commented 7 years ago

That's a great question -- we'll only contact you related to your open source contributions -- we don't use the data for any other reasons. Our legal team just likes having an address and phone number if there's a legal question about a particular contribution. The only people who have access to the data are the project leaders (so that they can see if someone has signed a CLA or not) and our Open Source Office. Nobody else has the data.

If there's any other questions I can answer regarding the CLAs or our open source office, please don't hesitate to ask.

cstockton commented 7 years ago

@jaredsmith I'll sign it after work as long as it will not be shared with third parties, used for marketing, advertising or recruitment purposes. That is: Only used to contact me in the event a legal dispute or question on code I've contributed. Though I would appreciate being tagged here first. Maybe a little clarity on this on the document would be helpful. Thanks!

cstockton commented 7 years ago

@anitgandhi This was signed.

anitgandhi commented 7 years ago

Thanks @cstockton . Have one change to request, see the review comment

cstockton commented 7 years ago

@anitgandhi Updated pull request thanks for the patience while I figured out the CLA stuff.

anitgandhi commented 7 years ago

No worries, glad the CLA stuff was sorted out @cstockton 👍