Skip to content

Commit

Permalink
Auto merge of rust-lang#90845 - JakobDegen:adt-drop-perf, r=Mark-Simu…
Browse files Browse the repository at this point in the history
…lacrum

Address performance regression introduced by rust-lang#90218

As part of the changes in rust-lang#90218 , the `adt_drop_tys` and friends code stopped recursing through the query system, meaning that intermediate computations did not get cached. This change adds the recursions back in without re-introducing any of the old issues.

On local benchmarks this fixes the 5% regressions in rust-lang#90504 ; the wg-grammar regressions didn't seem to move too much. I may take some time later to look into those.

Not sure who to request for review here, so will leave it up to whoever gets it.
  • Loading branch information
bors committed Nov 16, 2021
2 parents 0206312 + 746091c commit a2a7683
Showing 1 changed file with 61 additions and 23 deletions.
84 changes: 61 additions & 23 deletions compiler/rustc_ty_utils/src/needs_drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>
// needs drop.
let adt_has_dtor =
|adt_def: &ty::AdtDef| adt_def.destructor(tcx).map(|_| DtorType::Significant);
let res = drop_tys_helper(tcx, query.value, query.param_env, adt_has_dtor).next().is_some();
let res =
drop_tys_helper(tcx, query.value, query.param_env, adt_has_dtor, false).next().is_some();

debug!("needs_drop_raw({:?}) = {:?}", query, res);
res
Expand All @@ -27,10 +28,15 @@ fn has_significant_drop_raw<'tcx>(
tcx: TyCtxt<'tcx>,
query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
) -> bool {
let res =
drop_tys_helper(tcx, query.value, query.param_env, adt_consider_insignificant_dtor(tcx))
.next()
.is_some();
let res = drop_tys_helper(
tcx,
query.value,
query.param_env,
adt_consider_insignificant_dtor(tcx),
true,
)
.next()
.is_some();
debug!("has_significant_drop_raw({:?}) = {:?}", query, res);
res
}
Expand Down Expand Up @@ -141,9 +147,9 @@ where
Ok(tys) => tys,
};
for required_ty in tys {
let subst_ty =
let required =
tcx.normalize_erasing_regions(self.param_env, required_ty);
queue_type(self, subst_ty);
queue_type(self, required);
}
}
ty::Array(..) | ty::Opaque(..) | ty::Projection(..) | ty::Param(_) => {
Expand Down Expand Up @@ -186,39 +192,67 @@ fn drop_tys_helper<'tcx>(
ty: Ty<'tcx>,
param_env: rustc_middle::ty::ParamEnv<'tcx>,
adt_has_dtor: impl Fn(&ty::AdtDef) -> Option<DtorType>,
only_significant: bool,
) -> impl Iterator<Item = NeedsDropResult<Ty<'tcx>>> {
fn with_query_cache<'tcx>(
tcx: TyCtxt<'tcx>,
iter: impl IntoIterator<Item = Ty<'tcx>>,
only_significant: bool,
) -> NeedsDropResult<Vec<Ty<'tcx>>> {
iter.into_iter().try_fold(Vec::new(), |mut vec, subty| {
match subty.kind() {
ty::Adt(adt_id, subst) => {
for subty in if only_significant {
tcx.adt_significant_drop_tys(adt_id.did)?
} else {
tcx.adt_drop_tys(adt_id.did)?
} {
vec.push(subty.subst(tcx, subst));
}
}
_ => vec.push(subty),
};
Ok(vec)
})
}

let adt_components = move |adt_def: &ty::AdtDef, substs: SubstsRef<'tcx>| {
if adt_def.is_manually_drop() {
debug!("drop_tys_helper: `{:?}` is manually drop", adt_def);
return Ok(Vec::new().into_iter());
Ok(Vec::new())
} else if let Some(dtor_info) = adt_has_dtor(adt_def) {
match dtor_info {
DtorType::Significant => {
debug!("drop_tys_helper: `{:?}` implements `Drop`", adt_def);
return Err(AlwaysRequiresDrop);
Err(AlwaysRequiresDrop)
}
DtorType::Insignificant => {
debug!("drop_tys_helper: `{:?}` drop is insignificant", adt_def);

// Since the destructor is insignificant, we just want to make sure all of
// the passed in type parameters are also insignificant.
// Eg: Vec<T> dtor is insignificant when T=i32 but significant when T=Mutex.
return Ok(substs.types().collect::<Vec<Ty<'_>>>().into_iter());
with_query_cache(tcx, substs.types(), only_significant)
}
}
} else if adt_def.is_union() {
debug!("drop_tys_helper: `{:?}` is a union", adt_def);
return Ok(Vec::new().into_iter());
Ok(Vec::new())
} else {
with_query_cache(
tcx,
adt_def.all_fields().map(|field| {
let r = tcx.type_of(field.did).subst(tcx, substs);
debug!(
"drop_tys_helper: Subst into {:?} with {:?} gettng {:?}",
field, substs, r
);
r
}),
only_significant,
)
}
Ok(adt_def
.all_fields()
.map(|field| {
let r = tcx.type_of(field.did).subst(tcx, substs);
debug!("drop_tys_helper: Subst into {:?} with {:?} gettng {:?}", field, substs, r);
r
})
.collect::<Vec<_>>()
.into_iter())
.map(|v| v.into_iter())
};

NeedsDropTypes::new(tcx, param_env, ty, adt_components)
Expand Down Expand Up @@ -252,20 +286,24 @@ fn adt_drop_tys(tcx: TyCtxt<'_>, def_id: DefId) -> Result<&ty::List<Ty<'_>>, Alw
// significant.
let adt_has_dtor =
|adt_def: &ty::AdtDef| adt_def.destructor(tcx).map(|_| DtorType::Significant);
drop_tys_helper(tcx, tcx.type_of(def_id), tcx.param_env(def_id), adt_has_dtor)
// `tcx.type_of(def_id)` identical to `tcx.make_adt(def, identity_substs)`
drop_tys_helper(tcx, tcx.type_of(def_id), tcx.param_env(def_id), adt_has_dtor, false)
.collect::<Result<Vec<_>, _>>()
.map(|components| tcx.intern_type_list(&components))
}

// If `def_id` refers to a generic ADT, the queries above and below act as if they had been handed
// a `tcx.make_ty(def, identity_substs)` and as such it is legal to substitue the generic parameters
// of the ADT into the outputted `ty`s.
fn adt_significant_drop_tys(
tcx: TyCtxt<'_>,
def_id: DefId,
) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
drop_tys_helper(
tcx,
tcx.type_of(def_id),
tcx.type_of(def_id), // identical to `tcx.make_adt(def, identity_substs)`
tcx.param_env(def_id),
adt_consider_insignificant_dtor(tcx),
true,
)
.collect::<Result<Vec<_>, _>>()
.map(|components| tcx.intern_type_list(&components))
Expand Down

0 comments on commit a2a7683

Please sign in to comment.