-
Notifications
You must be signed in to change notification settings - Fork 565
⚡️ Speed up function has_tracing_enabled
by 15%
#4940
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
base: master
Are you sure you want to change the base?
⚡️ Speed up function has_tracing_enabled
by 15%
#4940
Conversation
The optimized code improves performance by restructuring the conditional logic to enable early returns and reduce unnecessary operations: **Key Optimizations:** 1. **Early return for `enable_tracing=False`**: Instead of evaluating the entire boolean expression, the code immediately returns `False` when `enable_tracing` is explicitly `False`. This eliminates the need to check the tracing keys (`traces_sample_rate` and `traces_sampler`) in cases where tracing is disabled. 2. **Removed redundant `bool()` wrapper**: The original code wrapped the entire expression in `bool()`, which adds function call overhead. The optimized version returns boolean values directly from the conditional expressions. 3. **Flattened conditional structure**: The optimized code separates the `enable_tracing` check from the tracing keys check, making the logic flow more linear and avoiding complex nested boolean expressions. **Performance Impact:** The 14% speedup is most pronounced when `enable_tracing=False` (20-29% faster in those test cases), as the function can exit early without performing additional dictionary lookups. Cases with missing or non-False `enable_tracing` values also benefit (8-24% faster) from the cleaner boolean logic and removal of the `bool()` wrapper. **Best suited for:** Applications where tracing is frequently disabled (`enable_tracing=False`) or where the function is called frequently with various option configurations, as the early return pattern and reduced function call overhead provide consistent performance gains across different scenarios.
return bool( | ||
options.get("enable_tracing") is not False | ||
and ( | ||
options.get("traces_sample_rate") is not None | ||
or options.get("traces_sampler") is not None | ||
) | ||
if options.get("enable_tracing") is False: | ||
return False | ||
|
||
return ( | ||
options.get("traces_sample_rate") is not None | ||
or options.get("traces_sampler") is not None | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with removing the bool
wrapper but question this part of the PR description:
Early return for enable_tracing=False: Instead of evaluating the entire boolean expression, the code immediately returns False when enable_tracing is explicitly False. This eliminates the need to check the tracing keys (traces_sample_rate and traces_sampler) in cases where tracing is disabled
Since the and
operator short-circuits in Python, if options.get("enable_tracing")
is False
, then we will not check the tracing keys, since the first part of the and
statement is already False
in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree, python short circuits so should not evaluate the whole expression. the description was generated by codeflash with LLMs so its possible for there to be some gaps. I will update the description.
Although the speedups do materialize, as you can see in the runtime details in the dropdown
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to know how much of the performance improvement can be attributed to removing the bool
wrapper and how much can be attributed to rewriting the expression.
Without justification that rewriting the short-circuiting statement by itself improves performance, I don't see a reason to merge the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I would add that it would be, in any case, preferable to split this PR into multiple PRs, each of which only make one change, rather than combining all of the changes into a single PR.
That way, we can consider each distinct change independently from each other.
📄 15% (0.15x) speedup for
has_tracing_enabled
insentry_sdk/tracing_utils.py
Looks like an important function to me. I have 60 more optimizations for sentry-python at https://github.com/codeflash-ai/sentry-python/pulls that i would like to collaborate with to merge.
⏱️ Runtime :
64.4 microseconds
→56.0 microseconds
(best of65
runs)📝 Explanation and details
The optimized code improves performance by restructuring the conditional logic to enable early returns and reduce unnecessary operations:
Key Optimizations:
Removed redundant
bool()
wrapper: The original code wrapped the entire expression inbool()
, which adds function call overhead. The optimized version returns boolean values directly from the conditional expressions.Flattened conditional structure: The optimized code separates the
enable_tracing
check from the tracing keys check, making the logic flow more linear and avoiding complex nested boolean expressions.Performance Impact:
The 14% speedup is most pronounced when
enable_tracing=False
(20-29% faster in those test cases), as the function can exit early without performing additional dictionary lookups. Cases with missing or non-Falseenable_tracing
values also benefit (8-24% faster) from the cleaner boolean logic and removal of thebool()
wrapper.Best suited for: Applications where tracing is frequently disabled (
enable_tracing=False
) or where the function is called frequently with various option configurations, as the early return pattern and reduced function call overhead provide consistent performance gains across different scenarios.✅ Correctness verification report:
⚙️ Existing Unit Tests and Runtime
test_basics.py::test_option_enable_tracing
🌀 Generated Regression Tests and Runtime
To edit these changes
git checkout codeflash/optimize-has_tracing_enabled-mg9lhdgc
and push.