Skip to content

Commit

Permalink
rework 32 vs 64-bit arch atomics support + a lot of import consolidat…
Browse files Browse the repository at this point in the history
…ion/cleanup
  • Loading branch information
tobz committed Apr 15, 2023
1 parent 9c56ae4 commit a3a706b
Show file tree
Hide file tree
Showing 17 changed files with 129 additions and 136 deletions.
4 changes: 4 additions & 0 deletions metrics-exporter-prometheus/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased] - ReleaseDate

### Changed

- Switch to `metrics`-exposed version of `AtomicU64`.

## [0.11.0] - 2022-07-20

### Changed
Expand Down
1 change: 0 additions & 1 deletion metrics-exporter-prometheus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ metrics-util = { version = "0.14", path = "../metrics-util", default-features =
thiserror = { version = "1", default-features = false }
quanta = { version = "0.11", default-features = false }
indexmap = { version = "1", default-features = false }
portable-atomic = { version = "1", default-features = false, features = ["fallback"] }

# Optional
hyper = { version = "0.14", default-features = false, features = ["tcp", "http1"], optional = true }
Expand Down
6 changes: 2 additions & 4 deletions metrics-exporter-prometheus/src/registry.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
use portable_atomic::AtomicU64;
use std::sync::Arc;

use metrics::HistogramFn;
use metrics_util::registry::GenerationalStorage;
use metrics_util::AtomicBucket;
use metrics::{atomics::AtomicU64, HistogramFn};
use metrics_util::{registry::GenerationalStorage, AtomicBucket};
use quanta::Instant;

pub type GenerationalAtomicStorage = GenerationalStorage<AtomicStorage>;
Expand Down
4 changes: 4 additions & 0 deletions metrics-util/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased] - ReleaseDate

### Changed

- Switch to `metrics`-exposed version of `AtomicU64`.

## [0.14.0] - 2022-07-20

### Changed
Expand Down
3 changes: 1 addition & 2 deletions metrics-util/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ required-features = ["handles"]
metrics = { version = "0.20", path = "../metrics" }
crossbeam-epoch = { version = "0.9.2", default-features = false, optional = true, features = ["alloc", "std"] }
crossbeam-utils = { version = "0.8", default-features = false, optional = true }
portable-atomic = { version = "1", default-features = false, features = ["fallback"], optional = true }
aho-corasick = { version = "0.7", default-features = false, optional = true, features = ["std"] }
indexmap = { version = "1", default-features = false, optional = true }
quanta = { version = "0.11", default-features = false, optional = true }
Expand Down Expand Up @@ -91,4 +90,4 @@ layer-filter = ["aho-corasick"]
layer-router = ["radix_trie"]
summary = ["sketches-ddsketch"]
recency = ["registry", "quanta"]
registry = ["portable-atomic", "crossbeam-epoch", "crossbeam-utils", "handles", "hashbrown", "num_cpus"]
registry = ["crossbeam-epoch", "crossbeam-utils", "handles", "hashbrown", "num_cpus"]
21 changes: 13 additions & 8 deletions metrics-util/src/debugging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,19 @@
//! and core parts of the `metrics` ecosystem, they can be beneficial for in-process collecting of
//! metrics in some limited cases.
use core::hash::Hash;
use std::cell::RefCell;
use std::sync::atomic::Ordering;
use std::sync::{Arc, Mutex};
use std::{collections::HashMap, fmt::Debug};

use crate::registry::AtomicStorage;
use crate::{kind::MetricKind, registry::Registry, CompositeKey};
use std::{
cell::RefCell,
collections::HashMap,
fmt::Debug,
hash::Hash,
sync::{atomic::Ordering, Arc, Mutex},
};

use crate::{
kind::MetricKind,
registry::{AtomicStorage, Registry},
CompositeKey,
};

use indexmap::IndexMap;
use metrics::{Counter, Gauge, Histogram, Key, KeyName, Recorder, SharedString, Unit};
Expand Down
3 changes: 1 addition & 2 deletions metrics-util/src/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,7 @@ where

