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. |
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:isBound=truehave very high variance (>20%) — marked with † — treat those deltas with caution.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: