<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jan 13, 2017 at 11:22 AM, Vladislav Egorov <span dir="ltr"><<a href="mailto:vegorov180@gmail.com" target="_blank">vegorov180@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">13.01.2017 19:51, Emil Velikov пишет:<span class="gmail-"><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
From: Emil Velikov <<a href="mailto:emil.velikov@collabora.com" target="_blank">emil.velikov@collabora.com</a>><br>
<br>
At the moment we support 5+ different implementations each with varying<br>
amount of bugs - from thread safely problems [1], to outright broken<br>
implementation(s) [2]<br>
<br>
In order to accommodate these we have 150+ lines of configure script and<br>
extra two configure toggles. Whist an actual implementation being<br>
~200loc and our current compat wrapping ~250.<br></blockquote></span></blockquote><div><br></div><div>Yes, this is a problem. Especially given that at least one of those implementations (openssl?) is something that a certain major game distributor likes to hard-link into things causing interesting and hard-to-debug problems. I am all for getting rid of the "piles of different dependencies" approach.<br><br>Also, something I would like to see (maybe a follow-on patch?) would a change to the mesa internal API to be able to put the SHA context on the stack and not need to malloc it. It's not really a memory or cycle-saving thing so much as it leaves one fewer cleanup paths you have to worry about.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Let's not forget that different people use different code paths, thus<br>
effectively makes it harder to test and debug since the default<br>
implementation is automatically detected.<br>
<br>
To minimise all these lovely experiences, import the "100% Public<br>
Domain" OpenBSD sha1 implementation. Clearly document any changes needed<br>
to get building correctly, since many/most of those can be upstreamed<br>
making future syncs easier.<br>
</blockquote>
<br></span>
It can hurt performance. OpenSSL implementation is optimized for all thinkable architectures and it will use hardware SHA-1 instructions on newer CPUs. From <a href="https://github.com/openssl/openssl/blob/master/crypto/sha/asm/sha1-x86_64.pl" rel="noreferrer" target="_blank">https://github.com/openssl/ope<wbr>nssl/blob/master/crypto/sha/<wbr>asm/sha1-x86_64.pl</a> :<br>
<br>
> Current performance is summarized in following table. Numbers are<br>
> CPU clock cycles spent to process single byte (less is better).<br>
><br>
> x86_64 SSSE3 AVX[2]<br>
> P4 9.05 -<br>
> Opteron 6.26 -<br>
> Core2 6.55 6.05/+8% -<br>
> Westmere 6.73 5.30/+27% -<br>
> Sandy Bridge 7.70 6.10/+26% 4.99/+54%<br>
> Ivy Bridge 6.06 4.67/+30% 4.60/+32%<br>
> Haswell 5.45 4.15/+31% 3.57/+53%<br>
> Skylake 5.18 4.06/+28% 3.54/+46%<br>
> Bulldozer 9.11 5.95/+53%<br>
> VIA Nano 9.32 7.15/+30%<br>
> Atom 10.3 9.17/+12%<br>
> Silvermont 13.1(*) 9.37/+40%<br>
> Goldmont 8.13 6.42/+27% 1.70/+380%(**)<br>
<br>
Quick benchmark on my Haswell of the OpenBSD implementation compiled with GCC5 -O2: ~8 cycles per byte on 32-bit, ~7 cycles per byte on 64-bit. But Haswell is a very powerful CPU, on weaker CPUs the difference would be probably larger, especially on new CPUs that have SHA instruction set.<br></blockquote><div><br></div><div>Thanks for the numbers. It sounds like, on Haswell, the openSSL implementation is about 2x as fast which is very useful to know. However, this isn't on a super perf-critical path. We never use SHA1 on any draw-time paths; we always use a simpler hash function in those cases and reserve SHA1 for when we really don't want collisions. That said, it's a bit more critical than Emil makes it sound. A typical Vulkan application may easily create 10k pipelines and each of those will involve hashing at least about 100B of data (not include the SPIR-V source). I doubt, however, that this is enough to really cause a problem given how much other work goes into building a pipeline.<br><br></div><div>Unfortunately, the OpenSSL implementation, while fast, is one of the ones that is causing problems. One of our favorite game distributors likes to hard-link against openssl in some of their games and/or libraries (not sure which). This means that, if mesa tries to dynamically open libssl, you get mysterious crashes due to slight differences between the system-installed version and the one that has been linked into the game. This makes trying to use the OpenSSL implementation a non-starter without being able to wholesale import the implementation.<br></div><div><br></div><div>Emil, I'm fine with this change. I haven't reviewed the details, but my gut tells me we can eat the perf difference for now. Consider that an Acked-by if you'd like but it would be good to have someone review at least the build system stuff.<br></div></div></div></div>