Skip to content

Box complex types in AnyValue enum (Improve perf by 10%, and size reduction by ~60%)#1993

Merged
lalitb merged 8 commits intoopen-telemetry:mainfrom
lalitb:box-log-anyvalue
Aug 8, 2024
Merged

Box complex types in AnyValue enum (Improve perf by 10%, and size reduction by ~60%)#1993
lalitb merged 8 commits intoopen-telemetry:mainfrom
lalitb:box-log-anyvalue

Conversation

@lalitb
Copy link
Member

@lalitb lalitb commented Aug 7, 2024

Fixes #1903

Changes

The AnyValue enum has been modified to box complex types, reducing the effective size of the enum for commonly used scenarios like integers and strings.

Existing:

#[derive(Debug, Clone, PartialEq)]
pub enum AnyValue {
    /// An integer value
    Int(i64),
    /// A double value
    Double(f64),
    /// A string value
    String(StringValue),
    /// A boolean value
    Boolean(bool),
    /// A byte array
    Bytes(Box<Vec<u8>>),
    /// An array of `Any` values
    ListAny(Box<Vec<AnyValue>>),
    /// A map of string keys to `Any` values, arbitrarily nested.
    Map(Box<HashMap<Key, AnyValue>>),
}

Modified:

#[derive(Debug, Clone, PartialEq)]
pub enum AnyValue {
    /// An integer value
    Int(i64),
    /// A double value
    Double(f64),
    /// A string value
    String(StringValue),
    /// A boolean value
    Boolean(bool),
    /// A byte array
    Bytes(Vec<u8>),
    /// An array of `Any` values
    ListAny(Vec<AnyValue>),
    /// A map of string keys to `Any` values, arbitrarily nested.
    Map(HashMap<Key, AnyValue>),
}

Since the size of an enum in Rust is determined by the size of its largest variant, boxing the complex types (which are rarely used) can reduce the effective size of the AnyValue enum for the more frequently used cases like integers and strings.

Size Comparison
Refer - https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9d2c7b8cc2a603afe23284816faae242

Without Box: 56 bytes
With Box: 24 bytes

By boxing, the overall size of the enum is reduced, leading to potential performance improvements. Complex types already allocate data on the heap; boxing simply means that both the data and metadata will now be heap-allocated.

Performance Benchmark:

opentelemetry-sdk/benches - full-log-with-4-attributes
PR: 116.32 ns
Main: 127.22 ns

opentelemetry-appender-tracing - ot-layer-enabled
PR: 305.00 ns
Main: 325.00 ns

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@lalitb lalitb requested a review from a team August 7, 2024 18:25
@lalitb lalitb changed the title Box complex types in AnyValue enum Box complex types in AnyValue enum (Improve perf by 10%, and size reduction by ~60%) Aug 7, 2024
@codecov
Copy link

codecov bot commented Aug 7, 2024

Codecov Report

Attention: Patch coverage is 83.78378% with 6 lines in your changes missing coverage. Please review.

Project coverage is 75.1%. Comparing base (fe10ab1) to head (5af3b7d).

Files Patch % Lines
opentelemetry/src/logs/record.rs 0.0% 3 Missing ⚠️
opentelemetry-appender-log/src/lib.rs 96.8% 1 Missing ⚠️
opentelemetry-proto/src/transform/logs.rs 0.0% 1 Missing ⚠️
opentelemetry-stdout/src/common.rs 0.0% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1993   +/-   ##
=====================================
  Coverage   75.1%   75.1%           
=====================================
  Files        122     122           
  Lines      20633   20642    +9     
=====================================
+ Hits       15497   15506    +9     
  Misses      5136    5136           

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

Copy link
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

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

Thanks for continuing to make perf get better!
LGTM with a changelog entry.

Could you also share stress tests with memory stats to see if it can show the improvement in memory as well?

@lalitb
Copy link
Member Author

lalitb commented Aug 7, 2024

Thanks for continuing to make perf get better! LGTM with a changelog entry.

Could you also share stress tests with memory stats to see if it can show the improvement in memory as well?

Here are the memory stats.

PR:

Throughput: 37,162,800 iterations/sec
Memory usage: 3.62 MB
CPU usage: 99.490875%
Virtual memory usage: 2249.08 MB


Throughput: 37,184,400 iterations/sec
Memory usage: 3.62 MB
CPU usage: 100%
Virtual memory usage: 2249.08 MB

Main:

Throughput: 35,348,600 iterations/sec
Memory usage: 4.12 MB
CPU usage: 99.83837%
Virtual memory usage: 2249.08 MB


Throughput: 36,159,000 iterations/sec
Memory usage: 4.12 MB
CPU usage: 99.863014%
Virtual memory usage: 2249.08 MB


Throughput: 36,223,800 iterations/sec
Memory usage: 4.25 MB
CPU usage: 99.78797%
Virtual memory usage: 2249.08 MB

, I think we can get better/detailed insights by integrating custom allocator (jemalloc) with the stress test

lalitb and others added 3 commits August 7, 2024 15:55
Co-authored-by: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com>
@lalitb lalitb merged commit 2409c18 into open-telemetry:main Aug 8, 2024
takumi-earth pushed a commit to earthlings-dev/opentelemetry-rust that referenced this pull request Jan 27, 2026
…uction by ~60%) (open-telemetry#1993)

Co-authored-by: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com>
Co-authored-by: Cijo Thomas <cijo.thomas@gmail.com>
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.

Performance Degradation with AnyValue Enum

3 participants