Skip to content

Conversation

@ywkaras
Copy link
Contributor

@ywkaras ywkaras commented Aug 9, 2023

This variable is used frequently in most threads. By avoiding unnecessary writes of it, locks of its cache line in (all) per-core caches are reduced. This should reduce CPU core stalls. The variable thus does not need to be thread_local.

This variable is used frequently in most threads.  By avoiding
unnecessary writes of it, locks of its cache line in (all)
per-core caches are reduced.  This should reduce CPU core
stalls.  The variable thus does not need to be thread_local.
@ywkaras ywkaras marked this pull request as draft August 9, 2023 00:42
@ywkaras ywkaras self-assigned this Aug 9, 2023
@ywkaras ywkaras added the Core label Aug 9, 2023
@ywkaras ywkaras added this to the 10.0.0 milestone Aug 9, 2023
@ywkaras
Copy link
Contributor Author

ywkaras commented Aug 9, 2023

So, in a nutshell, the issue is, how, and in what weird ways, are we going to fuzz the high-res clock, in order to improve performance. Yahoo prod says that the current fuzzing (that resulted from thread_local cur_time) changes ATS stats, making it look like transactions take longer.

Hard to know the relative performance of my change without testing. Depends how often get_hrtime_updated() is called. (Typically several hundred instructions execute per nanosecond I think.) The performance would also be better if ink_get_hrtime_internal() returns a time that is already fuzzed. That is, the granularity is not really one nanosecond.

We could improve performance of my change by adding fuzzing, by only updating cur_time if it's less than now by some delta.

@zwoop
Copy link
Contributor

zwoop commented Aug 9, 2023

@ywkaras I got rid of all the TSC stuff, so that's all gone. This version is as fast as current master, solves all of Yahoo's problems, simplifies the usage, and guarantees 1ms or better accuracy.

@ywkaras
Copy link
Contributor Author

ywkaras commented Aug 9, 2023

Seems like switching to a coarse clock may make the clock more fuzzy. So, maybe Yahoo prod would still see squirrelly metrics.

@ywkaras ywkaras closed this Aug 9, 2023
@zwoop zwoop removed this from the 10.0.0 milestone Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants