Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relax atomic operations of update_gauge and increment_counter #184

Merged
merged 1 commit into from Mar 16, 2021

Conversation

blt
Copy link
Contributor

@blt blt commented Mar 15, 2021

This commit relaxes the atomics of the aforementioned functions, removing the sync pressure on machines with large numbers of CPUs or ARM. As the reads of counters and gauges were formerly relaxed I believe this maintains the semantic intentions, though it is now possible for a writer to immediately read and "lose" the write temporarily if the read is done from a different CPU, in the case of the counter.

I noticed this while digging through vectordotdev/vector#6770. I haven't managed to test in our project because of vectordotdev/vector#6412.

Signed-off-by: Brian L. Troutwine brian@troutwine.us

This commit relaxes the atomics of the aforementioned functions, removing the
sync pressure on machines with large numbers of CPUs or ARM. As the reads of
counters and gauges were formerly relaxed I believe this maintains the semantic
intentions, though it is now possible for a writer to immediately read and
"lose" the write temporarily if the read is done from a different CPU, in the
case of the counter.

Signed-off-by: Brian L. Troutwine <brian@troutwine.us>
Copy link
Member

@tobz tobz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all seems reasonable; just a single spot we need to tweak.

metrics-util/src/handle.rs Show resolved Hide resolved
@tobz tobz merged commit f28630c into metrics-rs:main Mar 16, 2021
@tobz
Copy link
Member

tobz commented Mar 16, 2021

Merged. Thanks for you contribution!

@blt blt deleted the blt-relaxed_atomics branch March 16, 2021 16:20
mnpw pushed a commit to mnpw/metrics that referenced this pull request Mar 8, 2024
Relax atomic operations of `update_gauge` and `increment_counter`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants