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.

Sign up for GitHub

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

Jump to bottom

Low allocation OTLP trace marshaler #6410

Merged
merged 4 commits into from
May 1, 2024
Merged

Low allocation OTLP trace marshaler #6410

merged 4 commits into from
May 1, 2024

Conversation

laurit
Copy link
Contributor

@laurit laurit commented Apr 26, 2024

Subset of #6328 that contains only trace marshaling.

Benchmark                                                                     (numSpans)  Mode  Cnt        Score     Error   Units
RequestMarshalBenchmarks.createCustomMarshal                                          16  avgt   10        6.150 ±   0.715   us/op
RequestMarshalBenchmarks.createCustomMarshal:gc.alloc.rate                            16  avgt   10     2835.838 ± 315.210  MB/sec
RequestMarshalBenchmarks.createCustomMarshal:gc.alloc.rate.norm                       16  avgt   10    18200.003 ±   0.001    B/op
RequestMarshalBenchmarks.createCustomMarshal:gc.count                                 16  avgt   10       78.000            counts
RequestMarshalBenchmarks.createCustomMarshal:gc.time                                  16  avgt   10       77.000                ms
RequestMarshalBenchmarks.createCustomMarshal                                         512  avgt   10      186.124 ±   8.696   us/op
RequestMarshalBenchmarks.createCustomMarshal:gc.alloc.rate                           512  avgt   10     2873.058 ± 126.706  MB/sec
RequestMarshalBenchmarks.createCustomMarshal:gc.alloc.rate.norm                      512  avgt   10   560383.738 ±  17.191    B/op
RequestMarshalBenchmarks.createCustomMarshal:gc.count                                512  avgt   10       71.000            counts
RequestMarshalBenchmarks.createCustomMarshal:gc.time                                 512  avgt   10       82.000                ms
RequestMarshalBenchmarks.createCustomMarshalLowAllocation                             16  avgt   10        3.822 ±   0.112   us/op
RequestMarshalBenchmarks.createCustomMarshalLowAllocation:gc.alloc.rate               16  avgt   10        0.001 ±   0.001  MB/sec
RequestMarshalBenchmarks.createCustomMarshalLowAllocation:gc.alloc.rate.norm          16  avgt   10        0.002 ±   0.001    B/op
RequestMarshalBenchmarks.createCustomMarshalLowAllocation:gc.count                    16  avgt   10          ≈ 0            counts
RequestMarshalBenchmarks.createCustomMarshalLowAllocation                            512  avgt   10      137.433 ±  10.486   us/op
RequestMarshalBenchmarks.createCustomMarshalLowAllocation:gc.alloc.rate              512  avgt   10        0.001 ±   0.001  MB/sec
RequestMarshalBenchmarks.createCustomMarshalLowAllocation:gc.alloc.rate.norm         512  avgt   10        0.074 ±   0.014    B/op
RequestMarshalBenchmarks.createCustomMarshalLowAllocation:gc.count                   512  avgt   10          ≈ 0            counts
RequestMarshalBenchmarks.marshalCustom                                                16  avgt   10       13.325 ±   0.362   us/op
RequestMarshalBenchmarks.marshalCustom:gc.alloc.rate                                  16  avgt   10     1314.425 ±  35.678  MB/sec
RequestMarshalBenchmarks.marshalCustom:gc.alloc.rate.norm                             16  avgt   10    18368.007 ±   0.001    B/op
RequestMarshalBenchmarks.marshalCustom:gc.count                                       16  avgt   10       44.000            counts
RequestMarshalBenchmarks.marshalCustom:gc.time                                        16  avgt   10       47.000                ms
RequestMarshalBenchmarks.marshalCustom                                               512  avgt   10      482.968 ±  11.295   us/op
RequestMarshalBenchmarks.marshalCustom:gc.alloc.rate                                 512  avgt   10     1106.838 ±  25.556  MB/sec
RequestMarshalBenchmarks.marshalCustom:gc.alloc.rate.norm                            512  avgt   10   560544.256 ±   0.008    B/op
RequestMarshalBenchmarks.marshalCustom:gc.count                                      512  avgt   10       37.000            counts
RequestMarshalBenchmarks.marshalCustom:gc.time                                       512  avgt   10       41.000                ms
RequestMarshalBenchmarks.marshalCustomLowAllocation                                   16  avgt   10       14.876 ±   1.089   us/op
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.alloc.rate                     16  avgt   10        5.652 ±   0.394  MB/sec
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.alloc.rate.norm                16  avgt   10       88.008 ±   0.001    B/op
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.count                          16  avgt   10        2.000            counts
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.time                           16  avgt   10        5.000                ms
RequestMarshalBenchmarks.marshalCustomLowAllocation                                  512  avgt   10      525.182 ±  53.841   us/op
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.alloc.rate                    512  avgt   10        0.161 ±   0.016  MB/sec
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.alloc.rate.norm               512  avgt   10       88.270 ±   0.027    B/op
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.count                         512  avgt   10          ≈ 0            counts
RequestMarshalBenchmarks.marshalJson                                                  16  avgt   10       50.843 ±   1.571   us/op
RequestMarshalBenchmarks.marshalJson:gc.alloc.rate                                    16  avgt   10      615.603 ±  18.741  MB/sec
RequestMarshalBenchmarks.marshalJson:gc.alloc.rate.norm                               16  avgt   10    32816.027 ±   0.003    B/op
RequestMarshalBenchmarks.marshalJson:gc.count                                         16  avgt   10       20.000            counts
RequestMarshalBenchmarks.marshalJson:gc.time                                          16  avgt   10       23.000                ms
RequestMarshalBenchmarks.marshalJson                                                 512  avgt   10     1564.466 ±  24.863   us/op
RequestMarshalBenchmarks.marshalJson:gc.alloc.rate                                   512  avgt   10      661.690 ±  10.372  MB/sec
RequestMarshalBenchmarks.marshalJson:gc.alloc.rate.norm                              512  avgt   10  1085588.150 ±  15.889    B/op
RequestMarshalBenchmarks.marshalJson:gc.count                                        512  avgt   10       23.000            counts
RequestMarshalBenchmarks.marshalJson:gc.time                                         512  avgt   10       27.000                ms
RequestMarshalBenchmarks.marshalJsonLowAllocation                                     16  avgt   10       51.248 ±   0.936   us/op
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.alloc.rate                       16  avgt   10      173.268 ±   3.179  MB/sec
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.alloc.rate.norm                  16  avgt   10     9312.028 ±   0.002    B/op
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.count                            16  avgt   10        5.000            counts
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.time                             16  avgt   10        5.000                ms
RequestMarshalBenchmarks.marshalJsonLowAllocation                                    512  avgt   10     1680.969 ± 147.464   us/op
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.alloc.rate                      512  avgt   10      203.218 ±  17.142  MB/sec
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.alloc.rate.norm                 512  avgt   10   357220.414 ±  16.837    B/op
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.count                           512  avgt   10        7.000            counts
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.time                            512  avgt   10        8.000                ms

