perf: Optimize lowercase check in RequestInterface#2908
Conversation
Reduce total allocated memory from 442k to 206k bytes (-53.5%) and
objects from 3305 to 1538 (-53.5%) per Rails exception capture.
All changes are internal optimizations with zero behavior changes.
Key optimizations:
- Cache longest_load_path and compute_filename results (class-level,
invalidated on $LOAD_PATH changes)
- Cache backtrace line parsing and Line/Frame object creation (bounded
at 2048 entries)
- Optimize LineCache with Hash#fetch, direct context setting, and
per-(filename, lineno) caching
- Avoid unnecessary allocations: indexed regex captures, match? instead
of =~, byteslice, single-pass iteration in StacktraceBuilder
- RequestInterface: avoid env.dup, cache header name transforms, ASCII
fast-path for encoding
- Scope/BreadcrumbBuffer: shallow dup instead of deep_dup where inner
values are not mutated after duplication
- Hub#add_breadcrumb: hint default nil instead of {} to avoid empty
hash allocation
See sub-PRs for detailed review by risk level:
- #2902 (low risk) — hot path allocation avoidance
- #2903 (low risk) — LineCache optimization
- #2904 (medium risk) — load path and filename caching
- #2905 (needs review) — backtrace parse caching
- #2906 (needs review) — Frame object caching
- #2907 (needs review) — Scope/BreadcrumbBuffer shallow dup
- #2908 (medium risk) — RequestInterface optimizations
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| Gem::Version.new(::Rack.release) >= Gem::Version.new("3.0") | ||
| end | ||
|
|
||
| def filter_and_format_env(env, rack_env_whitelist, send_default_pii) |
There was a problem hiding this comment.
IP headers leak when rack_env_whitelist is empty
Medium Severity
When rack_env_whitelist is empty, filter_and_format_env returns the original env immediately without checking send_default_pii, so IP headers like REMOTE_ADDR, HTTP_CLIENT_IP, HTTP_X_REAL_IP, and HTTP_X_FORWARDED_FOR leak into the event sent to Sentry. Previously, env.dup followed by IP header deletion ensured the early-return path was already sanitized. Now that the dup+delete is removed, this fast path bypasses the new inline IP filtering entirely.
There was a problem hiding this comment.
this needs to be fixed by moving send_default_pii before this return
sl0thentr0py
force-pushed
the
perf/request-interface-optimizations
branch
from
2377baa to
2ace950
Compare
| Gem::Version.new(::Rack.release) >= Gem::Version.new("3.0") | ||
| end | ||
|
|
||
| def filter_and_format_env(env, rack_env_whitelist, send_default_pii) |
There was a problem hiding this comment.
this needs to be fixed by moving send_default_pii before this return
|
only the lowercase change here makes sense so I'm reverting the others since they're negligible optimizations and add needless caching complexity |
sl0thentr0py
force-pushed
the
perf/request-interface-optimizations
branch
from
2ace950 to
7b1ddce
Compare
sl0thentr0py
changed the title
2 participants
EDIT: only kept the lowercase check from here, leaving it in for reference
⚡ Medium risk — removes env.dup, adds header name cache
Part of #2901 (reduce memory allocations by ~53%)
Changes
Remove
env.dup: Instead of duplicating the entire Rack env hash and deleting IP headers, filter them inline in bothfilter_and_format_headersandfilter_and_format_envwhensend_default_piiis false. Avoids duplicating ~40+ entry hash on every request.Cache header name transformations:
HTTP_ACCEPT_LANGUAGE → Accept-Languageconversions are cached at the class level. Header names are deterministic and repeat on every request.ASCII fast-path:
ascii_only?strings are already valid UTF-8 — skip thedup+force_encodingdance for header values.hint: nildefault in Hub#add_breadcrumb: The empty{}was allocated on every breadcrumb call even though most callers don't use hints. Lazily materialized with|| {}only whenbefore_breadcrumbcallback is configured.Review focus
env.dupremoval: we no longer modify the Rack env at all (no deleting IP headers). Instead we skip them during iteration. The original env hash is unchanged. Is there any code path that relied on the IP headers being removed from the env after RequestInterface creation?