Skip to content

Commit 197856d

Browse files
committed
Auto merge of #17203 - kilpkonn:collapse_terms, r=Veykril
Fix OOM caused by term search The issue came from multi Cartesian product for exprs with many (25+) arguments, each having multiple options. The solution is two fold: ### Avoid blowing up in Cartesian product **Before the logic was:** 1. Find expressions for each argument/param - there may be many 2. Take the Cartesian product (which blows up in some cases) 4. If there are more than 2 options throw them away by squashing them to `Many` **Now the logic is:** 1. Find expressions for each argument/param and squash them to `Many` if there are more than 2 as otherwise we are guaranteed to also have more than 2 after taking the product which means squashing them anyway. 2. Take the Cartesian product on iterator 3. Start consuming it one by one 4. If there are more than 2 options throw them away by squashing them to `Many` (same as before) This is also why I had to update some tests as the expressions get squashed to many more eagerly. ### Use fuel to avoid long search times and high memory usage Now all the tactics use `should_continue: Fn() -> bool` to chech if they should keep iterating _(Similarly to chalk)_. This reduces the search times by a magnitude, for example from ~139ms/hole to ~14ms/hole for `ripgrep` crate. There are slightly less expressions found, but I think speed gain worth it for usability. Also note that syntactic hits decreases more because of squashing so you simple need to run search multiple times to get full terms. Also the worst case time (For example `nalgebra` crate cus it has tons of generics) has search times mostly under 200ms. Benchmarks on `ripgrep` crate Before: ``` Tail Expr syntactic hits: 291/1692 (17%) Tail Exprs found: 1253/1692 (74%) Term search avg time: 139ms ```` After: ``` Tail Expr syntactic hits: 239/1692 (14%) Tail Exprs found: 1226/1692 (72%) Term search avg time: 14ms ```
2 parents 03f935d + 8ef19e7 commit 197856d

File tree

17 files changed

+141
-37
lines changed

17 files changed

+141
-37
lines changed

src/tools/rust-analyzer/crates/hir/src/term_search.rs

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,13 @@ impl LookupTable {
127127
self.types_wishlist.insert(ty.clone());
128128
}
129129

130+
// Collapse suggestions if there are many
131+
if let Some(res) = &res {
132+
if res.len() > self.many_threshold {
133+
return Some(vec![Expr::Many(ty.clone())]);
134+
}
135+
}
136+
130137
res
131138
}
132139

@@ -158,6 +165,13 @@ impl LookupTable {
158165
self.types_wishlist.insert(ty.clone());
159166
}
160167

168+
// Collapse suggestions if there are many
169+
if let Some(res) = &res {
170+
if res.len() > self.many_threshold {
171+
return Some(vec![Expr::Many(ty.clone())]);
172+
}
173+
}
174+
161175
res
162176
}
163177

@@ -255,13 +269,13 @@ pub struct TermSearchConfig {
255269
pub enable_borrowcheck: bool,
256270
/// Indicate when to squash multiple trees to `Many` as there are too many to keep track
257271
pub many_alternatives_threshold: usize,
258-
/// Depth of the search eg. number of cycles to run
259-
pub depth: usize,
272+
/// Fuel for term search in "units of work"
273+
pub fuel: u64,
260274
}
261275

262276
impl Default for TermSearchConfig {
263277
fn default() -> Self {
264-
Self { enable_borrowcheck: true, many_alternatives_threshold: 1, depth: 6 }
278+
Self { enable_borrowcheck: true, many_alternatives_threshold: 1, fuel: 400 }
265279
}
266280
}
267281

@@ -280,8 +294,7 @@ impl Default for TermSearchConfig {
280294
/// transformation tactics. For example functions take as from set of types (arguments) to some
281295
/// type (return type). Other transformations include methods on type, type constructors and
282296
/// projections to struct fields (field access).
283-
/// 3. Once we manage to find path to type we are interested in we continue for single round to see
284-
/// if we can find more paths that take us to the `goal` type.
297+
/// 3. If we run out of fuel (term search takes too long) we stop iterating.
285298
/// 4. Return all the paths (type trees) that take us to the `goal` type.
286299
///
287300
/// Note that there are usually more ways we can get to the `goal` type but some are discarded to
@@ -297,21 +310,31 @@ pub fn term_search<DB: HirDatabase>(ctx: &TermSearchCtx<'_, DB>) -> Vec<Expr> {
297310
});
298311

299312
let mut lookup = LookupTable::new(ctx.config.many_alternatives_threshold, ctx.goal.clone());
313+
let fuel = std::cell::Cell::new(ctx.config.fuel);
314+
315+
let should_continue = &|| {
316+
let remaining = fuel.get();
317+
fuel.set(remaining.saturating_sub(1));
318+
if remaining == 0 {
319+
tracing::debug!("fuel exhausted");
320+
}
321+
remaining > 0
322+
};
300323

301324
// Try trivial tactic first, also populates lookup table
302325
let mut solutions: Vec<Expr> = tactics::trivial(ctx, &defs, &mut lookup).collect();
303326
// Use well known types tactic before iterations as it does not depend on other tactics
304327
solutions.extend(tactics::famous_types(ctx, &defs, &mut lookup));
305328

306-
for _ in 0..ctx.config.depth {
329+
while should_continue() {
307330
lookup.new_round();
308331

309-
solutions.extend(tactics::type_constructor(ctx, &defs, &mut lookup));
310-
solutions.extend(tactics::free_function(ctx, &defs, &mut lookup));
311-
solutions.extend(tactics::impl_method(ctx, &defs, &mut lookup));
312-
solutions.extend(tactics::struct_projection(ctx, &defs, &mut lookup));
313-
solutions.extend(tactics::impl_static_method(ctx, &defs, &mut lookup));
314-
solutions.extend(tactics::make_tuple(ctx, &defs, &mut lookup));
332+
solutions.extend(tactics::type_constructor(ctx, &defs, &mut lookup, should_continue));
333+
solutions.extend(tactics::free_function(ctx, &defs, &mut lookup, should_continue));
334+
solutions.extend(tactics::impl_method(ctx, &defs, &mut lookup, should_continue));
335+
solutions.extend(tactics::struct_projection(ctx, &defs, &mut lookup, should_continue));
336+
solutions.extend(tactics::impl_static_method(ctx, &defs, &mut lookup, should_continue));
337+
solutions.extend(tactics::make_tuple(ctx, &defs, &mut lookup, should_continue));
315338

316339
// Discard not interesting `ScopeDef`s for speedup
317340
for def in lookup.exhausted_scopedefs() {

src/tools/rust-analyzer/crates/hir/src/term_search/expr.rs

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,13 @@ impl Expr {
211211
}
212212
}
213213
Expr::Method { func, target, params, .. } => {
214-
if target.contains_many_in_illegal_pos() {
214+
if self.contains_many_in_illegal_pos(db) {
215215
return Ok(many_formatter(&target.ty(db)));
216216
}
217217

218218
let func_name = func.name(db).display(db.upcast()).to_string();
219219
let self_param = func.self_param(db).unwrap();
220-
let target = target.gen_source_code(
220+
let target_str = target.gen_source_code(
221221
sema_scope,
222222
many_formatter,
223223
prefer_no_std,
@@ -236,17 +236,20 @@ impl Expr {
236236
Some(trait_) => {
237237
let trait_name = mod_item_path_str(sema_scope, &ModuleDef::Trait(trait_))?;
238238
let target = match self_param.access(db) {
239-
crate::Access::Shared => format!("&{target}"),
240-
crate::Access::Exclusive => format!("&mut {target}"),
241-
crate::Access::Owned => target,
239+
crate::Access::Shared if !target.is_many() => format!("&{target_str}"),
240+
crate::Access::Exclusive if !target.is_many() => {
241+
format!("&mut {target_str}")
242+
}
243+
crate::Access::Owned => target_str,
244+
_ => many_formatter(&target.ty(db)),
242245
};
243246
let res = match args.is_empty() {
244247
true => format!("{trait_name}::{func_name}({target})",),
245248
false => format!("{trait_name}::{func_name}({target}, {args})",),
246249
};
247250
Ok(res)
248251
}
249-
None => Ok(format!("{target}.{func_name}({args})")),
252+
None => Ok(format!("{target_str}.{func_name}({args})")),
250253
}
251254
}
252255
Expr::Variant { variant, generics, params } => {
@@ -381,7 +384,7 @@ impl Expr {
381384
Ok(res)
382385
}
383386
Expr::Field { expr, field } => {
384-
if expr.contains_many_in_illegal_pos() {
387+
if expr.contains_many_in_illegal_pos(db) {
385388
return Ok(many_formatter(&expr.ty(db)));
386389
}
387390

@@ -395,7 +398,7 @@ impl Expr {
395398
Ok(format!("{strukt}.{field}"))
396399
}
397400
Expr::Reference(expr) => {
398-
if expr.contains_many_in_illegal_pos() {
401+
if expr.contains_many_in_illegal_pos(db) {
399402
return Ok(many_formatter(&expr.ty(db)));
400403
}
401404

@@ -466,10 +469,15 @@ impl Expr {
466469
/// macro!().bar()
467470
/// &macro!()
468471
/// ```
469-
fn contains_many_in_illegal_pos(&self) -> bool {
472+
fn contains_many_in_illegal_pos(&self, db: &dyn HirDatabase) -> bool {
470473
match self {
471-
Expr::Method { target, .. } => target.contains_many_in_illegal_pos(),
472-
Expr::Field { expr, .. } => expr.contains_many_in_illegal_pos(),
474+
Expr::Method { target, func, .. } => {
475+
match func.as_assoc_item(db).and_then(|it| it.container_or_implemented_trait(db)) {
476+
Some(_) => false,
477+
None => target.is_many(),
478+
}
479+
}
480+
Expr::Field { expr, .. } => expr.contains_many_in_illegal_pos(db),
473481
Expr::Reference(target) => target.is_many(),
474482
Expr::Many(_) => true,
475483
_ => false,

src/tools/rust-analyzer/crates/hir/src/term_search/tactics.rs

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//! * `ctx` - Context for the term search
55
//! * `defs` - Set of items in scope at term search target location
66
//! * `lookup` - Lookup table for types
7+
//! * `should_continue` - Function that indicates when to stop iterating
78
//! And they return iterator that yields type trees that unify with the `goal` type.
89
910
use std::iter;
@@ -97,16 +98,19 @@ pub(super) fn trivial<'a, DB: HirDatabase>(
9798
/// * `ctx` - Context for the term search
9899
/// * `defs` - Set of items in scope at term search target location
99100
/// * `lookup` - Lookup table for types
101+
/// * `should_continue` - Function that indicates when to stop iterating
100102
pub(super) fn type_constructor<'a, DB: HirDatabase>(
101103
ctx: &'a TermSearchCtx<'a, DB>,
102104
defs: &'a FxHashSet<ScopeDef>,
103105
lookup: &'a mut LookupTable,
106+
should_continue: &'a dyn std::ops::Fn() -> bool,
104107
) -> impl Iterator<Item = Expr> + 'a {
105108
let db = ctx.sema.db;
106109
let module = ctx.scope.module();
107110
fn variant_helper(
108111
db: &dyn HirDatabase,
109112
lookup: &mut LookupTable,
113+
should_continue: &dyn std::ops::Fn() -> bool,
110114
parent_enum: Enum,
111115
variant: Variant,
112116
config: &TermSearchConfig,
@@ -152,6 +156,7 @@ pub(super) fn type_constructor<'a, DB: HirDatabase>(
152156
.chain((non_default_type_params_len == 0).then_some(Vec::new()));
153157

154158
generic_params
159+
.filter(|_| should_continue())
155160
.filter_map(move |generics| {
156161
// Insert default type params
157162
let mut g = generics.into_iter();
@@ -194,8 +199,14 @@ pub(super) fn type_constructor<'a, DB: HirDatabase>(
194199
defs.iter()
195200
.filter_map(move |def| match def {
196201
ScopeDef::ModuleDef(ModuleDef::Variant(it)) => {
197-
let variant_exprs =
198-
variant_helper(db, lookup, it.parent_enum(db), *it, &ctx.config);
202+
let variant_exprs = variant_helper(
203+
db,
204+
lookup,
205+
should_continue,
206+
it.parent_enum(db),
207+
*it,
208+
&ctx.config,
209+
);
199210
if variant_exprs.is_empty() {
200211
return None;
201212
}
@@ -213,7 +224,9 @@ pub(super) fn type_constructor<'a, DB: HirDatabase>(
213224
let exprs: Vec<(Type, Vec<Expr>)> = enum_
214225
.variants(db)
215226
.into_iter()
216-
.flat_map(|it| variant_helper(db, lookup, *enum_, it, &ctx.config))
227+
.flat_map(|it| {
228+
variant_helper(db, lookup, should_continue, *enum_, it, &ctx.config)
229+
})
217230
.collect();
218231

219232
if exprs.is_empty() {
@@ -271,6 +284,7 @@ pub(super) fn type_constructor<'a, DB: HirDatabase>(
271284
.chain((non_default_type_params_len == 0).then_some(Vec::new()));
272285

273286
let exprs = generic_params
287+
.filter(|_| should_continue())
274288
.filter_map(|generics| {
275289
// Insert default type params
276290
let mut g = generics.into_iter();
@@ -345,10 +359,12 @@ pub(super) fn type_constructor<'a, DB: HirDatabase>(
345359
/// * `ctx` - Context for the term search
346360
/// * `defs` - Set of items in scope at term search target location
347361
/// * `lookup` - Lookup table for types
362+
/// * `should_continue` - Function that indicates when to stop iterating
348363
pub(super) fn free_function<'a, DB: HirDatabase>(
349364
ctx: &'a TermSearchCtx<'a, DB>,
350365
defs: &'a FxHashSet<ScopeDef>,
351366
lookup: &'a mut LookupTable,
367+
should_continue: &'a dyn std::ops::Fn() -> bool,
352368
) -> impl Iterator<Item = Expr> + 'a {
353369
let db = ctx.sema.db;
354370
let module = ctx.scope.module();
@@ -390,6 +406,7 @@ pub(super) fn free_function<'a, DB: HirDatabase>(
390406
.permutations(non_default_type_params_len);
391407

392408
let exprs: Vec<_> = generic_params
409+
.filter(|_| should_continue())
393410
.filter_map(|generics| {
394411
// Insert default type params
395412
let mut g = generics.into_iter();
@@ -474,10 +491,12 @@ pub(super) fn free_function<'a, DB: HirDatabase>(
474491
/// * `ctx` - Context for the term search
475492
/// * `defs` - Set of items in scope at term search target location
476493
/// * `lookup` - Lookup table for types
494+
/// * `should_continue` - Function that indicates when to stop iterating
477495
pub(super) fn impl_method<'a, DB: HirDatabase>(
478496
ctx: &'a TermSearchCtx<'a, DB>,
479497
_defs: &'a FxHashSet<ScopeDef>,
480498
lookup: &'a mut LookupTable,
499+
should_continue: &'a dyn std::ops::Fn() -> bool,
481500
) -> impl Iterator<Item = Expr> + 'a {
482501
let db = ctx.sema.db;
483502
let module = ctx.scope.module();
@@ -554,6 +573,7 @@ pub(super) fn impl_method<'a, DB: HirDatabase>(
554573
.permutations(non_default_fn_type_params_len);
555574

556575
let exprs: Vec<_> = generic_params
576+
.filter(|_| should_continue())
557577
.filter_map(|generics| {
558578
// Insert default type params
559579
let mut g = generics.into_iter();
@@ -645,17 +665,20 @@ pub(super) fn impl_method<'a, DB: HirDatabase>(
645665
/// * `ctx` - Context for the term search
646666
/// * `defs` - Set of items in scope at term search target location
647667
/// * `lookup` - Lookup table for types
668+
/// * `should_continue` - Function that indicates when to stop iterating
648669
pub(super) fn struct_projection<'a, DB: HirDatabase>(
649670
ctx: &'a TermSearchCtx<'a, DB>,
650671
_defs: &'a FxHashSet<ScopeDef>,
651672
lookup: &'a mut LookupTable,
673+
should_continue: &'a dyn std::ops::Fn() -> bool,
652674
) -> impl Iterator<Item = Expr> + 'a {
653675
let db = ctx.sema.db;
654676
let module = ctx.scope.module();
655677
lookup
656678
.new_types(NewTypesKey::StructProjection)
657679
.into_iter()
658680
.map(|ty| (ty.clone(), lookup.find(db, &ty).expect("Expr not in lookup")))
681+
.filter(|_| should_continue())
659682
.flat_map(move |(ty, targets)| {
660683
ty.fields(db).into_iter().filter_map(move |(field, filed_ty)| {
661684
if !field.is_visible_from(db, module) {
@@ -716,10 +739,12 @@ pub(super) fn famous_types<'a, DB: HirDatabase>(
716739
/// * `ctx` - Context for the term search
717740
/// * `defs` - Set of items in scope at term search target location
718741
/// * `lookup` - Lookup table for types
742+
/// * `should_continue` - Function that indicates when to stop iterating
719743
pub(super) fn impl_static_method<'a, DB: HirDatabase>(
720744
ctx: &'a TermSearchCtx<'a, DB>,
721745
_defs: &'a FxHashSet<ScopeDef>,
722746
lookup: &'a mut LookupTable,
747+
should_continue: &'a dyn std::ops::Fn() -> bool,
723748
) -> impl Iterator<Item = Expr> + 'a {
724749
let db = ctx.sema.db;
725750
let module = ctx.scope.module();
@@ -728,6 +753,7 @@ pub(super) fn impl_static_method<'a, DB: HirDatabase>(
728753
.clone()
729754
.into_iter()
730755
.chain(iter::once(ctx.goal.clone()))
756+
.filter(|_| should_continue())
731757
.flat_map(|ty| {
732758
Impl::all_for_type(db, ty.clone()).into_iter().map(move |imp| (ty.clone(), imp))
733759
})
@@ -801,6 +827,7 @@ pub(super) fn impl_static_method<'a, DB: HirDatabase>(
801827
.permutations(non_default_fn_type_params_len);
802828

803829
let exprs: Vec<_> = generic_params
830+
.filter(|_| should_continue())
804831
.filter_map(|generics| {
805832
// Insert default type params
806833
let mut g = generics.into_iter();
@@ -884,10 +911,12 @@ pub(super) fn impl_static_method<'a, DB: HirDatabase>(
884911
/// * `ctx` - Context for the term search
885912
/// * `defs` - Set of items in scope at term search target location
886913
/// * `lookup` - Lookup table for types
914+
/// * `should_continue` - Function that indicates when to stop iterating
887915
pub(super) fn make_tuple<'a, DB: HirDatabase>(
888916
ctx: &'a TermSearchCtx<'a, DB>,
889917
_defs: &'a FxHashSet<ScopeDef>,
890918
lookup: &'a mut LookupTable,
919+
should_continue: &'a dyn std::ops::Fn() -> bool,
891920
) -> impl Iterator<Item = Expr> + 'a {
892921
let db = ctx.sema.db;
893922
let module = ctx.scope.module();
@@ -896,6 +925,7 @@ pub(super) fn make_tuple<'a, DB: HirDatabase>(
896925
.types_wishlist()
897926
.clone()
898927
.into_iter()
928+
.filter(|_| should_continue())
899929
.filter(|ty| ty.is_tuple())
900930
.filter_map(move |ty| {
901931
// Double check to not contain unknown
@@ -915,6 +945,7 @@ pub(super) fn make_tuple<'a, DB: HirDatabase>(
915945
let exprs: Vec<Expr> = param_exprs
916946
.into_iter()
917947
.multi_cartesian_product()
948+
.filter(|_| should_continue())
918949
.map(|params| {
919950
let tys: Vec<Type> = params.iter().map(|it| it.ty(db)).collect();
920951
let tuple_ty = Type::new_tuple(module.krate().into(), &tys);

src/tools/rust-analyzer/crates/ide-assists/src/assist_config.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,5 @@ pub struct AssistConfig {
1616
pub prefer_no_std: bool,
1717
pub prefer_prelude: bool,
1818
pub assist_emit_must_use: bool,
19+
pub term_search_fuel: u64,
1920
}

src/tools/rust-analyzer/crates/ide-assists/src/handlers/term_search.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//! Term search assist
2-
use hir::term_search::TermSearchCtx;
2+
use hir::term_search::{TermSearchConfig, TermSearchCtx};
33
use ide_db::{
44
assists::{AssistId, AssistKind, GroupLabel},
55
famous_defs::FamousDefs,
@@ -34,7 +34,7 @@ pub(crate) fn term_search(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<
3434
sema: &ctx.sema,
3535
scope: &scope,
3636
goal: target_ty,
37-
config: Default::default(),
37+
config: TermSearchConfig { fuel: ctx.config.term_search_fuel, ..Default::default() },
3838
};
3939
let paths = hir::term_search::term_search(&term_search_ctx);
4040

0 commit comments

Comments
 (0)