Skip to content

Commit ccf5a59

Browse files
committed
perf(kotlin): one fact read per file, edges only where resolved
The JVM builder reread and reparsed a Kotlin file for every fragment and asked the resolver for the same definition once per use; the base side of a range spawned `git show` per file and per changed definition. Facts now come from the resolver's cache, uses are found by binary search on line, a definition is resolved once per fragment, base reads go through one `git cat-file --batch`, and a project lookup memoises every directory it walks. Impact on the 35-file yay-tsa commit 31935cdd7: 3.2 s -> 1.8 s warm. Output change: a Kotlin member use now links in the context graph only to the definition it resolves to. A candidate (untyped receiver, overload, dispatch) suggests in impact; it no longer adds an edge. Claude-Session: https://claude.ai/code/session_0176zGy9cjNvwmtiRJLjDcKp
1 parent 904cb30 commit ccf5a59

3 files changed

Lines changed: 61 additions & 26 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3434
implementations of a changed abstract member are their own relation, a
3535
removed declaration is found by comparing the file's declarations at the
3636
base and the head, and a public contract changes when the header does —
37-
parameter defaults and all.
37+
parameter defaults and all. In the context graph a Kotlin member use links
38+
only to the definition it resolves to: a candidate suggests, it does not
39+
link.
3840

3941
### Fixed
4042

‎crates/diffctx-native/src/edges/semantic/jvm.rs‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -494,10 +494,6 @@ impl EdgeBuilder for JVMEdgeBuilder {
494494
return FxHashMap::default();
495495
};
496496
let source = crate::source::current();
497-
let kotlin_facts = |f: &Fragment| {
498-
crate::source::read_to_string(Path::new(f.path()))
499-
.and_then(|text| crate::facts::kotlin::facts(&text))
500-
};
501497

502498
let import_weight = EDGE_WEIGHTS["jvm_import"].forward;
503499
let inheritance_weight = EDGE_WEIGHTS["jvm_inheritance"].forward;
@@ -771,9 +767,10 @@ impl EdgeBuilder for JVMEdgeBuilder {
771767
}
772768

