max_keys_per_crypt tuning
(too old to reply)
Solar Designer
2017-12-17 20:53:48 UTC
Raw Message
Then, OMP_SCALE - the additional scaling factor to compensate for OpenMP
thread desynchronization (to reduce the relative wait times for getting
the threads back in sync) - should only be applied when actually running
more than 1 thread. Currently we usually also apply it for the 1 thread
case, which I now think is wrong. We just got this fixed in
- self->params.min_keys_per_crypt *= omp_t;
- omp_t *= OMP_SCALE;
- self->params.max_keys_per_crypt *= omp_t;
+ if (omp_t > 1) {
+ self->params.min_keys_per_crypt *= omp_t;
+ omp_t *= OMP_SCALE;
+ self->params.max_keys_per_crypt *= omp_t;
+ }
Dhiru has made these changes now. Thanks!


And I've just realized that we should also rename the "omp_t" variable,
since its current name is confusing (the _t suffix is commonly used on
typedef's), reserved by POSIX (all *_t are), and isn't comfortably
unlikely not to clash with whatever a future version of POSIX might
define. In the core tree, I happen to have called a similar variable
simply "n". That's in the *_init() functions in DES_bs.c and MD5_std.c.
If we'd like a more descriptive name (such as in one format where this
needs to be a global static variable), it can be e.g. "threads".
We might also want to move common code like this into a function (maybe
in formats.c) that we'd use from formats' init(), instead of keeping it
as duplicate code like we have now.
I think the function should accept "self" and "scale" (we'll pass
OMP_SCALE) as parameters. It could be called unconditionally (including
on non-OpenMP), and would have the "#ifdef _OPENMP" inside.