Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Major performance regression on older Intel CPU #58

Closed
greatroar opened this issue Apr 4, 2021 · 9 comments
Closed

Major performance regression on older Intel CPU #58

greatroar opened this issue Apr 4, 2021 · 9 comments

Comments

@greatroar
Copy link

I got the following benchmark results for sha256-simd v0.1.1 on an Intel Core i7-3770K (Go 1.16.2, Linux):

name                speed
Hash/AVX_/8Bytes-8  34.7MB/s ± 0%
Hash/AVX_/1K-8       312MB/s ± 1%
Hash/AVX_/8K-8       332MB/s ± 0%
Hash/AVX_/1M-8       336MB/s ± 0%
Hash/AVX_/5M-8       334MB/s ± 1%
Hash/AVX_/10M-8      333MB/s ± 2%
Hash/SSSE/8Bytes-8  34.6MB/s ± 1%
Hash/SSSE/1K-8       311MB/s ± 0%
Hash/SSSE/8K-8       333MB/s ± 0%
Hash/SSSE/1M-8       334MB/s ± 0%
Hash/SSSE/5M-8       334MB/s ± 1%
Hash/SSSE/10M-8      333MB/s ± 1%
Hash/GEN_/8Bytes-8  23.8MB/s ± 1%
Hash/GEN_/1K-8       210MB/s ± 1%
Hash/GEN_/8K-8       225MB/s ± 1%
Hash/GEN_/1M-8       227MB/s ± 0%
Hash/GEN_/5M-8       225MB/s ± 1%
Hash/GEN_/10M-8      226MB/s ± 1%

Performance of the GEN version is unchanged in v1.0.0, as expected, so dropping the SSSE/AVX asm version reduces throughput by a third on this CPU. sha256-simd is no longer faster than the stdlib implementation.

Would you consider resurrecting the SSSE asm?

@harshavardhana
Copy link
Member

You should just use standard sha256 that comes with Go @greatroar this package is not useful otherwise. The purpose was to reduce the scope of this package and advise users to migrate to stdlib.

@greatroar
Copy link
Author

You mean this package is essentially being deprecated?

@harshavardhana
Copy link
Member

You mean this package is essentially being deprecated?

Correct @greatroar

@greatroar
Copy link
Author

Ok, thanks for the heads up.

drakkan added a commit to drakkan/sftpgo that referenced this issue Apr 5, 2021
sha256-simd is now deprecated

minio/sha256-simd#58

This could slow down sha256 computation on some CPU
@AudriusButkevicius
Copy link

AudriusButkevicius commented Apr 5, 2021

I don't think it makes sense to deprecate this package.

Go standard library still does not support native SHA extensions.

Best it can do is seems to be AVX2, even for modern CPUs, which results in a roughly ~5x difference.

From syncthing:

Single thread SHA256 performance is 1857 MB/s using minio/sha256-simd (431 MB/s using crypto/sha256).

@drakkan
Copy link

drakkan commented Apr 6, 2021

Hi,

can you please better clarify the package status? A pull request (#59) was just merged and this package is still used in MinIO, thank you

@klauspost
Copy link
Contributor

We are reducing the maintenance cost of this package, so it will be kept as long as there are benefits. For CPUs with SHA extensions the benefit is clear, and AVX512 can have a benefit as well, but requires special implementations.

If you want a version which detects older CPUs and use the SSE/AVX code feel free to fork it.

@drakkan
Copy link

drakkan commented Apr 6, 2021

@klauspost thank you. I'm mainly interested in SHA Extensions for x86 and ARM64 for ARM

@greatroar
Copy link
Author

On arm64, stdlib has used the SHA extensions since 2017: golang/go@7b8a7f8.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants