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

Use simpler locking in the Go 1.17 collector #975

Merged
merged 1 commit into from Jan 25, 2022

Commits on Jan 24, 2022

  1. Use simpler locking in the Go 1.17 collector

    A previous PR made it so that the Go 1.17 collector locked only around
    uses of rmSampleBuf, but really that means that Metric values may be
    sent over the channel containing some values from future metrics.Read
    calls. While generally-speaking this isn't a problem, we lose any
    consistency guarantees provided by the runtime/metrics package.
    
    Also, that optimization to not just lock around all of Collect was
    premature. Truthfully, Collect is called relatively infrequently, and
    its critical path is fairly fast (10s of µs). To prove it, this change
    also adds a benchmark.
    
    name            old time/op  new time/op  delta
    GoCollector-16  43.7µs ± 2%  43.2µs ± 2%   ~     (p=0.190 n=9+9)
    
    Note that because the benchmark is single-threaded it actually looks
    like it might be getting *slightly* faster, because all those Collect
    calls for the Metrics are direct calls instead of interface calls.
    
    Signed-off-by: Michael Anthony Knyszek <mknyszek@google.com>
    mknyszek committed Jan 24, 2022
    Copy the full SHA
    648c419 View commit details
    Browse the repository at this point in the history