From 9ce30281f6e618b53bc5e8d54be1cd4a4eae8cee Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 6 Mar 2020 23:04:14 +0100 Subject: Don't reuse the Chalk solver This slows down analysis-stats a bit (~5% in my measurement), but improves incremental checking a lot because we can reuse trait solve results. --- crates/ra_hir/src/db.rs | 3 +- crates/ra_hir_ty/src/db.rs | 8 --- crates/ra_hir_ty/src/traits.rs | 112 +++++++++++------------------------------ crates/ra_ide_db/src/change.rs | 1 - 4 files changed, 29 insertions(+), 95 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir/src/db.rs b/crates/ra_hir/src/db.rs index a77bf6de6..ee597cfd2 100644 --- a/crates/ra_hir/src/db.rs +++ b/crates/ra_hir/src/db.rs @@ -18,8 +18,7 @@ pub use hir_ty::db::{ FieldTypesQuery, GenericDefaultsQuery, GenericPredicatesForParamQuery, GenericPredicatesQuery, HirDatabase, HirDatabaseStorage, ImplDatumQuery, ImplSelfTyQuery, ImplTraitQuery, ImplsForTraitQuery, ImplsInCrateQuery, InternAssocTyValueQuery, InternChalkImplQuery, - InternTypeCtorQuery, StructDatumQuery, TraitDatumQuery, TraitSolveQuery, TraitSolverQuery, - TyQuery, ValueTyQuery, + InternTypeCtorQuery, StructDatumQuery, TraitDatumQuery, TraitSolveQuery, TyQuery, ValueTyQuery, }; #[test] diff --git a/crates/ra_hir_ty/src/db.rs b/crates/ra_hir_ty/src/db.rs index f79faa84d..7db28a1f8 100644 --- a/crates/ra_hir_ty/src/db.rs +++ b/crates/ra_hir_ty/src/db.rs @@ -66,14 +66,6 @@ pub trait HirDatabase: DefDatabase { #[salsa::invoke(crate::traits::impls_for_trait_query)] fn impls_for_trait(&self, krate: CrateId, trait_: TraitId) -> Arc<[ImplId]>; - /// This provides the Chalk trait solver instance. Because Chalk always - /// works from a specific crate, this query is keyed on the crate; and - /// because Chalk does its own internal caching, the solver is wrapped in a - /// Mutex and the query does an untracked read internally, to make sure the - /// cached state is thrown away when input facts change. - #[salsa::invoke(crate::traits::trait_solver_query)] - fn trait_solver(&self, krate: CrateId) -> crate::traits::TraitSolver; - // Interned IDs for Chalk integration #[salsa::interned] fn intern_type_ctor(&self, type_ctor: TypeCtor) -> crate::TypeCtorId; diff --git a/crates/ra_hir_ty/src/traits.rs b/crates/ra_hir_ty/src/traits.rs index bdf23ac02..8de588790 100644 --- a/crates/ra_hir_ty/src/traits.rs +++ b/crates/ra_hir_ty/src/traits.rs @@ -1,12 +1,9 @@ //! Trait solving using Chalk. -use std::{ - panic, - sync::{Arc, Mutex}, -}; +use std::{panic, sync::Arc}; use chalk_ir::cast::Cast; use hir_def::{expr::ExprId, DefWithBodyId, ImplId, TraitId, TypeAliasId}; -use ra_db::{impl_intern_key, salsa, Canceled, CrateId}; +use ra_db::{impl_intern_key, salsa, CrateId}; use ra_prof::profile; use rustc_hash::FxHashSet; @@ -19,74 +16,6 @@ use self::chalk::{from_chalk, Interner, ToChalk}; pub(crate) mod chalk; mod builtin; -#[derive(Debug, Clone)] -pub struct TraitSolver { - krate: CrateId, - inner: Arc>>, -} - -/// We need eq for salsa -impl PartialEq for TraitSolver { - fn eq(&self, other: &TraitSolver) -> bool { - Arc::ptr_eq(&self.inner, &other.inner) - } -} - -impl Eq for TraitSolver {} - -impl TraitSolver { - fn solve( - &self, - db: &impl HirDatabase, - goal: &chalk_ir::UCanonical>>, - ) -> Option> { - let context = ChalkContext { db, krate: self.krate }; - log::debug!("solve goal: {:?}", goal); - let mut solver = match self.inner.lock() { - Ok(it) => it, - // Our cancellation works via unwinding, but, as chalk is not - // panic-safe, we need to make sure to propagate the cancellation. - // Ideally, we should also make chalk panic-safe. - Err(_) => ra_db::Canceled::throw(), - }; - - let fuel = std::cell::Cell::new(CHALK_SOLVER_FUEL); - - let solution = panic::catch_unwind({ - let solver = panic::AssertUnwindSafe(&mut solver); - let context = panic::AssertUnwindSafe(&context); - move || { - solver.0.solve_limited(context.0, goal, || { - context.0.db.check_canceled(); - let remaining = fuel.get(); - fuel.set(remaining - 1); - if remaining == 0 { - log::debug!("fuel exhausted"); - } - remaining > 0 - }) - } - }); - - let solution = match solution { - Ok(it) => it, - Err(err) => { - if err.downcast_ref::().is_some() { - panic::resume_unwind(err) - } else { - log::error!("chalk panicked :-("); - // Reset the solver, as it is not panic-safe. - *solver = create_chalk_solver(); - None - } - } - }; - - log::debug!("solve({:?}) => {:?}", goal, solution); - solution - } -} - /// This controls the maximum size of types Chalk considers. If we set this too /// high, we can run into slow edge cases; if we set it too low, Chalk won't /// find some solutions. @@ -100,16 +29,6 @@ struct ChalkContext<'a, DB> { krate: CrateId, } -pub(crate) fn trait_solver_query( - db: &(impl HirDatabase + salsa::Database), - krate: CrateId, -) -> TraitSolver { - db.salsa_runtime().report_untracked_read(); - // krate parameter is just so we cache a unique solver per crate - log::debug!("Creating new solver for crate {:?}", krate); - TraitSolver { krate, inner: Arc::new(Mutex::new(create_chalk_solver())) } -} - fn create_chalk_solver() -> chalk_solve::Solver { let solver_choice = chalk_solve::SolverChoice::SLG { max_size: CHALK_SOLVER_MAX_SIZE, expected_answers: None }; @@ -239,10 +158,35 @@ pub(crate) fn trait_solve_query( // We currently don't deal with universes (I think / hope they're not yet // relevant for our use cases?) let u_canonical = chalk_ir::UCanonical { canonical, universes: 1 }; - let solution = db.trait_solver(krate).solve(db, &u_canonical); + let solution = solve(db, krate, &u_canonical); solution.map(|solution| solution_from_chalk(db, solution)) } +fn solve( + db: &impl HirDatabase, + krate: CrateId, + goal: &chalk_ir::UCanonical>>, +) -> Option> { + let context = ChalkContext { db, krate }; + log::debug!("solve goal: {:?}", goal); + let mut solver = create_chalk_solver(); + + let fuel = std::cell::Cell::new(CHALK_SOLVER_FUEL); + + let solution = solver.solve_limited(&context, goal, || { + context.db.check_canceled(); + let remaining = fuel.get(); + fuel.set(remaining - 1); + if remaining == 0 { + log::debug!("fuel exhausted"); + } + remaining > 0 + }); + + log::debug!("solve({:?}) => {:?}", goal, solution); + solution +} + fn solution_from_chalk( db: &impl HirDatabase, solution: chalk_solve::Solution, diff --git a/crates/ra_ide_db/src/change.rs b/crates/ra_ide_db/src/change.rs index 7e9310005..de5c6eb8f 100644 --- a/crates/ra_ide_db/src/change.rs +++ b/crates/ra_ide_db/src/change.rs @@ -362,7 +362,6 @@ impl RootDatabase { hir::db::GenericDefaultsQuery hir::db::ImplsInCrateQuery hir::db::ImplsForTraitQuery - hir::db::TraitSolverQuery hir::db::InternTypeCtorQuery hir::db::InternChalkImplQuery hir::db::InternAssocTyValueQuery -- cgit v1.2.3