cmd/syncthing: Add selectable sha256 package (fixes #3613, fixes #3614)

[calmh]

Purpose

This adds autodetection of the fastest hashing library on startup, thus handling the performance regression. It also adds an environment variable to control the selection, STHASHING=standard (Go standard library version, avoids SIGILL crash when the minio library has bugs on odd CPUs), STHASHING=minio (to force using the minio version) or unset for the default autodetection.

Testing

Works for me...

[AudriusButkevicius]

We could catch the sigill in monitor, and restart with alternative implementation and potentially set it in stone/config.

[AudriusButkevicius]

Also, because we are benching both implementations, perhaps it would be nice to print both hashing package performances?

[calmh]

That's a good idea, didn't think of that, will look into it!

[scienmind]

Regarding the startup hash bench:

The bench itself is quite short, and as noted on the forum discussion about the new hash algo. is is short enough to not bump up the CPU up to its full speed (on relatively powerful CPUs).

I tested it on few laptops - and unless either the PC is already under load or its on "Crazy Super Performance Power Plan", the clock speed won't bump up to max during the hash test, degrading the results. Ranging from 50MB/s (starting ST when PC is fully idle) up to 300+MB/s (when some CPU core is already busy before launching ST).

The point is - it may lead to wrong hash algorithm selection, giving an advantage to whichever one will run last. So it may be a good idea to either make hash test longer, or do few iterations of hash benchs, alternating the algorithm used.

[calmh]

Good point. It runs three iterations, but first one algo and then the other, and only for 75ms each. They should be interleaved.

[canton7]

How about running a longer test on first start, then saving the results? You'd need a good strategy for catching people changing their CPU, although I don't imagine that'd be a common occurrence.

[scienmind]

If by first start you mean "the first", when the ID and config are born, it's not optimal:
If anyone (I do :) runs a portable version, they'd be effectively changing CPUs...

(one of my ST instances is a a portable SyncTrayzor on a portable HDD that travels across bunch of PCs...)

[calmh]

It doesn't need to be that exhaustive, nobody dies if we get it wrong. A few hundred milliseconds at startup should be sufficient if done correctly.

[calmh]

Mmkay, so this now does the relevant benchmarking and reporting:

jb@unu:~/s/g/s/s/test $ STHASHING=standard syncthing -home h1 | grep perfo
[I6KAH] 17:58:19 INFO: Single thread hash performance is 346 MB/s using crypto/sha256 (0.00 MB/s using minio/sha256-simd).
^C
jb@unu:~/s/g/s/s/test $ STHASHING=minio syncthing -home h1 | grep perfo
[I6KAH] 17:58:28 INFO: Single thread hash performance is 357 MB/s using minio/sha256-simd (351 MB/s using crypto/sha256).
^C
jb@unu:~/s/g/s/s/test $ syncthing -home h1 | grep perfo
[I6KAH] 17:58:37 INFO: Single thread hash performance is 362 MB/s using minio/sha256-simd (355 MB/s using crypto/sha256).
^C

It also detects and acts on SIGILL, but doesn't do anything permanently about it. Also doesn't work when not running with the monitor process, but hey...

jb@unu:~/s/g/s/syncthing $ syncthing -home h1 
[monitor] 18:09:27 INFO: Starting syncthing
[H73EW] 18:09:27 INFO: syncthing v0.14.7+4-ge2cacc3-dirty-sha256 "Dysprosium Dragonfly" (go1.7.1 darwin-amd64) jb@unu.kastelo.net 2016-09-23 15:59:14 UTC
[H73EW] 18:09:27 INFO: My ID: H73EWZC-BE7F57I-2KBRW7K-YZ56BFF-GREHSBC-65GOKDE-N3Q5YH7-65N5YAK
SIGILL: illegal instruction
[monitor] 18:09:27 WARNING: 
*******************************************************************************
* Crash due to illegal instruction detected. This is most likely due to a CPU *
* incompatibility with the high performance hashing package. Switching to the *
* standard hashing package instead. Please report this issue at:              *
*                                                                             *
*   https://github.com/syncthing/syncthing/issues                             *
*                                                                             *
* Include the details of your CPU.                                            *
*******************************************************************************

[monitor] 18:09:27 INFO: Syncthing exited: exit status 2
[monitor] 18:09:28 INFO: Starting syncthing
[H73EW] 18:09:28 INFO: syncthing v0.14.7+4-ge2cacc3-dirty-sha256 "Dysprosium Dragonfly" (go1.7.1 darwin-amd64) jb@unu.kastelo.net 2016-09-23 15:59:14 UTC
[H73EW] 18:09:28 INFO: My ID: H73EWZC-BE7F57I-2KBRW7K-YZ56BFF-GREHSBC-65GOKDE-N3Q5YH7-65N5YAK
[H73EW] 18:09:28 INFO: Single thread hash performance is 339 MB/s using crypto/sha256 (0.00 MB/s using minio/sha256-simd).
[H73EW] 18:09:28 INFO: Starting deadlock detector with 20m0s timeout
[H73EW] 18:09:28 INFO: Ready to synchronize czxwh-ckkfq (readwrite)

Tested the SIGILL handling by changing some random bytes in the sha256 package assembly.

[AudriusButkevicius]

So does it switch to standard as it crashes? Asking as I see a return statement there.

[calmh]

Yes. The stdout reader sets the environment variable and returns (instead of continuing to process the rest of the sigill crash). The main loop in the monitor waits for the stdout and stderr readers to complete, then attempts another launch, at which point the STHASHING=standard variable takes effect and the start up works.

So it'll crash once on every monitor startup, with the message pointing to github. For those who notice we may get the bug report, for those who don't it should mostly just work. For those who run it under a service with -no-restart they'll come here to complain and we'll have to accept the bug report with CPU details and point towards setting STHASHING=standard.

[AudriusButkevicius]

@st-review merge

[st-review]

👌 Merged as d328e0f. Thanks, @calmh!