valkey
valkey copied to clipboard
Set the default to enable CPU clock for monotonic time tracking
back in https://github.com/redis/redis/pull/7644 we introduced more efficient monotonic time tracking which avoid using the clock_gettime posix which is running a vDSO syscall. The expected improvements is expected to be ~x3 times faster time access (reduction from ~100ns to 10-30ns). However, we never set this config to be the default as stated in the code:
/* Using the processor clock (aka TSC on x86) can provide improved performance
* throughout the server wherever the monotonic clock is used. The processor clock
* is significantly faster than calling 'clock_gettime' (POSIX). While this is
* generally safe on modern systems, this link provides additional information
* about use of the x86 TSC: http://oliveryang.net/2015/09/pitfalls-of-TSC-usage
*
* To use the processor clock, either uncomment this line, or build with
* CFLAGS="-DUSE_PROCESSOR_CLOCK"
* #define USE_PROCESSOR_CLOCK
The proposal here is to change the default compilation define to true (in case the CPU supports these instructions). we can avoid the CPU support testing during compile time as it is verified during the initialization of the clocks.
To be clear, that comment does explicitly state we don't build with the processor clock.
I think we should change the default in the next version, since I would like to have a few months in unstable for people to report any weird interactions.
@madolson personally I have some concerns about changing defaults during minor versions. I think it might be common that there is more flexibility in tests and verifications around minor version upgrade. in case this will cause impact or prove to be problematic, the mitigation envolvs compile a new binary, which is less friendly. Maybe we should target the next major release and focus on updating the docs+README till then?
@ranshid I don't have that strong of an opinion. I'm still not sure what is wrong with the docs, but I'm fine re-targeting it to 10.
@madolson Make a Valkey 10 project and move this.