773769
// Kotlin users bind through their syntax facts: a member use links to
774-
// the definitions the Kotlin binding model does not reject, whatever
775-
// the length of the name, and a spelling in a string or a comment,
776-
// or a receiver of another type, links nothing.
770+
// the definitions the Kotlin binding model resolves it to, whatever
771+
// the length of the name. A candidate (an untyped receiver, an
772+
// overload) is no edge — a guess suggests, it does not link — and a
773+
// spelling in a string or a comment links nothing.
777774
let kotlin_defs: FxHashMap<&str, Vec<&Fragment>> = {
778775
let mut m: FxHashMap<&str, Vec<&Fragment>> = FxHashMap::default();
779776
for f in jvm_frags.iter().filter(|f| is_kotlin(Path::new(f.path()))) {
@@ -795,15 +792,17 @@ impl EdgeBuilder for JVMEdgeBuilder {
795792
.map(|r| r.to_string_lossy().replace('\\', "/"))
796793
.unwrap_or_else(|_| p.to_string())
797794
};
795+
let mut defs_of: FxHashMap<&FragmentId, Option<crate::kotlin_bindings::KDef>> =
796+
FxHashMap::default();
798797
for jf in jvm_frags.iter().filter(|f| is_kotlin(Path::new(f.path()))) {
799-
let Some(facts) = kotlin_facts(jf) else {
798+
let user = rel(jf.path());
799+
let Some(facts) = resolver.facts(&user) else {
800800
continue;
801801
};
802-
let user = rel(jf.path());
803-
let mut names: Vec<&str> = facts
804-
.uses
802+
let first = facts.uses.partition_point(|u| u.line < jf.start_line());
803+
let mut names: Vec<&str> = facts.uses[first..]
805804
.iter()
806-
.filter(|u| u.line >= jf.start_line() && u.line <= jf.end_line())
805+
.take_while(|u| u.line <= jf.end_line())
807806
.map(|u| u.name.as_str())
808807
.collect();
809808
names.sort_unstable();
@@ -820,13 +819,18 @@ impl EdgeBuilder for JVMEdgeBuilder {
820819
if d.id == jf.id || crate::container::within(&jf.id, d) {
821820
continue;
822821
}
823-
let Some(def) = resolver.definition(&rel(d.path()), name, d.start_line())
822+
let Some(def) = defs_of
823+
.entry(&d.id)
824+
.or_insert_with(|| {
825+
resolver.definition(&rel(d.path()), name, d.start_line())
826+
})
827+
.as_ref()
824828
else {
825829
continue;
826830
};
827831
if resolver
828-
.classify(&user, (jf.start_line(), jf.end_line()), &def)
829-
.is_some()
832+
.classify(&user, (jf.start_line(), jf.end_line()), def)
833+
.is_some_and(|f| f.evidence == crate::bindings::Evidence::Resolved)
830834
{
831835
add_edge(&mut edges, &jf.id, &d.id, member_weight, reverse_factor);
832836
}

‎crates/diffctx-native/src/kotlin_bindings.rs‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,11 @@ pub struct KotlinResolver<'a> {
7373
root: &'a Path,
7474
source: &'a Source,
7575
facts: RefCell<FxHashMap<String, Option<Arc<KotlinFacts>>>>,
76+
/// Facts of other revisions, by (revision, path): the base side of a
77+
/// range is asked for once per changed definition.
78+
facts_at: RefCell<FxHashMap<(String, String), Option<Arc<KotlinFacts>>>>,
79+
/// One `git cat-file --batch` for those reads, not a process per file.
80+
batch: RefCell<Option<crate::git::CatFileBatch>>,
7681
type_files: RefCell<FxHashMap<String, Vec<String>>>,
7782
projects: RefCell<FxHashMap<PathBuf, Option<PathBuf>>>,
7883
deps: RefCell<FxHashMap<PathBuf, Vec<PathBuf>>>,
@@ -84,6 +89,8 @@ impl<'a> KotlinResolver<'a> {
8489
root,
8590
source,
8691
facts: RefCell::default(),
92+
facts_at: RefCell::default(),
93+
batch: RefCell::default(),
8794
type_files: RefCell::default(),
8895
projects: RefCell::default(),
8996
deps: RefCell::default(),
@@ -120,8 +127,23 @@ impl<'a> KotlinResolver<'a> {
120127

121128
/// The facts of `rel` at another revision of the repository.
122129
pub fn facts_at(&self, rev: &str, rel: &str) -> Option<Arc<KotlinFacts>> {
123-
let text = crate::git::show_file_at_revision(self.root, rev, Path::new(rel)).ok()?;
124-
crate::facts::kotlin::facts(&text)
130+
let key = (rev.to_string(), rel.to_string());
131+
if let Some(known) = self.facts_at.borrow().get(&key) {
132+
return known.clone();
133+
}
134+
let read = {
135+
let mut batch = self.batch.borrow_mut();
136+
if batch.is_none() {
137+
*batch = crate::git::CatFileBatch::new(self.root).ok();
138+
}
139+
batch
140+
.as_mut()
141+
.and_then(|b| b.get(rev, Path::new(rel)).ok())
142+
.or_else(|| crate::git::show_file_at_revision(self.root, rev, Path::new(rel)).ok())
143+
}
144+
.and_then(|text| crate::facts::kotlin::facts(&text));
145+
self.facts_at.borrow_mut().insert(key, read.clone());
146+
read
125147
}
126148

127149
/// The lines of `rel` where a use could reach `def` by spelling: its
@@ -147,10 +169,11 @@ impl<'a> KotlinResolver<'a> {
147169
let facts = self.facts(rel)?;
148170
let names = self.names_for(&facts, def);
149171
let mut best: Option<Finding> = None;
150-
for u in facts
151-
.uses
172+
let first = facts.uses.partition_point(|u| u.line < lines.0);
173+
for u in facts.uses[first..]
152174
.iter()
153-
.filter(|u| u.line >= lines.0 && u.line <= lines.1 && names.contains(&u.name))
175+
.take_while(|u| u.line <= lines.1)
176+
.filter(|u| names.contains(&u.name))
154177
{
155178
// The definition's own declaration is no use of it.
156179
if rel == def.path && def.decl.span.contains(&u.at) {
@@ -802,11 +825,14 @@ impl<'a> KotlinResolver<'a> {
802825
/// above it holding a build file.
803826
fn project_of(&self, rel: &str) -> Option<PathBuf> {
804827
let mut dir = Path::new(rel).parent()?.to_path_buf();
805-
let start = dir.clone();
806-
if let Some(known) = self.projects.borrow().get(&start) {
807-
return known.clone();
808-
}
828+
// Every directory walked through shares the answer, so a sibling
829+
// file stops at the first directory already asked about.
830+
let mut walked: Vec<PathBuf> = Vec::new();
809831
let found = loop {
832+
if let Some(known) = self.projects.borrow().get(&dir) {
833+
break known.clone();
834+
}
835+
walked.push(dir.clone());
810836
let has_build = ["build.gradle.kts", "build.gradle", "pom.xml"]
811837
.iter()
812838
.any(|b| {
@@ -821,7 +847,10 @@ impl<'a> KotlinResolver<'a> {
821847
break None;
822848
}
823849
};
824-
self.projects.borrow_mut().insert(start, found.clone());
850+
let mut projects = self.projects.borrow_mut();
851+
for d in walked {
852+
projects.insert(d, found.clone());
853+
}
825854
found
826855
}
827856

0 commit comments

Comments
 (0)