Skip to content

perf: Reduce memory allocations by ~53% during exception capture#2901

Closed
HazAT wants to merge 1 commit intomasterfrom
perf/reduce-memory-allocations
Closed

perf: Reduce memory allocations by ~53% during exception capture#2901
HazAT wants to merge 1 commit intomasterfrom
perf/reduce-memory-allocations

Conversation

@HazAT
Copy link
Member

@HazAT HazAT commented Mar 18, 2026

Summary

SCR-20260318-ohzp-2

This PR reduces memory allocations during Sentry exception capture in Rails applications by ~53% (442k → 206k bytes, 3305 → 1538 objects).

Benchmark: Rails exception capture (MemoryProfiler, 5-iteration average)

Metric Before After Improvement
Total allocated memory 442,558 bytes 205,918 bytes -53.5%
Total allocated objects 3,305 1,538 -53.5%
Retained memory 100,613 bytes 49,309 bytes -51.0%
Retained objects 924 432 -53.2%

Approach

All changes are internal optimizations — zero behavior changes. Same inputs produce identical outputs. All existing tests pass.

This PR is composed of 7 sub-PRs for easier review, grouped by risk level:

✅ Low risk (safe to merge with quick review)

⚡ Medium risk (caching with invalidation — review the invalidation logic)

⚠️ Needs closer look (new caching layers — review bounds and safety)

Each sub-PR can be reviewed and merged independently into master. This base branch merges all of them together for integration testing.

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
@HazAT HazAT force-pushed the perf/reduce-memory-allocations branch from 57bb1d1 to b81088e Compare March 18, 2026 15:12
Comment on lines +132 to 133
copy.user = user.dup
copy.transaction_name = transaction_name.dup
Copy link

Choose a reason for hiding this comment

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

Bug: The Scope#dup method calls .dup on @transaction_name and @transaction_source, which are nil by default, causing a TypeError during event capture in Ruby 2.7+.
Severity: CRITICAL

Suggested Fix

In Scope#dup, add a nil check before calling .dup on transaction_name and transaction_source. For example: copy.transaction_name = transaction_name.dup if transaction_name.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: sentry-ruby/lib/sentry/scope.rb#L132-L133

Potential issue: The `Scope#dup` method calls `.dup` on the `@transaction_name` and
`@transaction_source` attributes. These attributes are `nil` by default and are only
assigned a value if a transaction is explicitly configured. In Ruby versions 2.7 and
newer, calling `nil.dup` raises a `TypeError`. Because `Scope#dup` is invoked during
event capture, this will cause a crash for any captured event where a transaction name
has not been set, which is a common use case. The existing unit tests do not cover this
scenario, as they set a transaction name before testing the duplication logic.

Did we get this right? 👍 / 👎 to inform future reviews.

@HazAT
Copy link
Member Author

HazAT commented Mar 18, 2026

We can close this one, since we have it in the sub PRs

@HazAT HazAT closed this Mar 18, 2026
Comment on lines +90 to +92
cache_key = line.object_id
cached_frame = @frame_cache&.[](cache_key)
return cached_frame if cached_frame
Copy link

Choose a reason for hiding this comment

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

Bug: The @frame_cache uses line.object_id as a key, which can be reused by Ruby's garbage collector, leading to stale cache hits and incorrect stack frame data.
Severity: MEDIUM

Suggested Fix

The cache key for @frame_cache should be changed from line.object_id to a value that is guaranteed to be unique and stable, such as the Line object itself or a composite key derived from its attributes. This will prevent collisions caused by object_id reuse after garbage collection.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb#L90-L92

Potential issue: The `StacktraceBuilder` uses a `@frame_cache` keyed by
`line.object_id`. In Ruby, `object_id`s can be reused after an object is garbage
collected. A separate cache for `Line` objects, `@line_object_cache`, is cleared when it
reaches its size limit, allowing `Line` objects to be garbage collected. If a new `Line`
object is created and allocated the same `object_id` as a previously garbage-collected
one, the `@frame_cache` will incorrectly return the stale frame associated with the old
object. This results in incorrect stack trace data being reported.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 4 potential issues.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

else
prefix_str.length + 1
end
abs_path.byteslice(offset, abs_path.bytesize - offset)
Copy link

Choose a reason for hiding this comment

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

byteslice uses character length instead of byte length

Medium Severity

prefix_str.length returns the character count, but byteslice expects a byte offset. For file paths containing multi-byte UTF-8 characters, the character count is less than the byte count, causing byteslice to cut into the middle of a multi-byte character and produce a corrupt filename. The old code used character-based String#[] indexing which handled this correctly.

Additional Locations (1)
Fix in Cursor Fix in Web

Regexp.new("^(#{project_root}/)?#{app_dirs_pattern}")
cache_key = app_dirs_pattern
in_app_pattern = @in_app_pattern_cache.fetch(cache_key) do
@in_app_pattern_cache[cache_key] = Regexp.new("^(#{project_root}/)?#{app_dirs_pattern}")
Copy link

Choose a reason for hiding this comment

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

in_app_pattern cache ignores project_root in key

Medium Severity

The @in_app_pattern_cache uses only app_dirs_pattern as the cache key, but the constructed Regexp embeds project_root. If project_root changes while app_dirs_pattern stays the same (e.g., re-initialization or multiple clients), the cache returns a stale pattern containing the old project_root, causing incorrect in-app classification.

Fix in Cursor Fix in Web

frame.set_context(linecache, context_lines) if context_lines

@frame_cache ||= {}
@frame_cache[cache_key] = frame if @frame_cache.size < 2048
Copy link

Choose a reason for hiding this comment

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

Frame cache keyed by object_id is unsafe after GC

Medium Severity

Using line.object_id as a cache key in @frame_cache is unreliable. After Line.@line_object_cache clears and old Line objects are garbage collected, Ruby may reuse their object_id values for new, unrelated Line objects. Since StacktraceBuilder is long-lived (memoized on configuration), @frame_cache can return a stale Frame with the wrong filename, line number, and source context for a completely different backtrace line.

Fix in Cursor Fix in Web


frame.pre_context = pre
frame.context_line = ctx_line
frame.post_context = post
Copy link

Choose a reason for hiding this comment

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

Unused method set_frame_context added but never called

Low Severity

The set_frame_context method and its associated @context_cache instance variable are introduced in this PR but never called anywhere in the codebase. This is dead code that adds complexity and a caching layer (with its own memory overhead) that provides no benefit.

Fix in Cursor Fix in Web

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.

1 participant