Skip to content

Commit 56cd04a

Browse files
borsbors
committed
Auto merge of #93511 - cjgillot:query-copy, r=oli-obk
Ensure that queries only return Copy types. This should pervent the perf footgun of returning a result with an expensive `Clone` impl (like a `Vec` of a hash map). I went for the stupid solution of allocating on an arena everything that was not `Copy`. Some query results could be made Copy easily, but I did not really investigate.
2 parents 5d6ee0d + 8edd32c commit 56cd04a

File tree

27 files changed

+208
-182
lines changed

27 files changed

+208
-182
lines changed

Diff for: ‎compiler/rustc_attr/src/builtin.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -604,7 +604,7 @@ pub fn eval_condition(
604604
}
605605
}
606606

607-
#[derive(Debug, Encodable, Decodable, Clone, HashStable_Generic)]
607+
#[derive(Copy, Debug, Encodable, Decodable, Clone, HashStable_Generic)]
608608
pub struct Deprecation {
609609
pub since: Option<Symbol>,
610610
/// The note to issue a reason.

Diff for: ‎compiler/rustc_codegen_ssa/src/back/write.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ pub fn start_async_codegen<B: ExtraBackendMethods>(
477477
codegen_worker_receive,
478478
shared_emitter_main,
479479
future: coordinator_thread,
480-
output_filenames: tcx.output_filenames(()),
480+
output_filenames: tcx.output_filenames(()).clone(),
481481
}
482482
}
483483

