[Feature]: Lock-free Metrics Counting API

[shaun-cox]

Related Problems?

The general problem is one of minimizing the performance overheads of instrumentation and delivering on the mission.

#1374

Describe the solution you'd like:

If the API could take advantage of Rust type-system features to distinguish when in a "thread-local" context where things are !Send, then modifying metrics could be even more performant than they are today. i.e. not baking in the assumptions of Arc<Mutex<State>> when Rc<RefCell<State>> would do.

Considered Alternatives

Additional Context

No response

[jtescher]

👍 would be great to experiment with a LocalMeter or similar that could expose instruments that are not send nor sync. Could still have them get picked up by global metric readers through channels or other means of communication, would just need to be careful to aggregate them across threads as instruments could have similar names/attributes.

[cijothomas]

Per-Thread-Aggregators+ something which merges aggregations across the threads would be awesome (I tried implementing it for .NET, the merging-across-threads during export was tricky to implement)

[KallDrexx]

Edit: Ignore this. Not great 2am thought after thinking about it more

Ok here's a crazy thought. Keep in mind I'm still working my way through the code base and OpenTelemetry terminology, so feel free to tell me I'm way off.

Today when you create a Counter, it's ultimately given the Sum::measure() call as it's Measure<T>. Sum::measure() in turn calls it's internal ValueMap::measure(). The ValueMap is ultimately what holds all the attribute sets and their associated value, and the locking mechanism.

Right now we heavily rely on locking because ValueMap is the choke point for aggregation. Bounded instruments can help alleviate this but that only works for stable attributes. So while it may not be completely lock free, what if we can utilize atomics to reduce how often we have to lock?

How does this look in practice? My current thought is along the lines of:

• Counter<T> is created. This creates it's own HashMap<AttributeSet, Arc<Atomic<T>>.
• Counter<T>.add() is changed to take a mutable reference to self (meaning you must have an exclusive reference to that specific counter instance in order to call add()).
• Calling Counter::add() looks up the passed in attribute sets against it's local hash map.
  • If we have an entry for this attribute set, call fetch_add() on the atomic
  • if we do not have an entry for it
    • Call Sum::get_atomic() which calls ValueMap::get_atomic()
    • This acquires a lock, looks up the attribute set in the hashmap, and returns the appropriate atomic (or creates one and returns it if one does not already exist)
    • The counter then stores the Arc<Atomic<T>> in its local hashmap and calls fetch_add() on it.
• When the metrics collection process occurs, Sum::delta and its other methods are invoked as normal. This will acquire a lock and fetch the value from each atomic in the hashmap
• Whenever values need to be reset, swap() is called on each atomic

There are some internal trait modifications that would need to go into making this work, but I think it follows the current architecture.

While this isn't lock free, Counters only encounter a lock when they have add() invoked with an attribute set they have never seen before. Caching a single Counter<T> per thread makes this interesting. So there could be lock contention early on in the application's life while each counter gets distinct properties, but it should even out eventually.

Likewise, the metrics collection process will lock the whole ValueMap still, but that lock does not prevent incrementing any counters for attribute sets those counters already have.

This also enables some interesting scenarios, such as cleanup. When you drop a Counter, it drops all of it's Arc<Atomic<T> instances it contains. So when metrics collection/reset happens, it can check if the ValueMap's Arc::strong_count == 1. If so then it knows it has the only reference to the atomic for the counter and can clear it from it's list, as it knows the attribute set is currently not used by any counters.

This also dovetails into bounded instruments, because a bounded instrument is just a single counter with a single Arc<Atomic<T>>. So ti has no hashmap lookups and just increments the counter as needed without any locks (and has barely any other logic besides that).

