Fix issue #211. Allow key_stddev and key_median options to be set values #212
Conversation
memtier_benchmark.cpp
Outdated
| @@ -694,15 +694,15 @@ static int config_parse_args(int argc, char *argv[], struct benchmark_config *cf | |||
| break; | |||
| case o_key_stddev: | |||
| endptr = NULL; | |||
| cfg->key_stddev = (unsigned int) strtof(optarg, &endptr); | |||
| cfg->key_stddev = strtoull(optarg, &endptr, 10); | |||
There was a problem hiding this comment.
I don't like the loss of fractional part. Specifically in stddev.
But anyway I think the problem is just the wrong use of 32bit float instead of double (although even 32bit float can hold values much larger than 32bit integer).
I.e. strtod should solve it and keep both thr fractional part and a huge range.
There was a problem hiding this comment.
i missed the fact that we had casting ((unsigned int)), so we didn't have fractional part anyway.
but that casting is actually the problem (not the strtof), so it was maybe enough to just drop the casting.
but if we change these lines, let's got with strtod (changing to strtoull would be a breaking change, since old commands that used to work, will now error)
There was a problem hiding this comment.
I understand.
I have changed it to use strtod.
clkbug
force-pushed
the
allow-larger-key-median-and-stddev
branch
from
2bf9737
to
c5f2ddc
Compare
|
@clkbug Thank you |
I have fixed issue 211.
This changed key_stddev and key_median to read as unsigned long long.
In real-world use cases, the key ranges will be wide enough that the fractional portions will not matter.