From 7ba176ca134886b037b1c2900b5402895dd128dc Mon Sep 17 00:00:00 2001 From: Peter Harris Date: Wed, 4 Mar 2026 16:35:43 -0500 Subject: [PATCH] Fix caching gaps and reduce Pathname overhead in file lookups Cache nil results in TeamFinder.for_file when RustCodeOwners returns nil, preventing repeated uncached calls to the Rust layer for unowned files. Memoize Pathname.pwd and use string prefix stripping instead of Pathname#relative_path_from for the common case, eliminating costly Pathname allocations and cleanpath operations. Co-Authored-By: Claude Opus 4.6 --- lib/code_ownership.rb | 2 ++ .../private/file_path_finder.rb | 22 ++++++++++++++++--- lib/code_ownership/private/team_finder.rb | 3 +-- .../private/team_finder_spec.rb | 13 +++++++---- 4 files changed, 31 insertions(+), 9 deletions(-) diff --git a/lib/code_ownership.rb b/lib/code_ownership.rb index 38c7ae4..a78707d 100644 --- a/lib/code_ownership.rb +++ b/lib/code_ownership.rb @@ -315,5 +315,7 @@ def self.for_package(package) sig { void } def self.bust_caches! Private::FilePathTeamCache.bust_cache! + Private::FilePathFinder.instance_variable_set(:@pwd, nil) + Private::FilePathFinder.instance_variable_set(:@pwd_prefix, nil) end end diff --git a/lib/code_ownership/private/file_path_finder.rb b/lib/code_ownership/private/file_path_finder.rb index 2e79881..e14523f 100644 --- a/lib/code_ownership/private/file_path_finder.rb +++ b/lib/code_ownership/private/file_path_finder.rb @@ -6,13 +6,29 @@ module Private module FilePathFinder extend T::Sig + sig { returns(String) } + def self.pwd_prefix + @pwd_prefix ||= T.let("#{Dir.pwd}/", T.nilable(String)) + end + + sig { returns(Pathname) } + def self.pwd + @pwd ||= T.let(Pathname.pwd, T.nilable(Pathname)) + end + # Returns a string version of the relative path to a Rails constant, # or nil if it can't find anything sig { params(klass: T.nilable(T.any(T::Class[T.anything], T::Module[T.anything]))).returns(T.nilable(String)) } def self.path_from_klass(klass) if klass path = Object.const_source_location(klass.to_s)&.first - (path && Pathname.new(path).relative_path_from(Pathname.pwd).to_s) || nil + return nil unless path + + if path.start_with?(pwd_prefix) + path.delete_prefix(pwd_prefix) + else + Pathname.new(path).relative_path_from(pwd).to_s + end end rescue NameError nil @@ -30,7 +46,7 @@ def self.from_backtrace(backtrace) # ./app/controllers/some_controller.rb:43:in `block (3 levels) in create' # backtrace_line = if RUBY_VERSION >= '3.4.0' - %r{\A(#{Pathname.pwd}/|\./)? + %r{\A(#{pwd}/|\./)? (?.+) # Matches 'app/controllers/some_controller.rb' : (?\d+) # Matches '43' @@ -38,7 +54,7 @@ def self.from_backtrace(backtrace) '(?.*)' # Matches "`block (3 levels) in create'" \z}x else - %r{\A(#{Pathname.pwd}/|\./)? + %r{\A(#{pwd}/|\./)? (?.+) # Matches 'app/controllers/some_controller.rb' : (?\d+) # Matches '43' diff --git a/lib/code_ownership/private/team_finder.rb b/lib/code_ownership/private/team_finder.rb index 76eab7d..9f26e16 100644 --- a/lib/code_ownership/private/team_finder.rb +++ b/lib/code_ownership/private/team_finder.rb @@ -13,9 +13,8 @@ def self.for_file(file_path, allow_raise: false) return FilePathTeamCache.get(file_path) if FilePathTeamCache.cached?(file_path) result = T.let(RustCodeOwners.for_file(file_path), T.nilable(T::Hash[Symbol, String])) - return if result.nil? - if result[:team_name].nil? + if result.nil? || result[:team_name].nil? FilePathTeamCache.set(file_path, nil) else FilePathTeamCache.set(file_path, T.let(find_team!(T.must(result[:team_name]), allow_raise: allow_raise), T.nilable(CodeTeams::Team))) diff --git a/spec/lib/code_ownership/private/team_finder_spec.rb b/spec/lib/code_ownership/private/team_finder_spec.rb index 6a3639d..d4dac78 100644 --- a/spec/lib/code_ownership/private/team_finder_spec.rb +++ b/spec/lib/code_ownership/private/team_finder_spec.rb @@ -20,11 +20,16 @@ expect(CodeOwnership::Private::FilePathTeamCache.cached?(file_path)).to be true end - it 'does not cache when rust returns nil' do - allow(RustCodeOwners).to receive(:for_file).with(file_path).and_return(nil) + it 'caches nil when rust returns nil' do + allow(RustCodeOwners).to receive(:for_file).with(file_path) + .and_return(nil, { team_name: 'Bar' }) - expect(described_class.for_file(file_path)).to be_nil - expect(CodeOwnership::Private::FilePathTeamCache.cached?(file_path)).to be false + first = described_class.for_file(file_path) + second = described_class.for_file(file_path) + + expect(first).to be_nil + expect(second).to be_nil + expect(CodeOwnership::Private::FilePathTeamCache.cached?(file_path)).to be true end it 'caches nil when team_name is nil' do