-
Notifications
You must be signed in to change notification settings - Fork 166
fix(profiling): check long string before allocating #3561
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?
Conversation
|
Benchmarks [ profiler ]Benchmark execution time: 2026-01-09 21:16:06 Comparing candidate commit 1890aeb in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 29 metrics, 7 unstable metrics. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3561 +/- ##
==========================================
- Coverage 61.91% 61.90% -0.02%
==========================================
Files 140 140
Lines 13281 13281
Branches 1758 1758
==========================================
- Hits 8223 8221 -2
- Misses 4269 4272 +3
+ Partials 789 788 -1 see 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Description
It's rare but we sometimes see
extract_function_namein crash reports due to failed memory allocations. At the moment, refactoring this function return aResult<T, E>instead ofTwould be difficult because of its callers likecommon_labelswhich can't fail either.But I did notice an easy fix for some cases: move the large string check earlier. This won't fix OOMs due to genuinely hitting the memory limits, but it will reduce the damage caused by bugs letting through large strings in the first place, which are usually errors.
I also renamed
[large string]to[suspiciously large string]to indicate that these are typically bugs. Class + method names are typically not 64 KiB or larger in size, nor are file names.Reviewer checklist