Prototype bound instruments#8314
Conversation
…bound-instruments-1
|
For the bound=false case, 1/118,208,794 = 8 ns. Dang. Are java concurrent map lookups just that fast? |
That case has no concurrency (threads=1). We optimize map lookups slightly by caching the hashcode of our Attribute implementation. That number (and all of them frankly) is suspiciously fast, so I'm double checking things. Things are checking out initially. There is an issue with the cardinality=1 case, where its possible the JIT compiler is lifting hositing the map lookup, but its possible the JIT compiler could do that in a real application in a cardinality=1 case as well, so not wrong per say. But even the cardinality=128 cases where a JIT hoist is unlikely are blazing fast so speed can't be only attributed to JIT. I ran those benchmarks on my mac mini, which uses apple m4 chip. Currently on the main branch, running on the dedicated benchmark bare metal hardware, that same series gets https://open-telemetry.github.io/opentelemetry-java/benchmarks/ If you spot any problems with the methodology of MetricRecordBenchmark, please let me know. |
|
Yeah, I couldn't find any problem with the methodology or the code. For Go, the map lookup takes ~20 ns, and ~45 ns with high concurrency, and the atomic counter increment takes < 10 ns, so we see a bigger difference. I was curious if you had any tricks up your sleeve, or if java maps were just faster |
I was curious about the map lookup perf as well, so created a dedicated benchmark based on it: jack-berg@b9cf4c4 Parameters I test:
Results: threads=1 — ns/op
threads=4 — (4-thread aggregate; multiply by 4 for per-thread cost)
† ±11–16% variance ††† ±46% variance (discard) So lookups are really fast. Caching hashCodes matters a lot, especially as the size of keys becomes larger (this is intuitive). Cardinality matters a little bit, but not as much as key size. I only tested up to 1024, but given that default cardinality limit is 2k, I think this reasonably represents the use case. Taking these conclusions back to bound instruments, I think the benchmark setup I have for MetricRecordBenchmark is reasonable. The cardinality is small (128) and attributes are small (just 1*26 char key), but since we cache hashCode, those don't matter that much. I could increase cardinality and attribute size to increase the positive impact of bound instruments. |
…y-java into prototype-bound-instruments-1
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (62.87%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #8314 +/- ##
============================================
- Coverage 78.54% 78.35% -0.19%
- Complexity 8473 8513 +40
============================================
Files 1008 1008
Lines 28830 29055 +225
Branches 3570 3581 +11
============================================
+ Hits 22644 22766 +122
- Misses 5341 5443 +102
- Partials 845 846 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Resolves open-telemetry#4126. See issue for full details, but some of the key bits: - Adds new optional "bind" capability to synchronous instruments - Marked as "in development" - SDK implementation requirements phrase it as "A bound instrument MUST behave identically to calling the equivalent unbound recording operation with the pre-bound Attributes on each measurement". That is, the same behavior with respect to cardinality limits, exemplars, and anything else relevant. Since attributes are pre-bound, this excludes attributes processing from views, which is done at bind time. There have been a few prototypes built: - Java: open-telemetry/opentelemetry-java#8314 - Rust: open-telemetry/opentelemetry-rust#3421 - Go: open-telemetry/opentelemetry-go#8278 I'm particularly interested to here from @dashpole and @cijothomas if this aligns with their prototypes and ideas. Other interest has been expressed in .NET and Erlang, but no prototypes that I'm aware of.
…y-java into prototype-bound-instruments-1
2 participants
Builds off of #8308, #8313.
Related to open-telemetry/opentelemetry-specification#4126
Usage example:
MetricRecordBenchmark has been updated with a new
isBound=true|falseparameter. The following characterizes the change in performance fromisBound=falsetoisBound=true:Modest to large gains across the board, with larger gains for cases with reduced contention and cumulative temporality, where the map lookup represents a larger share of the time to record.
Leaving as draft because: