Skip to content

Commit 9f53fd0

Browse files
committed
Actually save an equality token so we do not churn the DocsCache
As you can see, valid_at was written only once, at startup. So DocsCache was a cache in name only. As soon as you touched prelude, the DocsCache would never be is_reusable again.
1 parent a5d7448 commit 9f53fd0

File tree

1 file changed

+21
-19
lines changed

1 file changed

+21
-19
lines changed

app/buck2_server/src/lsp.rs

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -100,17 +100,13 @@ enum ResolveLoadError {
100100
struct DocsCacheManager {
101101
docs_cache: Mutex<DocsCache>,
102102
fs: ProjectRoot,
103-
/// Used for checking if the DocsCache need refreshing. We need to refresh the DocsCache
104-
/// if the previous dice version does not match the current one.
105-
valid_at: DiceEquality,
106103
}
107104

108105
impl DocsCacheManager {
109106
async fn new(fs: ProjectRoot, mut dice_ctx: DiceTransaction) -> buck2_error::Result<Self> {
110107
Ok(Self {
111108
docs_cache: Mutex::new(Self::new_docs_cache(&fs, &mut dice_ctx).await?),
112109
fs,
113-
valid_at: dice_ctx.equality_token(),
114110
})
115111
}
116112

@@ -121,21 +117,14 @@ impl DocsCacheManager {
121117
let mut docs_cache = self.docs_cache.lock().await;
122118

123119
let fs = &self.fs;
124-
match self.is_reusable(&current_dice_ctx) {
125-
true => (),
126-
false => {
127-
let new_docs_cache = Self::new_docs_cache(fs, &mut current_dice_ctx).await?;
128-
*docs_cache = new_docs_cache
129-
}
130-
};
120+
if !docs_cache.is_reusable(&current_dice_ctx) {
121+
let new_docs_cache = Self::new_docs_cache(fs, &mut current_dice_ctx).await?;
122+
*docs_cache = new_docs_cache;
123+
}
131124

132125
Ok(docs_cache)
133126
}
134127

135-
fn is_reusable(&self, dice_ctx: &DiceTransaction) -> bool {
136-
dice_ctx.equivalent(&self.valid_at)
137-
}
138-
139128
async fn new_docs_cache(
140129
fs: &ProjectRoot,
141130
dice_ctx: &mut DiceTransaction,
@@ -152,7 +141,8 @@ impl DocsCacheManager {
152141
if let Some((import_path, docs)) = get_prelude_docs(dice_ctx, &builtin_names).await? {
153142
builtin_docs.push((Some(import_path), docs));
154143
}
155-
DocsCache::new(&builtin_docs, fs, &cell_resolver).await
144+
let equality_token = dice_ctx.equality_token();
145+
DocsCache::new(equality_token, &builtin_docs, fs, &cell_resolver).await
156146
}
157147
}
158148

@@ -205,6 +195,9 @@ struct DocsCache {
205195
builtin_docs: DocModule,
206196
/// A DocModule for build files specifically, with prelude symbols.
207197
buildfile_docs: DocModule,
198+
/// Used for checking if the DocsCache need refreshing. We need to refresh the DocsCache
199+
/// if the previous dice version does not match the current one.
200+
valid_at: Option<DiceEquality>,
208201
}
209202

210203
#[derive(buck2_error::Error, Debug)]
@@ -232,12 +225,19 @@ impl DocsCache {
232225
.map_err(|e| from_any_with_tag(e, buck2_error::ErrorTag::Lsp))
233226
}
234227

228+
fn is_reusable(&self, dice_ctx: &DiceTransaction) -> bool {
229+
self.valid_at
230+
.as_ref()
231+
.is_some_and(|valid_at| dice_ctx.equivalent(valid_at))
232+
}
233+
235234
async fn new(
235+
equality_token: DiceEquality,
236236
builtin_symbols: &[(Option<ImportPath>, DocModule)],
237237
fs: &ProjectRoot,
238238
cell_resolver: &CellResolver,
239239
) -> buck2_error::Result<Self> {
240-
Self::new_with_lookup(builtin_symbols, |location| async {
240+
Self::new_with_lookup(Some(equality_token), builtin_symbols, |location| async {
241241
Self::get_prelude_uri(location, fs, cell_resolver).await
242242
})
243243
.await
@@ -248,6 +248,7 @@ impl DocsCache {
248248
F: Fn(&'a ImportPath) -> Fut + 'a,
249249
Fut: Future<Output = buck2_error::Result<LspUrl>>,
250250
>(
251+
equality_token: Option<DiceEquality>,
251252
builtin_symbols: &'a [(Option<ImportPath>, DocModule)],
252253
location_lookup: F,
253254
) -> buck2_error::Result<Self> {
@@ -328,6 +329,7 @@ impl DocsCache {
328329
native_starlark_files,
329330
builtin_docs,
330331
buildfile_docs,
332+
valid_at: equality_token,
331333
})
332334
}
333335

@@ -1050,8 +1052,8 @@ mod tests {
10501052
}
10511053
}
10521054

1053-
let cache =
1054-
runtime.block_on(async { DocsCache::new_with_lookup(&docs, lookup_function).await })?;
1055+
let cache = runtime
1056+
.block_on(async { DocsCache::new_with_lookup(None, &docs, lookup_function).await })?;
10551057

10561058
assert_eq!(
10571059
&LspUrl::try_from(

0 commit comments

Comments
 (0)