diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2019-11-24 21:45:26 +0000 |
---|---|---|
committer | GitHub <[email protected]> | 2019-11-24 21:45:26 +0000 |
commit | f7f9757b6b144385ab8ce57b15764473b1f57331 (patch) | |
tree | dfcb584a03bd4e049291a05912508690cd8f9247 /crates/ra_hir/src | |
parent | 6a5dea778b16a35d765a3a98f2ac303c5cc3df25 (diff) | |
parent | d06904e90cdc1603ffcb714e70dab83905221f72 (diff) |
Merge #2396
2396: Switch to variant-granularity field type inference r=flodiebold a=matklad
r? @flodiebold
Previously, we had a `ty` query for each field. This PR switcthes to a query per struct, which returns an `ArenaMap` with `Ty`s.
I don't know which approach is better. What is bugging me about the original approach is that, if we do all queries on the "leaf" defs, in practice we get a ton of queries which repeatedly reach into the parent definition to compute module, resolver, etc. This *seems* wasteful (but I don't think this is really what causes any perf problems for us).
At the same time, I've been looking at Kotlin, and they seem to use the general pattern of analyzing the *parent* definition, and storing info about children into a `BindingContext`.
I don't really which way is preferable. I think I want to try this approach, where query granularity generally mirrors the data granularity. The primary motivation for me here is probably just hope that we can avoid adding a ton of helpers to a `StructField`, and maybe in general avoid the need to switch to a global `StructField`, using `LocalStructFieldId` most of the time internally.
For external API (ie, for `ra_ide_api`), I think we should continue with fine-grained `StructField::ty` approach, which internally fetches the table for the whole struct and indexes into it.
In terms of actual memory savings, the results are as follows:
```
This PR:
142kb FieldTypesQuery (deps)
38kb FieldTypesQuery
Status Quo:
208kb TypeForFieldQuery (deps)
18kb TypeForFieldQuery
```
Note how the table itself occupies more than twice as much space! I don't have an explanation for this: a plausible hypothesis is that single-field structs are very common and for them the table is a pessimisation.
THere's noticiable wallclock time difference.
Co-authored-by: Aleksey Kladov <[email protected]>
Diffstat (limited to 'crates/ra_hir/src')
-rw-r--r-- | crates/ra_hir/src/code_model.rs | 2 | ||||
-rw-r--r-- | crates/ra_hir/src/db.rs | 23 | ||||
-rw-r--r-- | crates/ra_hir/src/ty.rs | 5 | ||||
-rw-r--r-- | crates/ra_hir/src/ty/infer/coerce.rs | 13 | ||||
-rw-r--r-- | crates/ra_hir/src/ty/infer/expr.rs | 11 | ||||
-rw-r--r-- | crates/ra_hir/src/ty/infer/pat.rs | 9 | ||||
-rw-r--r-- | crates/ra_hir/src/ty/lower.rs | 30 |
7 files changed, 59 insertions, 34 deletions
diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs index ab0544a4c..3f44a50c4 100644 --- a/crates/ra_hir/src/code_model.rs +++ b/crates/ra_hir/src/code_model.rs | |||
@@ -305,7 +305,7 @@ impl StructField { | |||
305 | } | 305 | } |
306 | 306 | ||
307 | pub fn ty(&self, db: &impl HirDatabase) -> Ty { | 307 | pub fn ty(&self, db: &impl HirDatabase) -> Ty { |
308 | db.type_for_field(*self) | 308 | db.field_types(self.parent.into())[self.id].clone() |
309 | } | 309 | } |
310 | 310 | ||
311 | pub fn parent_def(&self, _db: &impl HirDatabase) -> VariantDef { | 311 | pub fn parent_def(&self, _db: &impl HirDatabase) -> VariantDef { |
diff --git a/crates/ra_hir/src/db.rs b/crates/ra_hir/src/db.rs index a9dab2d25..5084bbacf 100644 --- a/crates/ra_hir/src/db.rs +++ b/crates/ra_hir/src/db.rs | |||
@@ -2,6 +2,7 @@ | |||
2 | 2 | ||
3 | use std::sync::Arc; | 3 | use std::sync::Arc; |
4 | 4 | ||
5 | use ra_arena::map::ArenaMap; | ||
5 | use ra_db::salsa; | 6 | use ra_db::salsa; |
6 | 7 | ||
7 | use crate::{ | 8 | use crate::{ |
@@ -11,15 +12,19 @@ use crate::{ | |||
11 | CallableDef, FnSig, GenericPredicate, InferenceResult, Namespace, Substs, Ty, TypableDef, | 12 | CallableDef, FnSig, GenericPredicate, InferenceResult, Namespace, Substs, Ty, TypableDef, |
12 | TypeCtor, | 13 | TypeCtor, |
13 | }, | 14 | }, |
14 | Crate, DefWithBody, GenericDef, ImplBlock, StructField, Trait, | 15 | Crate, DefWithBody, GenericDef, ImplBlock, Trait, |
15 | }; | 16 | }; |
16 | 17 | ||
17 | pub use hir_def::db::{ | 18 | pub use hir_def::{ |
18 | BodyQuery, BodyWithSourceMapQuery, ConstDataQuery, CrateDefMapQuery, CrateLangItemsQuery, | 19 | db::{ |
19 | DefDatabase, DefDatabaseStorage, DocumentationQuery, EnumDataQuery, ExprScopesQuery, | 20 | BodyQuery, BodyWithSourceMapQuery, ConstDataQuery, CrateDefMapQuery, CrateLangItemsQuery, |
20 | FunctionDataQuery, GenericParamsQuery, ImplDataQuery, InternDatabase, InternDatabaseStorage, | 21 | DefDatabase, DefDatabaseStorage, DocumentationQuery, EnumDataQuery, ExprScopesQuery, |
21 | LangItemQuery, ModuleLangItemsQuery, RawItemsQuery, RawItemsWithSourceMapQuery, | 22 | FunctionDataQuery, GenericParamsQuery, ImplDataQuery, InternDatabase, |
22 | StaticDataQuery, StructDataQuery, TraitDataQuery, TypeAliasDataQuery, | 23 | InternDatabaseStorage, LangItemQuery, ModuleLangItemsQuery, RawItemsQuery, |
24 | RawItemsWithSourceMapQuery, StaticDataQuery, StructDataQuery, TraitDataQuery, | ||
25 | TypeAliasDataQuery, | ||
26 | }, | ||
27 | LocalStructFieldId, VariantId, | ||
23 | }; | 28 | }; |
24 | pub use hir_expand::db::{ | 29 | pub use hir_expand::db::{ |
25 | AstDatabase, AstDatabaseStorage, AstIdMapQuery, MacroArgQuery, MacroDefQuery, MacroExpandQuery, | 30 | AstDatabase, AstDatabaseStorage, AstIdMapQuery, MacroArgQuery, MacroDefQuery, MacroExpandQuery, |
@@ -35,8 +40,8 @@ pub trait HirDatabase: DefDatabase { | |||
35 | #[salsa::invoke(crate::ty::type_for_def)] | 40 | #[salsa::invoke(crate::ty::type_for_def)] |
36 | fn type_for_def(&self, def: TypableDef, ns: Namespace) -> Ty; | 41 | fn type_for_def(&self, def: TypableDef, ns: Namespace) -> Ty; |
37 | 42 | ||
38 | #[salsa::invoke(crate::ty::type_for_field)] | 43 | #[salsa::invoke(crate::ty::field_types_query)] |
39 | fn type_for_field(&self, field: StructField) -> Ty; | 44 | fn field_types(&self, var: VariantId) -> Arc<ArenaMap<LocalStructFieldId, Ty>>; |
40 | 45 | ||
41 | #[salsa::invoke(crate::ty::callable_item_sig)] | 46 | #[salsa::invoke(crate::ty::callable_item_sig)] |
42 | fn callable_item_signature(&self, def: CallableDef) -> FnSig; | 47 | fn callable_item_signature(&self, def: CallableDef) -> FnSig; |
diff --git a/crates/ra_hir/src/ty.rs b/crates/ra_hir/src/ty.rs index 309bd2727..f62316c1f 100644 --- a/crates/ra_hir/src/ty.rs +++ b/crates/ra_hir/src/ty.rs | |||
@@ -30,8 +30,9 @@ pub(crate) use autoderef::autoderef; | |||
30 | pub(crate) use infer::{infer_query, InferTy, InferenceResult}; | 30 | pub(crate) use infer::{infer_query, InferTy, InferenceResult}; |
31 | pub use lower::CallableDef; | 31 | pub use lower::CallableDef; |
32 | pub(crate) use lower::{ | 32 | pub(crate) use lower::{ |
33 | callable_item_sig, generic_defaults_query, generic_predicates_for_param_query, | 33 | callable_item_sig, field_types_query, generic_defaults_query, |
34 | generic_predicates_query, type_for_def, type_for_field, Namespace, TypableDef, | 34 | generic_predicates_for_param_query, generic_predicates_query, type_for_def, Namespace, |
35 | TypableDef, | ||
35 | }; | 36 | }; |
36 | pub(crate) use traits::{InEnvironment, Obligation, ProjectionPredicate, TraitEnvironment}; | 37 | pub(crate) use traits::{InEnvironment, Obligation, ProjectionPredicate, TraitEnvironment}; |
37 | 38 | ||
diff --git a/crates/ra_hir/src/ty/infer/coerce.rs b/crates/ra_hir/src/ty/infer/coerce.rs index 4ea038d99..54765da35 100644 --- a/crates/ra_hir/src/ty/infer/coerce.rs +++ b/crates/ra_hir/src/ty/infer/coerce.rs | |||
@@ -245,14 +245,17 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { | |||
245 | ty_app!(TypeCtor::Adt(Adt::Struct(struct1)), st1), | 245 | ty_app!(TypeCtor::Adt(Adt::Struct(struct1)), st1), |
246 | ty_app!(TypeCtor::Adt(Adt::Struct(struct2)), st2), | 246 | ty_app!(TypeCtor::Adt(Adt::Struct(struct2)), st2), |
247 | ) if struct1 == struct2 => { | 247 | ) if struct1 == struct2 => { |
248 | let fields = struct1.fields(self.db); | 248 | let field_tys = self.db.field_types(struct1.id.into()); |
249 | let (last_field, prev_fields) = fields.split_last()?; | 249 | let struct_data = self.db.struct_data(struct1.id.0); |
250 | |||
251 | let mut fields = struct_data.variant_data.fields().iter(); | ||
252 | let (last_field_id, _data) = fields.next_back()?; | ||
250 | 253 | ||
251 | // Get the generic parameter involved in the last field. | 254 | // Get the generic parameter involved in the last field. |
252 | let unsize_generic_index = { | 255 | let unsize_generic_index = { |
253 | let mut index = None; | 256 | let mut index = None; |
254 | let mut multiple_param = false; | 257 | let mut multiple_param = false; |
255 | last_field.ty(self.db).walk(&mut |ty| match ty { | 258 | field_tys[last_field_id].walk(&mut |ty| match ty { |
256 | &Ty::Param { idx, .. } => { | 259 | &Ty::Param { idx, .. } => { |
257 | if index.is_none() { | 260 | if index.is_none() { |
258 | index = Some(idx); | 261 | index = Some(idx); |
@@ -271,8 +274,8 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { | |||
271 | 274 | ||
272 | // Check other fields do not involve it. | 275 | // Check other fields do not involve it. |
273 | let mut multiple_used = false; | 276 | let mut multiple_used = false; |
274 | prev_fields.iter().for_each(|field| { | 277 | fields.for_each(|(field_id, _data)| { |
275 | field.ty(self.db).walk(&mut |ty| match ty { | 278 | field_tys[field_id].walk(&mut |ty| match ty { |
276 | &Ty::Param { idx, .. } if idx == unsize_generic_index => { | 279 | &Ty::Param { idx, .. } if idx == unsize_generic_index => { |
277 | multiple_used = true | 280 | multiple_used = true |
278 | } | 281 | } |
diff --git a/crates/ra_hir/src/ty/infer/expr.rs b/crates/ra_hir/src/ty/infer/expr.rs index 2996920c6..663ff9435 100644 --- a/crates/ra_hir/src/ty/infer/expr.rs +++ b/crates/ra_hir/src/ty/infer/expr.rs | |||
@@ -214,6 +214,8 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { | |||
214 | self.unify(&ty, &expected.ty); | 214 | self.unify(&ty, &expected.ty); |
215 | 215 | ||
216 | let substs = ty.substs().unwrap_or_else(Substs::empty); | 216 | let substs = ty.substs().unwrap_or_else(Substs::empty); |
217 | let field_types = | ||
218 | def_id.map(|it| self.db.field_types(it.into())).unwrap_or_default(); | ||
217 | for (field_idx, field) in fields.iter().enumerate() { | 219 | for (field_idx, field) in fields.iter().enumerate() { |
218 | let field_def = def_id.and_then(|it| match it.field(self.db, &field.name) { | 220 | let field_def = def_id.and_then(|it| match it.field(self.db, &field.name) { |
219 | Some(field) => Some(field), | 221 | Some(field) => Some(field), |
@@ -228,8 +230,9 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { | |||
228 | if let Some(field_def) = field_def { | 230 | if let Some(field_def) = field_def { |
229 | self.result.record_field_resolutions.insert(field.expr, field_def); | 231 | self.result.record_field_resolutions.insert(field.expr, field_def); |
230 | } | 232 | } |
231 | let field_ty = | 233 | let field_ty = field_def |
232 | field_def.map_or(Ty::Unknown, |field| field.ty(self.db)).subst(&substs); | 234 | .map_or(Ty::Unknown, |it| field_types[it.id].clone()) |
235 | .subst(&substs); | ||
233 | self.infer_expr_coerce(field.expr, &Expectation::has_type(field_ty)); | 236 | self.infer_expr_coerce(field.expr, &Expectation::has_type(field_ty)); |
234 | } | 237 | } |
235 | if let Some(expr) = spread { | 238 | if let Some(expr) = spread { |
@@ -252,7 +255,9 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { | |||
252 | .and_then(|idx| a_ty.parameters.0.get(idx).cloned()), | 255 | .and_then(|idx| a_ty.parameters.0.get(idx).cloned()), |
253 | TypeCtor::Adt(Adt::Struct(s)) => s.field(self.db, name).map(|field| { | 256 | TypeCtor::Adt(Adt::Struct(s)) => s.field(self.db, name).map(|field| { |
254 | self.write_field_resolution(tgt_expr, field); | 257 | self.write_field_resolution(tgt_expr, field); |
255 | field.ty(self.db).subst(&a_ty.parameters) | 258 | self.db.field_types(s.id.into())[field.id] |
259 | .clone() | ||
260 | .subst(&a_ty.parameters) | ||
256 | }), | 261 | }), |
257 | _ => None, | 262 | _ => None, |
258 | }, | 263 | }, |
diff --git a/crates/ra_hir/src/ty/infer/pat.rs b/crates/ra_hir/src/ty/infer/pat.rs index c125ddfbc..641d61e87 100644 --- a/crates/ra_hir/src/ty/infer/pat.rs +++ b/crates/ra_hir/src/ty/infer/pat.rs | |||
@@ -27,10 +27,11 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { | |||
27 | 27 | ||
28 | let substs = ty.substs().unwrap_or_else(Substs::empty); | 28 | let substs = ty.substs().unwrap_or_else(Substs::empty); |
29 | 29 | ||
30 | let field_tys = def.map(|it| self.db.field_types(it.into())).unwrap_or_default(); | ||
30 | for (i, &subpat) in subpats.iter().enumerate() { | 31 | for (i, &subpat) in subpats.iter().enumerate() { |
31 | let expected_ty = def | 32 | let expected_ty = def |
32 | .and_then(|d| d.field(self.db, &Name::new_tuple_field(i))) | 33 | .and_then(|d| d.field(self.db, &Name::new_tuple_field(i))) |
33 | .map_or(Ty::Unknown, |field| field.ty(self.db)) | 34 | .map_or(Ty::Unknown, |field| field_tys[field.id].clone()) |
34 | .subst(&substs); | 35 | .subst(&substs); |
35 | let expected_ty = self.normalize_associated_types_in(expected_ty); | 36 | let expected_ty = self.normalize_associated_types_in(expected_ty); |
36 | self.infer_pat(subpat, &expected_ty, default_bm); | 37 | self.infer_pat(subpat, &expected_ty, default_bm); |
@@ -56,10 +57,12 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> { | |||
56 | 57 | ||
57 | let substs = ty.substs().unwrap_or_else(Substs::empty); | 58 | let substs = ty.substs().unwrap_or_else(Substs::empty); |
58 | 59 | ||
60 | let field_tys = def.map(|it| self.db.field_types(it.into())).unwrap_or_default(); | ||
59 | for subpat in subpats { | 61 | for subpat in subpats { |
60 | let matching_field = def.and_then(|it| it.field(self.db, &subpat.name)); | 62 | let matching_field = def.and_then(|it| it.field(self.db, &subpat.name)); |
61 | let expected_ty = | 63 | let expected_ty = matching_field |
62 | matching_field.map_or(Ty::Unknown, |field| field.ty(self.db)).subst(&substs); | 64 | .map_or(Ty::Unknown, |field| field_tys[field.id].clone()) |
65 | .subst(&substs); | ||
63 | let expected_ty = self.normalize_associated_types_in(expected_ty); | 66 | let expected_ty = self.normalize_associated_types_in(expected_ty); |
64 | self.infer_pat(subpat.pat, &expected_ty, default_bm); | 67 | self.infer_pat(subpat.pat, &expected_ty, default_bm); |
65 | } | 68 | } |
diff --git a/crates/ra_hir/src/ty/lower.rs b/crates/ra_hir/src/ty/lower.rs index 2b40cb07d..a39beb2a0 100644 --- a/crates/ra_hir/src/ty/lower.rs +++ b/crates/ra_hir/src/ty/lower.rs | |||
@@ -14,8 +14,9 @@ use hir_def::{ | |||
14 | path::{GenericArg, PathSegment}, | 14 | path::{GenericArg, PathSegment}, |
15 | resolver::{HasResolver, Resolver, TypeNs}, | 15 | resolver::{HasResolver, Resolver, TypeNs}, |
16 | type_ref::{TypeBound, TypeRef}, | 16 | type_ref::{TypeBound, TypeRef}, |
17 | AdtId, GenericDefId, | 17 | AdtId, GenericDefId, LocalStructFieldId, VariantId, |
18 | }; | 18 | }; |
19 | use ra_arena::map::ArenaMap; | ||
19 | 20 | ||
20 | use super::{ | 21 | use super::{ |
21 | FnSig, GenericPredicate, ProjectionPredicate, ProjectionTy, Substs, TraitRef, Ty, TypeCtor, | 22 | FnSig, GenericPredicate, ProjectionPredicate, ProjectionTy, Substs, TraitRef, Ty, TypeCtor, |
@@ -29,7 +30,7 @@ use crate::{ | |||
29 | }, | 30 | }, |
30 | util::make_mut_slice, | 31 | util::make_mut_slice, |
31 | Const, Enum, EnumVariant, Function, GenericDef, ImplBlock, ModuleDef, Path, Static, Struct, | 32 | Const, Enum, EnumVariant, Function, GenericDef, ImplBlock, ModuleDef, Path, Static, Struct, |
32 | StructField, Trait, TypeAlias, Union, VariantDef, | 33 | Trait, TypeAlias, Union, |
33 | }; | 34 | }; |
34 | 35 | ||
35 | // FIXME: this is only really used in `type_for_def`, which contains a bunch of | 36 | // FIXME: this is only really used in `type_for_def`, which contains a bunch of |
@@ -549,16 +550,23 @@ pub(crate) fn callable_item_sig(db: &impl HirDatabase, def: CallableDef) -> FnSi | |||
549 | } | 550 | } |
550 | } | 551 | } |
551 | 552 | ||
552 | /// Build the type of a specific field of a struct or enum variant. | 553 | /// Build the type of all specific fields of a struct or enum variant. |
553 | pub(crate) fn type_for_field(db: &impl HirDatabase, field: StructField) -> Ty { | 554 | pub(crate) fn field_types_query( |
554 | let parent_def = field.parent_def(db); | 555 | db: &impl HirDatabase, |
555 | let resolver = match parent_def { | 556 | variant_id: VariantId, |
556 | VariantDef::Struct(it) => it.id.resolver(db), | 557 | ) -> Arc<ArenaMap<LocalStructFieldId, Ty>> { |
557 | VariantDef::EnumVariant(it) => it.parent.id.resolver(db), | 558 | let (resolver, var_data) = match variant_id { |
559 | VariantId::StructId(it) => (it.resolver(db), db.struct_data(it.0).variant_data.clone()), | ||
560 | VariantId::EnumVariantId(it) => ( | ||
561 | it.parent.resolver(db), | ||
562 | db.enum_data(it.parent).variants[it.local_id].variant_data.clone(), | ||
563 | ), | ||
558 | }; | 564 | }; |
559 | let var_data = parent_def.variant_data(db); | 565 | let mut res = ArenaMap::default(); |
560 | let type_ref = &var_data.fields()[field.id].type_ref; | 566 | for (field_id, field_data) in var_data.fields().iter() { |
561 | Ty::from_hir(db, &resolver, type_ref) | 567 | res.insert(field_id, Ty::from_hir(db, &resolver, &field_data.type_ref)) |
568 | } | ||
569 | Arc::new(res) | ||
562 | } | 570 | } |
563 | 571 | ||
564 | /// This query exists only to be used when resolving short-hand associated types | 572 | /// This query exists only to be used when resolving short-hand associated types |