Skip to content
This repository was archived by the owner on Feb 2, 2019. It is now read-only.

Unroll FeZero, FeOne#9

Closed
ValarDragon wants to merge 1 commit into
tendermint:masterfrom
ValarDragon:dev/unroll_constants
Closed

Unroll FeZero, FeOne#9
ValarDragon wants to merge 1 commit into
tendermint:masterfrom
ValarDragon:dev/unroll_constants

Conversation

@ValarDragon

Copy link
Copy Markdown

This unrolls FeZero and FeOne. This doesn't change the output of the functions, but increases its speed:

$ benchcmp old.txt new.txt
benchmark                    old ns/op     new ns/op     delta
BenchmarkKeyGeneration-8     100659        99175         -1.47%
BenchmarkSigning-8           102219        100941        -1.25%
BenchmarkVerification-8      278957        275965        -1.07%
BenchmarkKeyGeneration-8     120965        119236        -1.43%
BenchmarkMap-8               36722         36478         -0.66%

The idea was taken from agl#11

@liamsi liamsi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Not sure if we'd want to deviate further from AGL's (or golang/x/crypto's) code for these minor performance improvements though.

I'll leave it to the admins of this repo to decide (I don't have write access here anyways).

@ValarDragon

Copy link
Copy Markdown
Author

@ebuchman can this be merged?

@ValarDragon

Copy link
Copy Markdown
Author

repo abandoned.

@ValarDragon ValarDragon closed this Jan 2, 2019
@cwgoes

cwgoes commented Jan 3, 2019

Copy link
Copy Markdown

Does Golang's compiler really not do this?

(evidently not, per benchmarks)

@ValarDragon

Copy link
Copy Markdown
Author

Theres a lot of fighting the compiler you have to do for performant golang code :(, the compiler doesn't do many optimizations for you. (See https://www.youtube.com/watch?v=keydVd-Zn80 for the struggles of in-lining)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants