Skip to content

Commit 949bc6b

Browse files
authored
perf: avoid unnecessary allocations in hot paths (#2902)
Reduce memory allocations during exception capture with zero-risk changes that preserve identical behavior: Backtrace::Line.parse: - Use indexed regex captures (match[1]) instead of .to_a destructuring, avoiding intermediate array allocation - Add end_with? guard before .sub! for .class extension — skips string allocation on non-JRuby (99.9% of cases) - Use match? instead of =~ in #in_app to avoid MatchData allocation - Add nil guard for file in #in_app StacktraceInterface::Frame: - Remove stored @project_root/@strip_backtrace_load_path ivars (passed as args to compute_filename instead) — fewer entries in Interface#to_h - Use byteslice instead of [] for filename prefix stripping - Replace chomp(File::SEPARATOR) with end_with? check to avoid allocation - Inline under_project_root? to eliminate method call overhead StacktraceBuilder#build: - Replace select + reverse + map + compact chain with single reverse while loop, eliminating 3 intermediate array allocations RequestInterface: - Use delete_prefix instead of regex sub for HTTP_ prefix removal - Replace key.upcase != key with key.match?(LOWERCASE_PATTERN) to avoid allocating an upcased string for every Rack env entry - Cache Rack version check to avoid repeated Gem::Version comparisons
1 parent 430494e commit 949bc6b

File tree

5 files changed

+68
-39
lines changed

5 files changed

+68
-39
lines changed

sentry-ruby/lib/sentry/backtrace.rb

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,16 @@ class Backtrace
1010
# holder for an Array of Backtrace::Line instances
1111
attr_reader :lines
1212

13-
def self.parse(backtrace, project_root, app_dirs_pattern, &backtrace_cleanup_callback)
13+
# @deprecated project_root, in_app_pattern passed from outside
14+
# @deprecated app_dirs_pattern, in_app_pattern passed from outside
15+
def self.parse(backtrace, project_root, app_dirs_pattern, in_app_pattern: nil, &backtrace_cleanup_callback)
1416
ruby_lines = backtrace.is_a?(Array) ? backtrace : backtrace.split(/\n\s*/)
1517

1618
ruby_lines = backtrace_cleanup_callback.call(ruby_lines) if backtrace_cleanup_callback
1719

18-
in_app_pattern ||= begin
19-
Regexp.new("^(#{project_root}/)?#{app_dirs_pattern}")
20-
end
20+
# in_app_pattern is now passed in from StacktraceBuilder, so this regex won't be triggered
21+
# only here for backwards compat and will be deleted
22+
in_app_pattern ||= Regexp.new("^(#{project_root}/)?#{app_dirs_pattern}")
2123

2224
lines = ruby_lines.to_a.map do |unparsed_line|
2325
Line.parse(unparsed_line, in_app_pattern)

sentry-ruby/lib/sentry/backtrace/line.rb

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ class Backtrace
66
# Handles backtrace parsing line by line
77
class Line
88
RB_EXTENSION = ".rb"
9+
CLASS_EXTENSION = ".class"
910
# regexp (optional leading X: on windows, or JRuby9000 class-prefix)
1011
RUBY_INPUT_FORMAT = /
1112
^ \s* (?: [a-zA-Z]: | uri:classloader: )? ([^:]+ | <.*>):
@@ -37,12 +38,21 @@ def self.parse(unparsed_line, in_app_pattern = nil)
3738
ruby_match = unparsed_line.match(RUBY_INPUT_FORMAT)
3839

3940
if ruby_match
40-
_, file, number, _, module_name, method = ruby_match.to_a
41-
file.sub!(/\.class$/, RB_EXTENSION)
42-
module_name = module_name
41+
file = ruby_match[1]
42+
number = ruby_match[2]
43+
module_name = ruby_match[4]
44+
method = ruby_match[5]
45+
if file.end_with?(CLASS_EXTENSION)
46+
file.sub!(/\.class$/, RB_EXTENSION)
47+
end
4348
else
4449
java_match = unparsed_line.match(JAVA_INPUT_FORMAT)
45-
_, module_name, method, file, number = java_match.to_a
50+
if java_match
51+
module_name = java_match[1]
52+
method = java_match[2]
53+
file = java_match[3]
54+
number = java_match[4]
55+
end
4656
end
4757
new(file, number, method, module_name, in_app_pattern)
4858
end
@@ -74,12 +84,9 @@ def initialize(file, number, method, module_name, in_app_pattern)
7484

7585
def in_app
7686
return false unless in_app_pattern
87+
return false unless file
7788

78-
if file =~ in_app_pattern
79-
true
80-
else
81-
false
82-
end
89+
file.match?(in_app_pattern)
8390
end
8491

8592
# Reconstructs the line in a readable fashion

sentry-ruby/lib/sentry/interfaces/request.rb

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ class RequestInterface < Interface
1111
"HTTP_X_FORWARDED_FOR"
1212
].freeze
1313

14+
# Regex to detect lowercase chars — match? is allocation-free (no MatchData/String)
15+
LOWERCASE_PATTERN = /[a-z]/.freeze
16+
1417
# See Sentry server default limits at
1518
# https://github.com/getsentry/sentry/blob/master/src/sentry/conf/server.py
1619
MAX_BODY_LIMIT = 4096 * 4
@@ -93,7 +96,7 @@ def filter_and_format_headers(env, send_default_pii)
9396
next if key == "HTTP_AUTHORIZATION" && !send_default_pii
9497

9598
# Rack stores headers as HTTP_WHAT_EVER, we need What-Ever
96-
key = key.sub(/^HTTP_/, "")
99+
key = key.delete_prefix("HTTP_")
97100
key = key.split("_").map(&:capitalize).join("-")
98101

99102
memo[key] = Utils::EncodingHelper.encode_to_utf_8(value.to_s)
@@ -107,9 +110,6 @@ def filter_and_format_headers(env, send_default_pii)
107110
end
108111
end
109112

110-
# Regex to detect lowercase chars — match? is allocation-free (no MatchData/String)
111-
LOWERCASE_PATTERN = /[a-z]/.freeze
112-
113113
def is_skippable_header?(key)
114114
key.match?(LOWERCASE_PATTERN) || # lower-case envs aren't real http headers
115115
key == "HTTP_COOKIE" || # Cookies don't go here, they go somewhere else
@@ -122,12 +122,18 @@ def is_skippable_header?(key)
122122
# if the request has legitimately sent a Version header themselves.
123123
# See: https://github.com/rack/rack/blob/028438f/lib/rack/handler/cgi.rb#L29
124124
def is_server_protocol?(key, value, protocol_version)
125-
rack_version = Gem::Version.new(::Rack.release)
126-
return false if rack_version >= Gem::Version.new("3.0")
125+
return false if self.class.rack_3_or_above?
127126

128127
key == "HTTP_VERSION" && value == protocol_version
129128
end
130129

130+
def self.rack_3_or_above?
131+
return @rack_3_or_above if defined?(@rack_3_or_above)
132+
133+
@rack_3_or_above = defined?(::Rack) &&
134+
Gem::Version.new(::Rack.release) >= Gem::Version.new("3.0")
135+
end
136+
131137
def filter_and_format_env(env, rack_env_whitelist)
132138
return env if rack_env_whitelist.empty?
133139

sentry-ruby/lib/sentry/interfaces/stacktrace.rb

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,35 +28,43 @@ class Frame < Interface
2828
:lineno, :module, :pre_context, :post_context, :vars
2929

3030
def initialize(project_root, line, strip_backtrace_load_path = true)
31-
@project_root = project_root
32-
@strip_backtrace_load_path = strip_backtrace_load_path
33-
3431
@abs_path = line.file
3532
@function = line.method if line.method
3633
@lineno = line.number
3734
@in_app = line.in_app
3835
@module = line.module_name if line.module_name
39-
@filename = compute_filename
36+
@filename = compute_filename(project_root, strip_backtrace_load_path)
4037
end
4138

4239
def to_s
4340
"#{@filename}:#{@lineno}"
4441
end
4542

46-
def compute_filename
43+
def compute_filename(project_root, strip_backtrace_load_path)
4744
return if abs_path.nil?
48-
return abs_path unless @strip_backtrace_load_path
45+
return abs_path unless strip_backtrace_load_path
4946

47+
under_root = project_root && abs_path.start_with?(project_root)
5048
prefix =
51-
if under_project_root? && in_app
52-
@project_root
53-
elsif under_project_root?
54-
longest_load_path || @project_root
49+
if under_root && in_app
50+
project_root
51+
elsif under_root
52+
longest_load_path || project_root
5553
else
5654
longest_load_path
5755
end
5856

59-
prefix ? abs_path[prefix.to_s.chomp(File::SEPARATOR).length + 1..-1] : abs_path
57+
if prefix
58+
prefix_str = prefix.to_s
59+
offset = if prefix_str.end_with?(File::SEPARATOR)
60+
prefix_str.bytesize
61+
else
62+
prefix_str.bytesize + 1
63+
end
64+
abs_path.byteslice(offset, abs_path.bytesize - offset)
65+
else
66+
abs_path
67+
end
6068
end
6169

6270
def set_context(linecache, context_lines)
@@ -77,10 +85,6 @@ def to_h(*args)
7785

7886
private
7987

80-
def under_project_root?
81-
@project_root && abs_path.start_with?(@project_root)
82-
end
83-
8488
def longest_load_path
8589
$LOAD_PATH.select { |path| abs_path.start_with?(path.to_s) }.max_by(&:size)
8690
end

sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ def initialize(
4646
@context_lines = context_lines
4747
@backtrace_cleanup_callback = backtrace_cleanup_callback
4848
@strip_backtrace_load_path = strip_backtrace_load_path
49+
@in_app_pattern = Regexp.new("^(#{project_root}/)?#{app_dirs_pattern}") if app_dirs_pattern
4950
end
5051

5152
# Generates a StacktraceInterface with the given backtrace.
@@ -64,13 +65,21 @@ def initialize(
6465
# @yieldparam frame [StacktraceInterface::Frame]
6566
# @return [StacktraceInterface]
6667
def build(backtrace:, &frame_callback)
67-
parsed_lines = parse_backtrace_lines(backtrace).select(&:file)
68+
parsed_lines = parse_backtrace_lines(backtrace)
69+
70+
# Build frames in reverse order, skipping lines without files
71+
# Single pass instead of select + reverse + map + compact
72+
frames = []
73+
i = parsed_lines.size - 1
74+
while i >= 0
75+
line = parsed_lines[i]
76+
i -= 1
77+
next unless line.file
6878

69-
frames = parsed_lines.reverse.map do |line|
7079
frame = convert_parsed_line_into_frame(line)
7180
frame = frame_callback.call(frame) if frame_callback
72-
frame
73-
end.compact
81+
frames << frame if frame
82+
end
7483

7584
StacktraceInterface.new(frames: frames)
7685
end
@@ -85,7 +94,8 @@ def convert_parsed_line_into_frame(line)
8594

8695
def parse_backtrace_lines(backtrace)
8796
Backtrace.parse(
88-
backtrace, project_root, app_dirs_pattern, &backtrace_cleanup_callback
97+
backtrace, project_root, app_dirs_pattern,
98+
in_app_pattern: @in_app_pattern, &backtrace_cleanup_callback
8999
).lines
90100
end
91101
end

0 commit comments

Comments
 (0)