Skip to content

Conversation

gruebel
Copy link
Member

@gruebel gruebel commented Mar 8, 2025

Description

  • During my work on stabilizing baggage in the Rust project I checked the implementation in other languages and saw some possible improvements.
  • The suggested implementation keeps the original intent by not exposing the mutable baggage dict and creates a copy before modifying it. If you are ok with it, then I can also remove the copy() and directly manipulate the baggage in the context, which would further improve the performance.
  • The current approach improves the performance by roughly 10x

you can easily test it

    import timeit
    from types import MappingProxyType

    data = {i: i for i in range(10)}  # if the baggage has more values, then performance gain becomes even bigger

    time_proxy = timeit.timeit(lambda: dict(MappingProxyType(data)), number=100000)
    time_copy = timeit.timeit(lambda: data.copy(), number=100000)

    print(f"proxy: {time_proxy:.6f} seconds")
    print(f"copy: {time_copy:.6f} seconds")
    
    -> proxy: 0.042052 seconds
    -> copy: 0.004569 seconds

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@gruebel gruebel requested a review from a team as a code owner March 8, 2025 21:26
Copy link
Member

@emdneto emdneto left a comment

Choose a reason for hiding this comment

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

Interesting. I was curious about the results for larger baggage set, and opened #4468 with a benchmark so we can compare the results.

But, in summary the results for baggage size of 10000 were improved by ~8.4x

@xrmx
Copy link
Contributor

xrmx commented Mar 11, 2025

@gruebel Please add a changelog entry.

This also shows that we have a very broad typing 😅

@xrmx xrmx moved this to Ready for review in @xrmx's Python PR digest Mar 11, 2025
@gruebel
Copy link
Member Author

gruebel commented Mar 11, 2025

@xrmx done 🙂

@gruebel
Copy link
Member Author

gruebel commented Mar 15, 2025

@emdneto @lzchen is anything missing or is it good to merge? 😄

@xrmx xrmx merged commit 6394d2b into open-telemetry:main Mar 17, 2025
385 checks passed
@github-project-automation github-project-automation bot moved this from Ready for review to Done in @xrmx's Python PR digest Mar 17, 2025
([#4458](https://github.com/open-telemetry/opentelemetry-python/pull/4458))
- pylint-ci updated python version to 3.13
([#4450](https://github.com/open-telemetry/opentelemetry-python/pull/4450))
- Improve performance of baggage operations
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong release 😢

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, sorry 🙈

xrmx added a commit that referenced this pull request Mar 17, 2025
xrmx added a commit that referenced this pull request Mar 17, 2025
@gruebel gruebel deleted the improve-baggage branch March 18, 2025 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants