From b86ccae23f2753e48ca9946e745a666d698ec85e Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Wed, 18 Mar 2026 15:53:36 +0100 Subject: [PATCH] perf: Add FilenameCache to cache compute_filename results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changed below autoresearch implementation to just have one universal FilenameCache --- Add class-level caches to StacktraceInterface for two expensive per-frame operations that repeat with identical inputs: longest_load_path: Previously iterated $LOAD_PATH for every frame, creating many intermediate strings. Now cached by abs_path with automatic invalidation when $LOAD_PATH.size changes (e.g. after Bundler.require). compute_filename: Many frames share identical abs_paths (same gem files appear in every exception). Results are cached in separate in_app/ not_in_app hashes keyed by abs_path only, avoiding composite array keys. Cache invalidates on project_root or $LOAD_PATH changes. Both caches are deterministic — same inputs always produce the same filename. The caches grow proportionally to the number of unique source files seen, which is naturally bounded in any application. --- .../lib/sentry/interfaces/stacktrace.rb | 38 +++------------- .../sentry/interfaces/stacktrace_builder.rb | 5 ++- .../lib/sentry/utils/filename_cache.rb | 45 +++++++++++++++++++ .../spec/sentry/interfaces/stacktrace_spec.rb | 9 ++-- sentry-ruby/spec/sentry/transport_spec.rb | 4 +- 5 files changed, 64 insertions(+), 37 deletions(-) create mode 100644 sentry-ruby/lib/sentry/utils/filename_cache.rb diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace.rb index 187ee177f..dae34a3ab 100644 --- a/sentry-ruby/lib/sentry/interfaces/stacktrace.rb +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace.rb @@ -27,44 +27,24 @@ class Frame < Interface attr_accessor :abs_path, :context_line, :function, :in_app, :filename, :lineno, :module, :pre_context, :post_context, :vars - def initialize(project_root, line, strip_backtrace_load_path = true) + def initialize(project_root, line, strip_backtrace_load_path = true, filename_cache: nil) + @strip_backtrace_load_path = strip_backtrace_load_path + @filename_cache = filename_cache + @abs_path = line.file @function = line.method if line.method @lineno = line.number @in_app = line.in_app @module = line.module_name if line.module_name - @filename = compute_filename(project_root, strip_backtrace_load_path) + @filename = compute_filename end def to_s "#{@filename}:#{@lineno}" end - def compute_filename(project_root, strip_backtrace_load_path) - return if abs_path.nil? - return abs_path unless strip_backtrace_load_path - - under_root = project_root && abs_path.start_with?(project_root) - prefix = - if under_root && in_app - project_root - elsif under_root - longest_load_path || project_root - else - longest_load_path - end - - if prefix - prefix_str = prefix.to_s - offset = if prefix_str.end_with?(File::SEPARATOR) - prefix_str.bytesize - else - prefix_str.bytesize + 1 - end - abs_path.byteslice(offset, abs_path.bytesize - offset) - else - abs_path - end + def compute_filename + @filename_cache&.compute_filename(abs_path, in_app, @strip_backtrace_load_path) end def set_context(linecache, context_lines) @@ -84,10 +64,6 @@ def to_h(*args) end private - - def longest_load_path - $LOAD_PATH.select { |path| abs_path.start_with?(path.to_s) }.max_by(&:size) - end end end end diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb index 76d74f9da..14bea700b 100644 --- a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "sentry/utils/filename_cache" + module Sentry class StacktraceBuilder # @return [String] @@ -47,6 +49,7 @@ def initialize( @backtrace_cleanup_callback = backtrace_cleanup_callback @strip_backtrace_load_path = strip_backtrace_load_path @in_app_pattern = Regexp.new("^(#{project_root}/)?#{app_dirs_pattern}") if app_dirs_pattern + @filename_cache = FilenameCache.new(project_root) end # Generates a StacktraceInterface with the given backtrace. @@ -87,7 +90,7 @@ def build(backtrace:, &frame_callback) private def convert_parsed_line_into_frame(line) - frame = StacktraceInterface::Frame.new(project_root, line, strip_backtrace_load_path) + frame = StacktraceInterface::Frame.new(project_root, line, strip_backtrace_load_path, filename_cache: @filename_cache) frame.set_context(linecache, context_lines) if context_lines frame end diff --git a/sentry-ruby/lib/sentry/utils/filename_cache.rb b/sentry-ruby/lib/sentry/utils/filename_cache.rb new file mode 100644 index 000000000..0472eaf74 --- /dev/null +++ b/sentry-ruby/lib/sentry/utils/filename_cache.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module Sentry + class FilenameCache + def initialize(project_root) + @project_root = project_root + @load_paths = $LOAD_PATH.map(&:to_s).sort_by(&:size).reverse.freeze + @cache = {} + end + + def compute_filename(abs_path, in_app, strip_backtrace_load_path) + return unless abs_path + return abs_path unless strip_backtrace_load_path + + @cache.fetch(abs_path) do + under_root = @project_root && abs_path.start_with?(@project_root) + prefix = + if under_root && in_app + @project_root + elsif under_root + longest_load_path(abs_path) || @project_root + else + longest_load_path(abs_path) + end + + @cache[abs_path] = if prefix + offset = if prefix.end_with?(File::SEPARATOR) + prefix.bytesize + else + prefix.bytesize + 1 + end + abs_path.byteslice(offset, abs_path.bytesize - offset) + else + abs_path + end + end + end + + private + + def longest_load_path(abs_path) + @load_paths.find { |path| abs_path.start_with?(path) } + end + end +end diff --git a/sentry-ruby/spec/sentry/interfaces/stacktrace_spec.rb b/sentry-ruby/spec/sentry/interfaces/stacktrace_spec.rb index f28c3fd4f..b12935264 100644 --- a/sentry-ruby/spec/sentry/interfaces/stacktrace_spec.rb +++ b/sentry-ruby/spec/sentry/interfaces/stacktrace_spec.rb @@ -12,16 +12,17 @@ let(:lines) do Sentry::Backtrace.parse(raw_lines, configuration.project_root, configuration.app_dirs_pattern).lines end + let(:filename_cache) { Sentry::FilenameCache.new(configuration.project_root) } it "initializes a Frame with the correct info from the given Backtrace::Line object" do - first_frame = Sentry::StacktraceInterface::Frame.new(configuration.project_root, lines.first) + first_frame = Sentry::StacktraceInterface::Frame.new(configuration.project_root, lines.first, true, filename_cache: filename_cache) expect(first_frame.filename).to match(/base.rb/) expect(first_frame.in_app).to eq(false) expect(first_frame.function).to eq("save") expect(first_frame.lineno).to eq(10) - second_frame = Sentry::StacktraceInterface::Frame.new(configuration.project_root, lines.last) + second_frame = Sentry::StacktraceInterface::Frame.new(configuration.project_root, lines.last, true, filename_cache: filename_cache) expect(second_frame.filename).to match(/post.rb/) expect(second_frame.in_app).to eq(true) @@ -30,11 +31,11 @@ end it "does not strip load path when strip_backtrace_load_path is false" do - first_frame = Sentry::StacktraceInterface::Frame.new(configuration.project_root, lines.first, false) + first_frame = Sentry::StacktraceInterface::Frame.new(configuration.project_root, lines.first, false, filename_cache: filename_cache) expect(first_frame.filename).to eq(first_frame.abs_path) expect(first_frame.filename).to eq(raw_lines.first.split(':').first) - second_frame = Sentry::StacktraceInterface::Frame.new(configuration.project_root, lines.last, false) + second_frame = Sentry::StacktraceInterface::Frame.new(configuration.project_root, lines.last, false, filename_cache: filename_cache) expect(second_frame.filename).to eq(second_frame.abs_path) expect(second_frame.filename).to eq(raw_lines.last.split(':').first) end diff --git a/sentry-ruby/spec/sentry/transport_spec.rb b/sentry-ruby/spec/sentry/transport_spec.rb index 56a941bcf..e2c67954e 100644 --- a/sentry-ruby/spec/sentry/transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport_spec.rb @@ -342,7 +342,9 @@ frames: frame_list_size.times.map do |zero_based_index| Sentry::StacktraceInterface::Frame.new( "/fake/path", - Sentry::Backtrace::Line.parse("app.rb:#{zero_based_index + 1}:in `/'", in_app_pattern) + Sentry::Backtrace::Line.parse("app.rb:#{zero_based_index + 1}:in `/'", in_app_pattern), + true, + filename_cache: Sentry::FilenameCache.new("/fake/path") ) end, )