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

Low allocation OTLP metrics marshaler #6422

Merged
merged 2 commits into from
May 7, 2024

Conversation

laurit
Copy link
Contributor

@laurit laurit commented May 2, 2024

Continuation of #6410

Benchmark                                                                 Mode  Cnt     Score    Error   Units
MetricsRequestMarshalerBenchmark.marshalStateful                          avgt   10     1.013 ±  0.021   us/op
MetricsRequestMarshalerBenchmark.marshalStateful:gc.alloc.rate            avgt   10  3080.843 ± 62.442  MB/sec
MetricsRequestMarshalerBenchmark.marshalStateful:gc.alloc.rate.norm       avgt   10  3272.006 ±  0.001    B/op
MetricsRequestMarshalerBenchmark.marshalStateful:gc.count                 avgt   10    92.000           counts
MetricsRequestMarshalerBenchmark.marshalStateful:gc.time                  avgt   10    32.000               ms
MetricsRequestMarshalerBenchmark.marshalStatefulJson                      avgt   10     5.096 ±  0.079   us/op
MetricsRequestMarshalerBenchmark.marshalStatefulJson:gc.alloc.rate        avgt   10  1489.729 ± 23.126  MB/sec
MetricsRequestMarshalerBenchmark.marshalStatefulJson:gc.alloc.rate.norm   avgt   10  7960.030 ±  0.001    B/op
MetricsRequestMarshalerBenchmark.marshalStatefulJson:gc.count             avgt   10    45.000           counts
MetricsRequestMarshalerBenchmark.marshalStatefulJson:gc.time              avgt   10    17.000               ms
MetricsRequestMarshalerBenchmark.marshalStateless                         avgt   10     1.206 ±  0.016   us/op
MetricsRequestMarshalerBenchmark.marshalStateless:gc.alloc.rate           avgt   10    18.991 ±  0.248  MB/sec
MetricsRequestMarshalerBenchmark.marshalStateless:gc.alloc.rate.norm      avgt   10    24.007 ±  0.001    B/op
MetricsRequestMarshalerBenchmark.marshalStateless:gc.count                avgt   10       ≈ 0           counts
MetricsRequestMarshalerBenchmark.marshalStatelessJson                     avgt   10     4.292 ±  0.041   us/op
MetricsRequestMarshalerBenchmark.marshalStatelessJson:gc.alloc.rate       avgt   10   847.853 ±  8.212  MB/sec
MetricsRequestMarshalerBenchmark.marshalStatelessJson:gc.alloc.rate.norm  avgt   10  3816.025 ±  0.001    B/op
MetricsRequestMarshalerBenchmark.marshalStatelessJson:gc.count            avgt   10    25.000           counts
MetricsRequestMarshalerBenchmark.marshalStatelessJson:gc.time             avgt   10    11.000               ms

@laurit laurit requested a review from a team May 2, 2024 09:10
Copy link

codecov bot commented May 2, 2024

Codecov Report

Attention: Patch coverage is 96.95431% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 91.26%. Comparing base (b5699ad) to head (113e663).
Report is 6 commits behind head on main.

Files Patch % Lines
.../metrics/LowAllocationMetricsRequestMarshaler.java 76.47% 3 Missing and 1 partial ⚠️
...nternal/otlp/metrics/MetricStatelessMarshaler.java 94.36% 2 Missing and 2 partials ⚠️
...ernal/otlp/metrics/ExemplarStatelessMarshaler.java 94.73% 0 Missing and 2 partials ⚠️
...orter/internal/otlp/metrics/ExemplarMarshaler.java 80.00% 0 Missing and 1 partial ⚠️
...nternal/otlp/metrics/NumberDataPointMarshaler.java 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6422      +/-   ##
============================================
+ Coverage     90.85%   91.26%   +0.41%     
- Complexity     6028     6107      +79     
============================================
  Files           651      667      +16     
  Lines         17729    18114     +385     
  Branches       1777     1792      +15     
============================================
+ Hits          16107    16532     +425     
+ Misses         1105     1061      -44     
- Partials        517      521       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

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

Left a few straight forward comments, but looks good. Thanks!

public int getBinarySerializedSize(SummaryData summary, MarshalerContext context) {
int size = 0;
size +=
StatelessMarshalerUtil.sizeRepeatedMessageWithContext(
Copy link
Member

Choose a reason for hiding this comment

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

What's the didfference between the sizeRepeatedMessageWithContext overloads? One accepts a MarshalerContext.Key and another doesn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ones with key use forEach to traverse Collection or Map. Key is used to cache the Consumer that is passed to forEach. The ones without key use a counted loop.

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