#[cfg(test)]
mod tests {
use metrics::{CounterFn, Key};
use portable_atomic::AtomicU64;
use metrics::{atomics::AtomicU64, CounterFn, Key};

use super::Registry;
use std::sync::{atomic::Ordering, Arc};
Expand Down
3 changes: 1 addition & 2 deletions metrics-util/src/registry/storage.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::sync::Arc;

use metrics::{CounterFn, GaugeFn, HistogramFn};
use portable_atomic::AtomicU64;
use metrics::{atomics::AtomicU64, CounterFn, GaugeFn, HistogramFn};

use crate::AtomicBucket;

Expand Down
16 changes: 15 additions & 1 deletion metrics/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased] - ReleaseDate

### Added

- A new module, `atomics`, exposes the atomic integer type that `CounterFn` and `GaugeFn` are
implemented for. This will publicly re-export the type for usage by downstream crates. (Credit to
[@repi](https://github.com/repi) for the original PR (#347) that did this.)

### Changed

- `portable-atomic` is only used on 32-bit architectures.

### Removed

- Removed the `std-atomics` feature flag.

## [0.20.1] - 2022-07-22

### Changed

- Bumped the dependency on `metrics-macros` to correctly use the updated versions that are necessary
for handling the recent `&'static str` -> `SharedString` change to `Recorder::descrfibe_*`.
for handling the recent `&'static str` -> `SharedString` change to `Recorder::describe_*`.

We'll also yank 0.20.0 once this is released to avoid the patch version triggering a breaking
change jump in transitive dependencies, and so people can't pick up a version of `metrics` that
Expand Down
6 changes: 2 additions & 4 deletions metrics/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,11 @@ bench = false
name = "macros"
harness = false

[features]
default = ["std-atomics"]
std-atomics = []

[dependencies]
metrics-macros = { version = "0.6", path = "../metrics-macros" }
ahash = { version = "0.8", default-features = false }

[target.'cfg(target_pointer_width = "32")'.dependencies]
portable-atomic = { version = "1", default-features = false, features = ["fallback"] }

[dev-dependencies]
Expand Down
64 changes: 64 additions & 0 deletions metrics/src/atomics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
//! Atomic types used for metrics.
//!
//! As the most commonly used types for metrics storage are atomic integers, implementations of
//! `CounterFn` and `GaugeFn` must be provided in this crate due to Rust's "orphan rules", which
//! disallow a crate from implementing a foreign trait on a foreign type.
//!
//! Further, we always require an atomic integer of a certain size regardless of whether the
//! standard library exposes an atomic integer of that size for the target architecture.
//!
//! As such, the atomic types that we provide handle implementations for are publicly re-exporter
//! here for downstream crates to utilize.
use std::sync::atomic::Ordering;

#[cfg(target_pointer_width = "32")]
pub use portable_atomic::AtomicU64;
#[cfg(not(target_pointer_width = "32"))]
pub use std::sync::atomic::AtomicU64;

use super::{CounterFn, GaugeFn};

impl CounterFn for AtomicU64 {
fn increment(&self, value: u64) {
let _ = self.fetch_add(value, Ordering::Release);
}

fn absolute(&self, value: u64) {
let _ = self.fetch_max(value, Ordering::AcqRel);
}
}

impl GaugeFn for AtomicU64 {
fn increment(&self, value: f64) {
loop {
let result = self.fetch_update(Ordering::AcqRel, Ordering::Relaxed, |curr| {
let input = f64::from_bits(curr);
let output = input + value;
Some(output.to_bits())
});

if result.is_ok() {
break;
}
}
}

fn decrement(&self, value: f64) {
loop {
let result = self.fetch_update(Ordering::AcqRel, Ordering::Relaxed, |curr| {
let input = f64::from_bits(curr);
let output = input - value;
Some(output.to_bits())
});

if result.is_ok() {
break;
}
}
}

fn set(&self, value: f64) {
let _ = self.swap(value.to_bits(), Ordering::AcqRel);
}
}
19 changes: 10 additions & 9 deletions metrics/src/cow.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use std::{
borrow::Borrow,
cmp::Ordering,
fmt,
hash::{Hash, Hasher},
marker::PhantomData,
mem::ManuallyDrop,
ptr::{slice_from_raw_parts, NonNull},
};

use crate::label::Label;
use alloc::borrow::Borrow;
use alloc::string::String;
use alloc::vec::Vec;
use core::cmp::Ordering;
use core::fmt;
use core::hash::{Hash, Hasher};
use core::marker::PhantomData;
use core::mem::ManuallyDrop;
use core::ptr::{slice_from_raw_parts, NonNull};

/// A clone-on-write smart pointer with an optimized memory layout.
pub struct Cow<'a, T: Cowable + ?Sized + 'a> {
Expand Down
93 changes: 1 addition & 92 deletions metrics/src/handles.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use portable_atomic::AtomicU64;
use std::sync::{atomic::Ordering, Arc};
use std::sync::Arc;

use crate::IntoF64;

Expand Down Expand Up @@ -144,96 +143,6 @@ impl Histogram {
}
}

impl CounterFn for AtomicU64 {
fn increment(&self, value: u64) {
let _ = self.fetch_add(value, Ordering::Release);
}

fn absolute(&self, value: u64) {
let _ = self.fetch_max(value, Ordering::AcqRel);
}
}

impl GaugeFn for AtomicU64 {
fn increment(&self, value: f64) {
loop {
let result = self.fetch_update(Ordering::AcqRel, Ordering::Relaxed, |curr| {
let input = f64::from_bits(curr);
let output = input + value;
Some(output.to_bits())
});

if result.is_ok() {
break;
}
}
}

fn decrement(&self, value: f64) {
loop {
let result = self.fetch_update(Ordering::AcqRel, Ordering::Relaxed, |curr| {
let input = f64::from_bits(curr);
let output = input - value;
Some(output.to_bits())
});

if result.is_ok() {
break;
}
}
}

fn set(&self, value: f64) {
let _ = self.swap(value.to_bits(), Ordering::AcqRel);
}
}

#[cfg(feature = "std-atomics")]
impl CounterFn for std::sync::atomic::AtomicU64 {
fn increment(&self, value: u64) {
let _ = self.fetch_add(value, Ordering::Release);
}

fn absolute(&self, value: u64) {
let _ = self.fetch_max(value, Ordering::AcqRel);
}
}

#[cfg(feature = "std-atomics")]
impl GaugeFn for std::sync::atomic::AtomicU64 {
fn increment(&self, value: f64) {
loop {
let result = self.fetch_update(Ordering::AcqRel, Ordering::Relaxed, |curr| {
let input = f64::from_bits(curr);
let output = input + value;
Some(output.to_bits())
});

if result.is_ok() {
break;
}
}
}

fn decrement(&self, value: f64) {
loop {
let result = self.fetch_update(Ordering::AcqRel, Ordering::Relaxed, |curr| {
let input = f64::from_bits(curr);
let output = input - value;
Some(output.to_bits())
});

if result.is_ok() {
break;
}
}
}

fn set(&self, value: f64) {
let _ = self.swap(value.to_bits(), Ordering::AcqRel);
}
}

impl<T> CounterFn for Arc<T>
where
T: CounterFn,
Expand Down
10 changes: 4 additions & 6 deletions metrics/src/key.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
use crate::{cow::Cow, IntoLabels, KeyHasher, Label, SharedString};
use alloc::{string::String, vec::Vec};
use core::{fmt, hash::Hash, slice::Iter};
use portable_atomic::AtomicU64;
use crate::{atomics::AtomicU64, cow::Cow, IntoLabels, KeyHasher, Label, SharedString};
use std::{
cmp,
hash::Hasher,
cmp, fmt,
hash::{Hash, Hasher},
slice::Iter,
sync::atomic::{AtomicBool, Ordering},
};

Expand Down
1 change: 0 additions & 1 deletion metrics/src/label.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::slice::Iter;

use crate::SharedString;
use alloc::vec::Vec;

/// Metadata for a metric key in the form of a key/value pair.
///
Expand Down
2 changes: 1 addition & 1 deletion metrics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@
#![deny(missing_docs)]
#![cfg_attr(docsrs, feature(doc_cfg), deny(rustdoc::broken_intra_doc_links))]

extern crate alloc;
pub mod atomics;

mod common;
pub use self::common::*;
Expand Down
9 changes: 6 additions & 3 deletions metrics/src/recorder.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use std::fmt;

use self::cell::RecorderOnceCell;
use crate::{Counter, Gauge, Histogram, Key, KeyName, SharedString, Unit};
use core::fmt;

mod cell {
use super::{Recorder, SetRecorderError};
use std::cell::UnsafeCell;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::{
cell::UnsafeCell,
sync::atomic::{AtomicUsize, Ordering},
};

// FIXME: This can't be a const new function because trait objects aren't allowed in const fns
// This was stabilized in 1.61, so it can be cleaned up when it becomes the MSRV
Expand Down

0 comments on commit a3a706b

Please # to comment.