Reconsider OpenMP autotune
@magnumripper is already working on this.
Since Nov 24, 2020.
- Dominant language
- C
- Stars
- 13.6k
- Forks
- 2.6k
- PR merge metrics
- No merged PRs in 30d
Description
I don't understand the uses of min_keys_per_crypt (why min and not max) in omp_autotune.c when running 1 thread. This actually hurts:
solar@gcc202:~/john/src$ OMP_NUM_THREADS=1 ../run/john -test -form=sip -v=5
initUnicode(UNICODE, ASCII/ASCII)
ASCII -> ASCII -> ASCII
Warning: OpenMP is disabled; a non-OpenMP build may be faster
Benchmarking: SIP [MD5 32/64]... Loaded 8 hashes with 8 different salts to test db from test vectors
SIP MKPC autotune using test db
MKPC 1: 327313 crypts (327313x1) in 1.000000 seconds, 327313 c/s +
Autotune found best speed at MKPC of 1 (1 * 1)
PASS,
Test mask: ?a?a?l?u?d?d?s
DONE
Many salts: 488852 c/s real, 488852 c/s virtual
Only one salt: 438397 c/s real, 440600 c/s virtual
solar@gcc202:~/john/src$ OMP_NUM_THREADS=2 ../run/john -test -form=sip -v=5
initUnicode(UNICODE, ASCII/ASCII)
ASCII -> ASCII -> ASCII
Will run 2 OpenMP threads
Benchmarking: SIP [MD5 32/64]... (2xOMP) Loaded 8 hashes with 8 different salts to test db from test vectors
SIP OMP autotune using test db
OMP scale 1: 3224704 crypts (25193x128) in 1.000000 seconds, 3224704 c/s +
Autotune found best speed at OMP scale of 1
PASS,
Test mask: ?a?a?l?u?d?d?s
DONE
Many salts: 3812K c/s real, 1906K c/s virtual
Only one salt: 3532K c/s real, 1770K c/s virtual
As you can see, 1 thread's speed is ~8x lower than 2 threads. It was supposed to be only under 2x lower. Indeed, this can be worked around:
solar@gcc202:~/john/src$ OMP_NUM_THREADS=1 ../run/john -test -form=sip -v=5 -tune=128
initUnicode(UNICODE, ASCII/ASCII)
ASCII -> ASCII -> ASCII
Warning: OpenMP is disabled; a non-OpenMP build may be faster
Benchmarking: SIP [MD5 32/64]... Loaded 8 hashes with 8 different salts to test db from test vectors
PASS,
Test mask: ?a?a?l?u?d?d?s
DONE
Many salts: 2076K c/s real, 2076K c/s virtual
Only one salt: 1990K c/s real, 1990K c/s virtual
While we could also work around it by setting a higher min_keys_per_crypt in the format, that's not what that field is for; it is mostly for uses by "single crack" mode, where we need to know the minimum that doesn't cause dummy computation (no lower than SIMD width, etc.) There's no expectation that min_keys_per_crypt is sufficient for optimal performance; the only expectation is it's sufficient not to leave any hardware completely unused (that the format would use at the current thread count at all).
The above is on sparc64, where we don't use our OMP_SCALE presets by default, but I guess it should be possible to observe the same issue on x86-64 by forcing autotune.
Other observations:
We tune on 8-character strings starting with:
char key[PLAINTEXT_BUFFER_SIZE] = "tUne0000";
However, we've since standardized on 7 characters for benchmarks and OpenCL autotune. We should here, too.
We measure time needed to run crypt_all() only, excluding set_key() and cmp_all() and always passing NULL for salt. Ideally, we'd do the same thing benchmark and maybe OpenCL autotune do, where they use mask mode.
By only measuring crypt_all(), we autotune for the case of many salts (excluding comparisons, which is wrong). Ideally, we'd autotune for the actual salt count remaining after loading.
Autotuning for the case of many salts is especially wrong for formats that are not even salted. For those, set_key() should always be in the loop. But ideally we'd just use the actual salt count, which will just happen to always be 1 for those formats. No need to special-case them.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.