Skip to content

Disable OpenSSL by default#1086

Merged
lgritz merged 1 commit intoAcademySoftwareFoundation:masterfrom
lgritz:lg-openssl
Mar 9, 2015
Merged

Disable OpenSSL by default#1086
lgritz merged 1 commit intoAcademySoftwareFoundation:masterfrom
lgritz:lg-openssl

Conversation

@lgritz
Copy link
Collaborator

@lgritz lgritz commented Mar 7, 2015

A while back, we found that using OpenSSL's SHA-1 implementation rather than our usual one resulted in a significant speedup of maketx.

I've done some retimings, now that a lot else has changed about maketx's internals (including adopting the practice of hashing blocks of scanlines, making it amenable to multithreading), and found that the speedup we get by using OpenSLL's SHA-1 is barely measurable. The minuscule speedup doesn't seem worth the trouble of imposing OpenSSL as a dependency.

In light of this, I propose this patch to change the default to using the SHA-1 implementation that's compiled in, thus removing OpenSSL as a dependency.

Now it's still an option, mainly so I can continue to compare the two methods. But it's possible that in the future, we may remove the potential for using OpenSSL altogether.

@fpsunflower
Copy link
Contributor

LGTM

Reducing the number of dependencies is always a good thing 👍

A while back, we found that using OpenSSL's SHA-1 implementation rather than
our usual one resulted in a significant speedup of maketx.

I've done some retimings, now that a lot else has changed about maketx's
internals (including adopting the practice of hashing blocks of
scanlines, making it amenable to multithreading), and found that the
speedup we get by using OpenSLL's SHA-1 is barely measurable. The
minuscule speedup doesn't seem worth the trouble of imposing OpenSSL as
a dependency.

In light of this, I propose this patch to change the default to using
the SHA-1 implementation that's compiled in, thus removing OpenSSL as a
dependency.

Now it's still an option, mainly so I can continue to compare the two
methods. But it's possible that in the future, we may remove the
potential for using OpenSSL altogether.
@lgritz lgritz merged commit 9d171d4 into AcademySoftwareFoundation:master Mar 9, 2015
@lgritz lgritz deleted the lg-openssl branch March 9, 2015 18:37
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