This is all predicated that atomics are performant enough, and that the atomig crate works well (since otherwise we don't have generic atomics).

[fly3366]

+lgtm

anytime high perf application such like a proxy or network switch need thread local metrics. And merge all on global collect. even like local lock free tracer?

[fly3366]

may same like LocalConuter which impl in tikv-prometheus?

[cijothomas]

Not planning for 1.0 stable, removing milestone. This is an important feature, but not mandatory for declaring 1.0 stable.

[martintmk]

The issue peaked my interested. I was evaluating the effects on performance when using the Open Telemetry types in single-threaded runtimes. These types use multi-threaded and are using atomics and locks internally. To see the effect on performance I created a simple scenario:

How many metrics can be reported from 10 threads in 2 minutes?

• Scenario 1: Each threads uses the shared instance of MeterProvider
• Scenario 2: Each threads uses it's own instance of MeterProvider

The code for the scenario is done in #2651

The results are quite positive:

Scenario	Mode	Duration	Threads	Reported Metrics (millions)	Improvement Factor
Scenario 1	Shared	120s	10	388	Baseline
Scenario 2	PerThread	120s	10	3354	~8.66x faster

This indicates that single-threaded runtimes can just create a per-thread instances of Open Telemetry types and get almost linear scalability.

Following these results, I was also interested what how using types such as Arc and Mutex affects the performance. In Scenario 3, I replaced the Mutex with RefCell and Arc with Rc and run the test again.

The results are even more surprising:

Scenario	Mode	Duration	Threads	Reported Metrics (millions)	Speedup vs Shared
1	Shared	120s	10	388	1x (baseline)
2	PerThread	120s	10	3354	~8.64x faster
3	PerThread-NoLocks-NoAtomics	120s	10	3358	~8.65x faster

Mutexes and atomics impose minimal performance overhead when there is no contention. The key to improved performance is reducing contention by using per-thread instances of OpenTelemetry types.

Based on the experiments described above, I conclude that implementing dedicated thread-local types, such as LocalMeter and LocalMeterProvider, offers only marginal performance benefits.

One downside of using per-thread instances is that each thread maintains its own aggregates, which multiplies the amount of reported data by the number of threads. In scenarios where each thread experiences high load, additional global aggregation may be unnecessary since the metrics are already aggregated and the extra data volume is minimal. However, this aspect warrants further exploration.

cc @shaun-cox, @cijothomas

[sandersaares]

The vast majority of OTel time spent in this benchmark is on "HashMap stuff". I suspect this part of OTel is likely to be suboptimal and could benefit from significant optimization work to speed it up. Now, that is an entirely separate topic but it means that any effects we may see from synchronization are "lost in the noise" because the CPU is busy calculating hashes and comparing strings for almost the entire benchmark duration. Even HashMap::get() alone takes up 69.9% of the CPU time in this benchmark.

I recommend we remove from scope this "HashMap stuff" under the assumption that vast overhead will significantly reduce with future optimizations and try narrow down the benchmark scenario on the part of OTel that is affected by synchronization considerations. Potentially this could be achieved by not attaching labels to the events, so there is not much data to hash/compare and the HashMap part of the workload is simpler? I only speculate here.

It strikes me that the inner loop in these benchmarks checks for now.elapsed() on every iteration - getting the current timestamp can be a rather expensive operation. If we "zoom in" to the workload that is not "HashMap stuff" then this might start to become a significant overhead that we might best eliminate.

Right now, this elapsed() takes up 8.1% of the CPU time spent in the benchmark. We could do batches of 10K measurements between each check of the timestamp to eliminate this factor.

Between the "HashMap stuff" and the `elapsed()´, I can see a small 5-ish percent of actual "measurement work" but to be honest this currently dissolves into the noise under all the overhead. I will try do a few quick adjustments and see if I can bring it more to the surface.

Aside, I also suggest we benchmark the effects of data publishing, if that should prove feasible somehow. In the current benchmark, there is no data publishing, right? I see a ManualReader created but nothing reads the data from it. Is reading the data equally fast with both modes? Even though reading is a rare event, relatively speaking, what happens when it does occur? For example, does reading the data cause any contention or latency spike in data producers? Might be difficult to measure but it is a potential question of interest - writers should not be penalized much when a read happens to coincide with a write.