@laurit laurit requested a review from a team April 26, 2024 06:51
@codecov Codecov
Copy link

codecov bot commented Apr 26, 2024 •
edited
Loading

Codecov Report

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

Project coverage is 90.79%. Comparing base (51ff6bc) to head (b9aa0fb).
Report is 2 commits behind head on main.

Files Patch % Lines
...elemetry/exporter/internal/marshal/Serializer.java 59.75% 29 Missing and 4 partials ⚠️
...orter/internal/marshal/StatelessMarshalerUtil.java 79.74% 26 Missing and 6 partials ⚠️
...ry/exporter/internal/marshal/MarshalerContext.java 73.11% 22 Missing and 3 partials ⚠️
...rter/internal/otlp/KeyValueStatelessMarshaler.java 73.33% 6 Missing and 6 partials ⚠️
...internal/otlp/ArrayAnyValueStatelessMarshaler.java 77.77% 2 Missing and 2 partials ⚠️
...tlp/traces/LowAllocationTraceRequestMarshaler.java 76.47% 3 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #6410      +/-   ##
============================================
- Coverage     91.12%   90.79%   -0.34%     
- Complexity     5856     6013     +157     
============================================
  Files           636      651      +15     
  Lines         17062    17709     +647     
  Branches       1733     1769      +36     
============================================
+ Hits          15548    16079     +531     
- Misses         1019     1113      +94     
- Partials        495      517      +22

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

@breedx-splk
Copy link
Contributor

Rebase or merge #6411 ?

@laurit laurit force-pushed the low-alloc-otlp-trace branch from cba329d to 3dc6585 Compare April 26, 2024 20:40
jack-berg
jack-berg reviewed Apr 26, 2024
View reviewed changes
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.

This is looking really good! Left several comment but they all should be easy to resolve.

@jack-berg
Copy link
Member

This is the part of the benchmark which gets me excited. marshalCustom is the current stateful marshaling. marshalCustomLowAllocation is what you've prototyped here.

  • Time to marshal shows a small dip from 13.325 to 14.876
  • Normalized gc allocation drops from 18,368 down to 88! Awesome!
RequestMarshalBenchmarks.marshalCustom                                                16  avgt   10       13.325 ±   0.362   us/op
RequestMarshalBenchmarks.marshalCustom:gc.alloc.rate                                  16  avgt   10     1314.425 ±  35.678  MB/sec
RequestMarshalBenchmarks.marshalCustom:gc.alloc.rate.norm                             16  avgt   10    18368.007 ±   0.001    B/op
RequestMarshalBenchmarks.marshalCustom:gc.count                                       16  avgt   10       44.000            counts
RequestMarshalBenchmarks.marshalCustom:gc.time                                        16  avgt   10       47.000                ms
RequestMarshalBenchmarks.marshalCustomLowAllocation                                   16  avgt   10       14.876 ±   1.089   us/op
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.alloc.rate                     16  avgt   10        5.652 ±   0.394  MB/sec
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.alloc.rate.norm                16  avgt   10       88.008 ±   0.001    B/op
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.count                          16  avgt   10        2.000            counts
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.time                           16  avgt   10        5.000                ms

jack-berg
jack-berg reviewed Apr 29, 2024
View reviewed changes
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.

Just a couple of loose ends at this point.

jack-berg
jack-berg approved these changes Apr 29, 2024
View reviewed changes
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.

Great stuff! Very excited about this!

@jack-berg
Copy link
Member

@open-telemetry/java-approvers does anyone intend on reviewing this? If not, I'll merge so we can begin the followup work.

@jack-berg jack-berg merged commit 2e59f54 into open-telemetry:main May 1, 2024
18 checks passed
@laurit laurit deleted the low-alloc-otlp-trace branch May 2, 2024 08:55
This was referenced May 2, 2024
Merged
Merged
This was referenced May 3, 2024
Merged
Merged
Closed
Closed
@jack-berg jack-berg mentioned this pull request May 24, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jack-berg jack-berg jack-berg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@laurit @breedx-splk @jack-berg