Skip to content

Commit d030e01

Browse files
pharris2411claude
andauthored
Fix caching gaps and reduce Pathname overhead in file lookups (#161)
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 <noreply@anthropic.com>
1 parent 863ebc7 commit d030e01

4 files changed

Lines changed: 31 additions & 9 deletions

File tree

lib/code_ownership.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,5 +315,7 @@ def self.for_package(package)
315315
sig { void }
316316
def self.bust_caches!
317317
Private::FilePathTeamCache.bust_cache!
318+
Private::FilePathFinder.instance_variable_set(:@pwd, nil)
319+
Private::FilePathFinder.instance_variable_set(:@pwd_prefix, nil)
318320
end
319321
end

lib/code_ownership/private/file_path_finder.rb

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,29 @@ module Private
66
module FilePathFinder
77
extend T::Sig
88

9+
sig { returns(String) }
10+
def self.pwd_prefix
11+
@pwd_prefix ||= T.let("#{Dir.pwd}/", T.nilable(String))
12+
end
13+
14+
sig { returns(Pathname) }
15+
def self.pwd
16+
@pwd ||= T.let(Pathname.pwd, T.nilable(Pathname))
17+
end
18+
919
# Returns a string version of the relative path to a Rails constant,
1020
# or nil if it can't find anything
1121
sig { params(klass: T.nilable(T.any(T::Class[T.anything], T::Module[T.anything]))).returns(T.nilable(String)) }
1222
def self.path_from_klass(klass)
1323
if klass
1424
path = Object.const_source_location(klass.to_s)&.first
15-
(path && Pathname.new(path).relative_path_from(Pathname.pwd).to_s) || nil
25+
return nil unless path
26+
27+
if path.start_with?(pwd_prefix)
28+
path.delete_prefix(pwd_prefix)
29+
else
30+
Pathname.new(path).relative_path_from(pwd).to_s
31+
end
1632
end
1733
rescue NameError
1834
nil
@@ -30,15 +46,15 @@ def self.from_backtrace(backtrace)
3046
# ./app/controllers/some_controller.rb:43:in `block (3 levels) in create'
3147
#
3248
backtrace_line = if RUBY_VERSION >= '3.4.0'
33-
%r{\A(#{Pathname.pwd}/|\./)?
49+
%r{\A(#{pwd}/|\./)?
3450
(?<file>.+) # Matches 'app/controllers/some_controller.rb'
3551
:
3652
(?<line>\d+) # Matches '43'
3753
:in\s
3854
'(?<function>.*)' # Matches "`block (3 levels) in create'"
3955
\z}x
4056
else
41-
%r{\A(#{Pathname.pwd}/|\./)?
57+
%r{\A(#{pwd}/|\./)?
4258
(?<file>.+) # Matches 'app/controllers/some_controller.rb'
4359
:
4460
(?<line>\d+) # Matches '43'

lib/code_ownership/private/team_finder.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,8 @@ def self.for_file(file_path, allow_raise: false)
1313
return FilePathTeamCache.get(file_path) if FilePathTeamCache.cached?(file_path)
1414

1515
result = T.let(RustCodeOwners.for_file(file_path), T.nilable(T::Hash[Symbol, String]))
16-
return if result.nil?
1716

18-
if result[:team_name].nil?
17+
if result.nil? || result[:team_name].nil?
1918
FilePathTeamCache.set(file_path, nil)
2019
else
2120
FilePathTeamCache.set(file_path, T.let(find_team!(T.must(result[:team_name]), allow_raise: allow_raise), T.nilable(CodeTeams::Team)))

spec/lib/code_ownership/private/team_finder_spec.rb

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,16 @@
2020
expect(CodeOwnership::Private::FilePathTeamCache.cached?(file_path)).to be true
2121
end
2222

23-
it 'does not cache when rust returns nil' do
24-
allow(RustCodeOwners).to receive(:for_file).with(file_path).and_return(nil)
23+
it 'caches nil when rust returns nil' do
24+
allow(RustCodeOwners).to receive(:for_file).with(file_path)
25+
.and_return(nil, { team_name: 'Bar' })
2526

26-
expect(described_class.for_file(file_path)).to be_nil
27-
expect(CodeOwnership::Private::FilePathTeamCache.cached?(file_path)).to be false
27+
first = described_class.for_file(file_path)
28+
second = described_class.for_file(file_path)
29+
30+
expect(first).to be_nil
31+
expect(second).to be_nil
32+
expect(CodeOwnership::Private::FilePathTeamCache.cached?(file_path)).to be true
2833
end
2934

3035
it 'caches nil when team_name is nil' do

0 commit comments

Comments
 (0)