@@ -1050,7 +1050,7 @@ fn start_executing_work<B: ExtraBackendMethods>(
10501050
cgu_reuse_tracker: sess.cgu_reuse_tracker.clone(),
10511051
coordinator_send,
10521052
diag_emitter: shared_emitter.clone(),
1053-
output_filenames: tcx.output_filenames(()),
1053+
output_filenames: tcx.output_filenames(()).clone(),
10541054
regular_module_config: regular_config,
10551055
metadata_module_config: metadata_config,
10561056
allocator_module_config: allocator_config,

Diff for: ‎compiler/rustc_codegen_ssa/src/base.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -843,7 +843,7 @@ impl CrateInfo {
843843
used_crate_source: Default::default(),
844844
lang_item_to_crate: Default::default(),
845845
missing_lang_items: Default::default(),
846-
dependency_formats: tcx.dependency_formats(()),
846+
dependency_formats: tcx.dependency_formats(()).clone(),
847847
windows_subsystem,
848848
};
849849
let lang_items = tcx.lang_items();
@@ -860,7 +860,7 @@ impl CrateInfo {
860860
info.native_libraries
861861
.insert(cnum, tcx.native_libraries(cnum).iter().map(Into::into).collect());
862862
info.crate_name.insert(cnum, tcx.crate_name(cnum).to_string());
863-
info.used_crate_source.insert(cnum, tcx.used_crate_source(cnum));
863+
info.used_crate_source.insert(cnum, tcx.used_crate_source(cnum).clone());
864864
if tcx.is_compiler_builtins(cnum) {
865865
info.compiler_builtins = Some(cnum);
866866
}

Diff for: ‎compiler/rustc_interface/src/passes.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -658,13 +658,13 @@ fn write_out_deps(
658658
boxed_resolver.borrow_mut().access(|resolver| {
659659
for cnum in resolver.cstore().crates_untracked() {
660660
let source = resolver.cstore().crate_source_untracked(cnum);
661-
if let Some((path, _)) = source.dylib {
661+
if let Some((path, _)) = &source.dylib {
662662
files.push(escape_dep_filename(&path.display().to_string()));
663663
}
664-
if let Some((path, _)) = source.rlib {
664+
if let Some((path, _)) = &source.rlib {
665665
files.push(escape_dep_filename(&path.display().to_string()));
666666
}
667-
if let Some((path, _)) = source.rmeta {
667+
if let Some((path, _)) = &source.rmeta {
668668
files.push(escape_dep_filename(&path.display().to_string()));
669669
}
670670
}

Diff for: ‎compiler/rustc_metadata/src/rmeta/decoder.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ crate struct CrateMetadata {
120120
/// How to link (or not link) this crate to the currently compiled crate.
121121
dep_kind: Lock<CrateDepKind>,
122122
/// Filesystem location of this crate.
123-
source: CrateSource,
123+
source: Lrc<CrateSource>,
124124
/// Whether or not this crate should be consider a private dependency
125125
/// for purposes of the 'exported_private_dependencies' lint
126126
private_dep: bool,
@@ -1875,7 +1875,7 @@ impl CrateMetadata {
18751875
cnum_map,
18761876
dependencies,
18771877
dep_kind: Lock::new(dep_kind),
1878-
source,
1878+
source: Lrc::new(source),
18791879
private_dep,
18801880
host_hash,
18811881
extern_crate: Lock::new(None),
@@ -1903,7 +1903,7 @@ impl CrateMetadata {
19031903
}
19041904

19051905
crate fn source(&self) -> &CrateSource {
1906-
&self.source
1906+
&*self.source
19071907
}
19081908

19091909
crate fn dep_kind(&self) -> CrateDepKind {

Diff for: ‎compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

+8-13
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use crate::foreign_modules;
33
use crate::native_libs;
44

55
use rustc_ast as ast;
6-
use rustc_data_structures::stable_map::FxHashMap;
76
use rustc_hir::def::{CtorKind, DefKind, Res};
87
use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, CRATE_DEF_INDEX, LOCAL_CRATE};
98
use rustc_hir::definitions::{DefKey, DefPath, DefPathHash};
@@ -13,7 +12,7 @@ use rustc_middle::middle::stability::DeprecationEntry;
1312
use rustc_middle::ty::fast_reject::SimplifiedType;
1413
use rustc_middle::ty::query::{ExternProviders, Providers};
1514
use rustc_middle::ty::{self, TyCtxt, Visibility};
16-
use rustc_session::cstore::{CrateSource, CrateStore, ForeignModule};
15+
use rustc_session::cstore::{CrateSource, CrateStore};
1716
use rustc_session::utils::NativeLibKind;
1817
use rustc_session::{Session, StableCrateId};
1918
use rustc_span::hygiene::{ExpnHash, ExpnId};
@@ -179,10 +178,8 @@ provide! { <'tcx> tcx, def_id, other, cdata,
179178

180179
reachable_non_generics
181180
}
182-
native_libraries => { Lrc::new(cdata.get_native_libraries(tcx.sess).collect()) }
183-
foreign_modules => {
184-
Lrc::new(cdata.get_foreign_modules(tcx.sess).map(|m| (m.def_id, m)).collect())
185-
}
181+
native_libraries => { cdata.get_native_libraries(tcx.sess).collect() }
182+
foreign_modules => { cdata.get_foreign_modules(tcx.sess).map(|m| (m.def_id, m)).collect() }
186183
crate_hash => { cdata.root.hash }
187184
crate_host_hash => { cdata.host_hash }
188185
crate_name => { cdata.root.name }
@@ -212,7 +209,7 @@ provide! { <'tcx> tcx, def_id, other, cdata,
212209
r
213210
}
214211

215-
used_crate_source => { Lrc::new(cdata.source.clone()) }
212+
used_crate_source => { Lrc::clone(&cdata.source) }
216213

217214
exported_symbols => {
218215
let syms = cdata.exported_symbols(tcx);
@@ -266,13 +263,11 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) {
266263
},
267264
native_libraries: |tcx, cnum| {
268265
assert_eq!(cnum, LOCAL_CRATE);
269-
Lrc::new(native_libs::collect(tcx))
266+
native_libs::collect(tcx)
270267
},
271268
foreign_modules: |tcx, cnum| {
272269
assert_eq!(cnum, LOCAL_CRATE);
273-
let modules: FxHashMap<DefId, ForeignModule> =
274-
foreign_modules::collect(tcx).into_iter().map(|m| (m.def_id, m)).collect();
275-
Lrc::new(modules)
270+
foreign_modules::collect(tcx).into_iter().map(|m| (m.def_id, m)).collect()
276271
},
277272

278273
// Returns a map from a sufficiently visible external item (i.e., an
@@ -354,7 +349,7 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) {
354349
visible_parent_map.entry(child).or_insert(parent);
355350
}
356351

357-
Lrc::new(visible_parent_map)
352+
visible_parent_map
358353
},
359354

360355
dependency_formats: |tcx, ()| Lrc::new(crate::dependency_format::calculate(tcx)),
@@ -438,7 +433,7 @@ impl CStore {
438433
self.get_crate_data(def.krate).get_fn_has_self_parameter(def.index)
439434
}
440435

441-
pub fn crate_source_untracked(&self, cnum: CrateNum) -> CrateSource {
436+
pub fn crate_source_untracked(&self, cnum: CrateNum) -> Lrc<CrateSource> {
442437
self.get_crate_data(cnum).source.clone()
443438
}
444439

Diff for: ‎compiler/rustc_metadata/src/rmeta/encoder.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1742,7 +1742,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
17421742
hash: self.tcx.crate_hash(cnum),
17431743
host_hash: self.tcx.crate_host_hash(cnum),
17441744
kind: self.tcx.dep_kind(cnum),
1745-
extra_filename: self.tcx.extra_filename(cnum),
1745+
extra_filename: self.tcx.extra_filename(cnum).clone(),
17461746
};
17471747
(cnum, dep)
17481748
})

Diff for: ‎compiler/rustc_middle/src/arena.rs

+4
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ macro_rules! arena_types {
5252
Vec<rustc_middle::traits::query::OutlivesBound<'tcx>>
5353
>
5454
>,
55+
[] dtorck_constraint: rustc_middle::traits::query::DtorckConstraint<'tcx>,
56+
[] candidate_step: rustc_middle::traits::query::CandidateStep<'tcx>,
57+
[] autoderef_bad_ty: rustc_middle::traits::query::MethodAutoderefBadTy<'tcx>,
5558
[] type_op_subtype:
5659
rustc_middle::infer::canonical::Canonical<'tcx,
5760
rustc_middle::infer::canonical::QueryResponse<'tcx, ()>
@@ -95,6 +98,7 @@ macro_rules! arena_types {
9598
// This is used to decode the &'tcx [Span] for InlineAsm's line_spans.
9699
[decode] span: rustc_span::Span,
97100
[decode] used_trait_imports: rustc_data_structures::fx::FxHashSet<rustc_hir::def_id::LocalDefId>,
101+
[decode] impl_source: rustc_middle::traits::ImplSource<'tcx, ()>,
98102

99103
[] dep_kind: rustc_middle::dep_graph::DepKindStruct,
100104
]);

Diff for: ‎compiler/rustc_middle/src/middle/stability.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ pub enum StabilityLevel {
2929
}
3030

3131
/// An entry in the `depr_map`.
32-
#[derive(Clone, HashStable, Debug)]
32+
#[derive(Copy, Clone, HashStable, Debug)]
3333
pub struct DeprecationEntry {
3434
/// The metadata of the attribute associated with this entry.
3535
pub attr: Deprecation,

Diff for: ‎compiler/rustc_middle/src/query/mod.rs

+33-16
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,8 @@ rustc_queries! {
215215
desc { |tcx| "elaborating item bounds for `{}`", tcx.def_path_str(key) }
216216
}
217217

218-
query native_libraries(_: CrateNum) -> Lrc<Vec<NativeLib>> {
218+
query native_libraries(_: CrateNum) -> Vec<NativeLib> {
219+
storage(ArenaCacheSelector<'tcx>)
219220
desc { "looking up the native libraries of a linked crate" }
220221
separate_provide_extern
221222
}
@@ -254,6 +255,7 @@ rustc_queries! {
254255
/// Create a THIR tree for debugging.
255256
query thir_tree(key: ty::WithOptConstParam<LocalDefId>) -> String {
256257
no_hash
258+
storage(ArenaCacheSelector<'tcx>)
257259
desc { |tcx| "constructing THIR tree for `{}`", tcx.def_path_str(key.did.to_def_id()) }
258260
}
259261

@@ -368,6 +370,7 @@ rustc_queries! {
368370
query symbols_for_closure_captures(
369371
key: (LocalDefId, DefId)
370372
) -> Vec<rustc_span::Symbol> {
373+
storage(ArenaCacheSelector<'tcx>)
371374
desc {
372375
|tcx| "symbols for captures of closure `{}` in `{}`",
373376
tcx.def_path_str(key.1),
@@ -538,7 +541,7 @@ rustc_queries! {
538541

539542
query adt_dtorck_constraint(
540543
key: DefId
541-
) -> Result<DtorckConstraint<'tcx>, NoSolution> {
544+
) -> Result<&'tcx DtorckConstraint<'tcx>, NoSolution> {
542545
desc { |tcx| "computing drop-check constraints for `{}`", tcx.def_path_str(key) }
543546
}
544547

@@ -646,8 +649,8 @@ rustc_queries! {
646649
/// The map returned for `tcx.impl_item_implementor_ids(impl_id)` would be
647650
///`{ trait_f: impl_f, trait_g: impl_g }`
648651
query impl_item_implementor_ids(impl_id: DefId) -> FxHashMap<DefId, DefId> {
649-
desc { |tcx| "comparing impl items against trait for {}", tcx.def_path_str(impl_id) }
650652
storage(ArenaCacheSelector<'tcx>)
653+
desc { |tcx| "comparing impl items against trait for {}", tcx.def_path_str(impl_id) }
651654
}
652655

653656
/// Given an `impl_id`, return the trait it implements.
@@ -1042,6 +1045,7 @@ rustc_queries! {
10421045
/// Gets the rendered value of the specified constant or associated constant.
10431046
/// Used by rustdoc.
10441047
query rendered_const(def_id: DefId) -> String {
1048+
storage(ArenaCacheSelector<'tcx>)
10451049
desc { |tcx| "rendering constant intializer of `{}`", tcx.def_path_str(def_id) }
10461050
separate_provide_extern
10471051
}
@@ -1091,7 +1095,7 @@ rustc_queries! {
10911095

10921096
query codegen_fulfill_obligation(
10931097
key: (ty::ParamEnv<'tcx>, ty::PolyTraitRef<'tcx>)
1094-
) -> Result<ImplSource<'tcx, ()>, ErrorReported> {
1098+
) -> Result<&'tcx ImplSource<'tcx, ()>, ErrorReported> {
10951099
cache_on_disk_if { true }
10961100
desc { |tcx|
10971101
"checking if `{}` fulfills its obligations",
@@ -1237,6 +1241,7 @@ rustc_queries! {
12371241
}
12381242

12391243
query dependency_formats(_: ()) -> Lrc<crate::middle::dependency_format::Dependencies> {
1244+
storage(ArenaCacheSelector<'tcx>)
12401245
desc { "get the linkage format of all dependencies" }
12411246
}
12421247

@@ -1369,13 +1374,15 @@ rustc_queries! {
13691374
/// You likely want to call `Instance::upstream_monomorphization()`
13701375
/// instead of invoking this query directly.
13711376
query upstream_monomorphizations_for(def_id: DefId)
1372-
-> Option<&'tcx FxHashMap<SubstsRef<'tcx>, CrateNum>> {
1373-
desc { |tcx|
1374-
"collecting available upstream monomorphizations for `{}`",
1375-
tcx.def_path_str(def_id),
1376-
}
1377-
separate_provide_extern
1377+
-> Option<&'tcx FxHashMap<SubstsRef<'tcx>, CrateNum>>
1378+
{
1379+
storage(ArenaCacheSelector<'tcx>)
1380+
desc { |tcx|
1381+
"collecting available upstream monomorphizations for `{}`",
1382+
tcx.def_path_str(def_id),
13781383
}
1384+
separate_provide_extern
1385+
}
13791386

13801387
/// Returns the upstream crate that exports drop-glue for the given
13811388
/// type (`substs` is expected to be a single-item list containing the
@@ -1396,7 +1403,8 @@ rustc_queries! {
13961403
desc { "available upstream drop-glue for `{:?}`", substs }
13971404
}
13981405

1399-
query foreign_modules(_: CrateNum) -> Lrc<FxHashMap<DefId, ForeignModule>> {
1406+
query foreign_modules(_: CrateNum) -> FxHashMap<DefId, ForeignModule> {
1407+
storage(ArenaCacheSelector<'tcx>)
14001408
desc { "looking up the foreign modules of a linked crate" }
14011409
separate_provide_extern
14021410
}
@@ -1422,11 +1430,13 @@ rustc_queries! {
14221430
separate_provide_extern
14231431
}
14241432
query extra_filename(_: CrateNum) -> String {
1433+
storage(ArenaCacheSelector<'tcx>)
14251434
eval_always
14261435
desc { "looking up the extra filename for a crate" }
14271436
separate_provide_extern
14281437
}
14291438
query crate_extern_paths(_: CrateNum) -> Vec<PathBuf> {
1439+
storage(ArenaCacheSelector<'tcx>)
14301440
eval_always
14311441
desc { "looking up the paths for extern crates" }
14321442
separate_provide_extern
@@ -1478,8 +1488,7 @@ rustc_queries! {
14781488
/// for each parameter if a trait object were to be passed for that parameter.
14791489
/// For example, for `struct Foo<'a, T, U>`, this would be `['static, 'static]`.
14801490
/// For `struct Foo<'a, T: 'a, U>`, this would instead be `['a, 'static]`.
1481-
query object_lifetime_defaults_map(_: LocalDefId)
1482-
-> Option<Vec<ObjectLifetimeDefault>> {
1491+
query object_lifetime_defaults(_: LocalDefId) -> Option<&'tcx [ObjectLifetimeDefault]> {
14831492
desc { "looking up lifetime defaults for a region on an item" }
14841493
}
14851494
query late_bound_vars_map(_: LocalDefId)
@@ -1488,6 +1497,7 @@ rustc_queries! {
14881497
}
14891498

14901499
query lifetime_scope_map(_: LocalDefId) -> Option<FxHashMap<ItemLocalId, LifetimeScopeForPath>> {
1500+
storage(ArenaCacheSelector<'tcx>)
14911501
desc { "finds the lifetime scope for an HirId of a PathSegment" }
14921502
}
14931503

@@ -1501,7 +1511,7 @@ rustc_queries! {
15011511
/// check whether the forest is empty.
15021512
query type_uninhabited_from(
15031513
key: ty::ParamEnvAnd<'tcx, Ty<'tcx>>
1504-
) -> ty::inhabitedness::DefIdForest {
1514+
) -> ty::inhabitedness::DefIdForest<'tcx> {
15051515
desc { "computing the inhabitedness of `{:?}`", key }
15061516
remap_env_constness
15071517
}
@@ -1566,7 +1576,8 @@ rustc_queries! {
15661576
desc { "calculating the missing lang items in a crate" }
15671577
separate_provide_extern
15681578
}
1569-
query visible_parent_map(_: ()) -> Lrc<DefIdMap<DefId>> {
1579+
query visible_parent_map(_: ()) -> DefIdMap<DefId> {
1580+
storage(ArenaCacheSelector<'tcx>)
15701581
desc { "calculating the visible parent map" }
15711582
}
15721583
query trimmed_def_paths(_: ()) -> FxHashMap<DefId, Symbol> {
@@ -1579,6 +1590,7 @@ rustc_queries! {
15791590
separate_provide_extern
15801591
}
15811592
query used_crate_source(_: CrateNum) -> Lrc<CrateSource> {
1593+
storage(ArenaCacheSelector<'tcx>)
15821594
eval_always
15831595
desc { "looking at the source for a crate" }
15841596
separate_provide_extern
@@ -1669,7 +1681,11 @@ rustc_queries! {
16691681
desc { "optimization level used by backend" }
16701682
}
16711683

1672-
query output_filenames(_: ()) -> Arc<OutputFilenames> {
1684+
/// Return the filenames where output artefacts shall be stored.
1685+
///
1686+
/// This query returns an `&Arc` because codegen backends need the value even after the `TyCtxt`
1687+
/// has been destroyed.
1688+
query output_filenames(_: ()) -> &'tcx Arc<OutputFilenames> {
16731689
eval_always
16741690
desc { "output_filenames" }
16751691
}
@@ -1911,6 +1927,7 @@ rustc_queries! {
19111927
/// all of the cases that the normal `ty::Ty`-based wfcheck does. This is fine,
19121928
/// because the `ty::Ty`-based wfcheck is always run.
19131929
query diagnostic_hir_wf_check(key: (ty::Predicate<'tcx>, traits::WellFormedLoc)) -> Option<traits::ObligationCause<'tcx>> {
1930+
storage(ArenaCacheSelector<'tcx>)
19141931
eval_always
19151932
no_hash
19161933
desc { "performing HIR wf-checking for predicate {:?} at item {:?}", key.0, key.1 }

0 commit comments

Comments
 (0)