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

Add an optimized Attributes implementation for instrumenter #3136

Merged
merged 1 commit into from
May 31, 2021

Conversation

anuraaga
Copy link
Contributor

Now that this is being used in much of our instrumentation better fix the performance regression :)

After

Benchmark                                                    Mode  Cnt          Score          Error   Units
InstrumenterBenchmark.start                                  avgt   15          0.229 ±        0.021   us/op
InstrumenterBenchmark.start:heap.total.after                 avgt   15  203703364.267 ± 36000273.266   bytes
InstrumenterBenchmark.start:heap.total.before                avgt   15   30129083.733 ±  2825338.757   bytes
InstrumenterBenchmark.start:heap.used.after                  avgt   15   62447095.467 ± 42848479.032   bytes
InstrumenterBenchmark.start:heap.used.before                 avgt   15    7324172.800 ±     8812.281   bytes
InstrumenterBenchmark.start:·gc.alloc.rate                   avgt   15       1553.595 ±      222.391  MB/sec
InstrumenterBenchmark.start:·gc.alloc.rate.norm              avgt   15        554.667 ±       41.732    B/op
InstrumenterBenchmark.start:·gc.churn.G1_Eden_Space          avgt   15       1522.278 ±      214.994  MB/sec
InstrumenterBenchmark.start:·gc.churn.G1_Eden_Space.norm     avgt   15        543.720 ±       42.457    B/op
InstrumenterBenchmark.start:·gc.churn.G1_Old_Gen             avgt   15          0.669 ±        0.004  MB/sec
InstrumenterBenchmark.start:·gc.churn.G1_Old_Gen.norm        avgt   15          0.241 ±        0.022    B/op
InstrumenterBenchmark.start:·gc.count                        avgt   15        492.000                 counts
InstrumenterBenchmark.start:·gc.time                         avgt   15        140.000                     ms
InstrumenterBenchmark.startEnd                               avgt   15          0.375 ±        0.007   us/op
InstrumenterBenchmark.startEnd:heap.total.after              avgt   15  197831338.667 ± 15409141.526   bytes
InstrumenterBenchmark.startEnd:heap.total.before             avgt   15   30198988.800 ±  1705339.089   bytes
InstrumenterBenchmark.startEnd:heap.used.after               avgt   15   61750949.867 ± 34775705.363   bytes
InstrumenterBenchmark.startEnd:heap.used.before              avgt   15    7325356.267 ±     8920.324   bytes
InstrumenterBenchmark.startEnd:·gc.alloc.rate                avgt   15       1395.926 ±       26.107  MB/sec
InstrumenterBenchmark.startEnd:·gc.alloc.rate.norm           avgt   15        824.000 ±        0.001    B/op
InstrumenterBenchmark.startEnd:·gc.churn.G1_Eden_Space       avgt   15       1364.868 ±       31.175  MB/sec
InstrumenterBenchmark.startEnd:·gc.churn.G1_Eden_Space.norm  avgt   15        805.695 ±       12.931    B/op
InstrumenterBenchmark.startEnd:·gc.churn.G1_Old_Gen          avgt   15          0.667 ±        0.004  MB/sec
InstrumenterBenchmark.startEnd:·gc.churn.G1_Old_Gen.norm     avgt   15          0.394 ±        0.008    B/op
InstrumenterBenchmark.startEnd:·gc.count                     avgt   15        474.000                 counts
InstrumenterBenchmark.startEnd:·gc.time                      avgt   15        131.000                     ms

Before

Benchmark                                                    Mode  Cnt          Score          Error   Units
InstrumenterBenchmark.start                                  avgt   15          0.755 ±        0.035   us/op
InstrumenterBenchmark.start:heap.total.after                 avgt   15  149177412.267 ±   513120.009   bytes
InstrumenterBenchmark.start:heap.total.before                avgt   15   29150412.800 ±  1539360.028   bytes
InstrumenterBenchmark.start:heap.used.after                  avgt   15   49199561.600 ± 26731691.947   bytes
InstrumenterBenchmark.start:heap.used.before                 avgt   15    7330857.600 ±     7877.344   bytes
InstrumenterBenchmark.start:·gc.alloc.rate                   avgt   15        535.930 ±       23.192  MB/sec
InstrumenterBenchmark.start:·gc.alloc.rate.norm              avgt   15        637.334 ±       41.732    B/op
InstrumenterBenchmark.start:·gc.churn.G1_Eden_Space          avgt   15        507.477 ±       36.403  MB/sec
InstrumenterBenchmark.start:·gc.churn.G1_Eden_Space.norm     avgt   15        603.467 ±       51.943    B/op
InstrumenterBenchmark.start:·gc.churn.G1_Old_Gen             avgt   15          0.662 ±        0.002  MB/sec
InstrumenterBenchmark.start:·gc.churn.G1_Old_Gen.norm        avgt   15          0.787 ±        0.037    B/op
InstrumenterBenchmark.start:·gc.count                        avgt   15        339.000                 counts
InstrumenterBenchmark.start:·gc.time                         avgt   15         86.000                     ms
InstrumenterBenchmark.startEnd                               avgt   15          1.008 ±        0.031   us/op
InstrumenterBenchmark.startEnd:heap.total.after              avgt   15  133868202.667 ± 43551357.060   bytes
InstrumenterBenchmark.startEnd:heap.total.before             avgt   15   30059178.667 ±  2346316.128   bytes
InstrumenterBenchmark.startEnd:heap.used.after               avgt   15   45406940.267 ± 22520874.174   bytes
InstrumenterBenchmark.startEnd:heap.used.before              avgt   15    7333456.000 ±     8444.228   bytes
InstrumenterBenchmark.startEnd:·gc.alloc.rate                avgt   15        554.537 ±       26.649  MB/sec
InstrumenterBenchmark.startEnd:·gc.alloc.rate.norm           avgt   15        880.000 ±       50.078    B/op
InstrumenterBenchmark.startEnd:·gc.churn.G1_Eden_Space       avgt   15        532.234 ±       35.156  MB/sec
InstrumenterBenchmark.startEnd:·gc.churn.G1_Eden_Space.norm  avgt   15        844.567 ±       60.364    B/op
InstrumenterBenchmark.startEnd:·gc.churn.G1_Old_Gen          avgt   15          0.663 ±        0.002  MB/sec
InstrumenterBenchmark.startEnd:·gc.churn.G1_Old_Gen.norm     avgt   15          1.052 ±        0.034    B/op
InstrumenterBenchmark.startEnd:·gc.count                     avgt   15        349.000                 counts
InstrumenterBenchmark.startEnd:·gc.time                      avgt   15         88.000                     ms

* couple of methods still require copying to satisfy the interface contracts, but in practice
* should never be called by user code even though they can.
*/
final class UnsafeAttributes extends HashMap<AttributeKey<?>, Object>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkwatson It's your favorite pattern again :)

Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


Context context = parentContext;

for (RequestListener requestListener : requestListeners) {
context = requestListener.start(context, attributes);
}

attributes.forEach((key, value) -> spanBuilder.setAttribute((AttributeKey) key, value));
spanBuilder.setAllAttributes(attributes);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any ideas how we can init the SpanBuilder with the already built Attributes map, to avoid this copy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm - I guess it requires an internal use API for that in the SDK. We can try it but it'll be more hacky.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to clarify, did not mean for this pr, something (possibly) for the future..

@anuraaga anuraaga merged commit c3ed9a3 into open-telemetry:main May 31, 2021
robododge pushed a commit to robododge/opentelemetry-java-instrumentation that referenced this pull request Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants