From 65b9e5124a1dde7e451ff60859b7ae8837c014f4 Mon Sep 17 00:00:00 2001 From: Timothy Maloney Date: Sun, 12 Sep 2021 13:39:18 -0700 Subject: [PATCH 01/11] Feat: added caching schema for debuginfo type names --- .../src/debuginfo/metadata.rs | 61 ++++++++++++++++--- 1 file changed, 51 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 9272435a330a5..6bc77c32179ba 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -152,6 +152,8 @@ pub struct TypeMap<'ll, 'tcx> { type_to_metadata: FxHashMap, &'ll DIType>, /// A map from types to `UniqueTypeId`. This is an N:1 mapping. type_to_unique_id: FxHashMap, UniqueTypeId>, + /// A map from `UniqueTypeId` to it's type-name string. + unique_id_to_type_name: FxHashMap, } impl TypeMap<'ll, 'tcx> { @@ -201,6 +203,21 @@ impl TypeMap<'ll, 'tcx> { } } + /// Adds a `UniqueTypeId` to type-name mapping to the `TypeMap`. The + /// method will fail if the mapping already exists. + fn register_unique_id_with_type_name( + &mut self, + unique_type_id: UniqueTypeId, + type_name: String + ) { + if self.unique_id_to_type_name.insert(unique_type_id, type_name).is_some() { + bug!( + "type name for unique ID '{}' is already in the `TypeMap`!", + self.get_unique_type_id_as_string(unique_type_id) + ); + } + } + fn find_metadata_for_type(&self, type_: Ty<'tcx>) -> Option<&'ll DIType> { self.type_to_metadata.get(&type_).cloned() } @@ -209,6 +226,10 @@ impl TypeMap<'ll, 'tcx> { self.unique_id_to_metadata.get(&unique_type_id).cloned() } + fn find_type_name_for_unique_id(&self, unique_type_id: UniqueTypeId) -> Option { + self.unique_id_to_type_name.get(&unique_type_id).cloned() + } + /// Gets the string representation of a `UniqueTypeId`. This method will fail if /// the ID is unknown. fn get_unique_type_id_as_string(&self, unique_type_id: UniqueTypeId) -> &str { @@ -374,6 +395,26 @@ macro_rules! return_if_metadata_created_in_meantime { }; } +fn check_type_name_cache( + cx: &CodegenCx<'ll, 'tcx>, + ty: Ty<'tcx>, + qualified: bool, +) -> String { + let mut type_map = debug_context(cx).type_map.borrow_mut(); + let unique_type_id = type_map.get_unique_type_id_of_type(cx, ty); + match type_map.find_type_name_for_unique_id(unique_type_id) { + Some(type_name) => { type_name }, + None => { + let type_name = compute_debuginfo_type_name(cx.tcx, ty, qualified); + type_map.register_unique_id_with_type_name( + unique_type_id, + type_name.clone(), + ); + type_name + }, + } +} + fn fixed_vec_metadata( cx: &CodegenCx<'ll, 'tcx>, unique_type_id: UniqueTypeId, @@ -422,7 +463,7 @@ fn vec_slice_metadata( return_if_metadata_created_in_meantime!(cx, unique_type_id); - let slice_type_name = compute_debuginfo_type_name(cx.tcx, slice_ptr_type, true); + let slice_type_name = check_type_name_cache(cx, slice_ptr_type, true); let (pointer_size, pointer_align) = cx.size_and_align_of(data_ptr_type); let (usize_size, usize_align) = cx.size_and_align_of(cx.tcx.types.usize); @@ -520,10 +561,10 @@ fn trait_pointer_metadata( Some(trait_object_type) => match trait_object_type.kind() { ty::Adt(def, _) => ( Some(get_namespace_for_item(cx, def.did)), - compute_debuginfo_type_name(cx.tcx, trait_object_type, false), + check_type_name_cache(cx, trait_object_type, false), ), ty::RawPtr(_) | ty::Ref(..) => { - (NO_SCOPE_METADATA, compute_debuginfo_type_name(cx.tcx, trait_object_type, true)) + (NO_SCOPE_METADATA, check_type_name_cache(cx, trait_object_type, true)) } _ => { bug!( @@ -536,7 +577,7 @@ fn trait_pointer_metadata( // No object type, use the trait type directly (no scope here since the type // will be wrapped in the dyn$ synthetic type). - None => (NO_SCOPE_METADATA, compute_debuginfo_type_name(cx.tcx, trait_type, true)), + None => (NO_SCOPE_METADATA, check_type_name_cache(cx, trait_type, true)), }; let file_metadata = unknown_file_metadata(cx); @@ -987,7 +1028,7 @@ fn foreign_type_metadata( ) -> &'ll DIType { debug!("foreign_type_metadata: {:?}", t); - let name = compute_debuginfo_type_name(cx.tcx, t, false); + let name = check_type_name_cache(cx, t, false); create_struct_stub(cx, t, &name, unique_type_id, NO_SCOPE_METADATA, DIFlags::FlagZero) } @@ -997,7 +1038,7 @@ fn pointer_type_metadata( pointee_type_metadata: &'ll DIType, ) -> &'ll DIType { let (pointer_size, pointer_align) = cx.size_and_align_of(pointer_type); - let name = compute_debuginfo_type_name(cx.tcx, pointer_type, false); + let name = check_type_name_cache(cx, pointer_type, false); unsafe { llvm::LLVMRustDIBuilderCreatePointerType( DIB(cx), @@ -1300,7 +1341,7 @@ fn prepare_struct_metadata( unique_type_id: UniqueTypeId, span: Span, ) -> RecursiveTypeDescription<'ll, 'tcx> { - let struct_name = compute_debuginfo_type_name(cx.tcx, struct_type, false); + let struct_name = check_type_name_cache(cx, struct_type, false); let (struct_def_id, variant) = match struct_type.kind() { ty::Adt(def, _) => (def.did, def.non_enum_variant()), @@ -1406,7 +1447,7 @@ fn prepare_tuple_metadata( span: Span, containing_scope: Option<&'ll DIScope>, ) -> RecursiveTypeDescription<'ll, 'tcx> { - let tuple_name = compute_debuginfo_type_name(cx.tcx, tuple_type, false); + let tuple_name = check_type_name_cache(cx, tuple_type, false); let struct_stub = create_struct_stub( cx, @@ -1470,7 +1511,7 @@ fn prepare_union_metadata( unique_type_id: UniqueTypeId, span: Span, ) -> RecursiveTypeDescription<'ll, 'tcx> { - let union_name = compute_debuginfo_type_name(cx.tcx, union_type, false); + let union_name = check_type_name_cache(cx, union_type, false); let (union_def_id, variant) = match union_type.kind() { ty::Adt(def, _) => (def.did, def.non_enum_variant()), @@ -2025,7 +2066,7 @@ fn prepare_enum_metadata( outer_field_tys: Vec>, ) -> RecursiveTypeDescription<'ll, 'tcx> { let tcx = cx.tcx; - let enum_name = compute_debuginfo_type_name(tcx, enum_type, false); + let enum_name = check_type_name_cache(cx, enum_type, false); let containing_scope = get_namespace_for_item(cx, enum_def_id); // FIXME: This should emit actual file metadata for the enum, but we From 490b2683d1c25710f8f067aa6889904c22d6e076 Mon Sep 17 00:00:00 2001 From: Timothy Maloney Date: Sun, 12 Sep 2021 14:49:47 -0700 Subject: [PATCH 02/11] Style: pleased almighty tidy --- .../src/debuginfo/metadata.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 6bc77c32179ba..cab753ae970f3 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -208,7 +208,7 @@ impl TypeMap<'ll, 'tcx> { fn register_unique_id_with_type_name( &mut self, unique_type_id: UniqueTypeId, - type_name: String + type_name: String, ) { if self.unique_id_to_type_name.insert(unique_type_id, type_name).is_some() { bug!( @@ -395,23 +395,16 @@ macro_rules! return_if_metadata_created_in_meantime { }; } -fn check_type_name_cache( - cx: &CodegenCx<'ll, 'tcx>, - ty: Ty<'tcx>, - qualified: bool, -) -> String { +fn check_type_name_cache(cx: &CodegenCx<'ll, 'tcx>, ty: Ty<'tcx>, qualified: bool) -> String { let mut type_map = debug_context(cx).type_map.borrow_mut(); let unique_type_id = type_map.get_unique_type_id_of_type(cx, ty); match type_map.find_type_name_for_unique_id(unique_type_id) { - Some(type_name) => { type_name }, + Some(type_name) => type_name, None => { let type_name = compute_debuginfo_type_name(cx.tcx, ty, qualified); - type_map.register_unique_id_with_type_name( - unique_type_id, - type_name.clone(), - ); + type_map.register_unique_id_with_type_name(unique_type_id, type_name.clone()); type_name - }, + } } } From 2e01bf1da9501980e641a141e6891b0c3d7948a6 Mon Sep 17 00:00:00 2001 From: Timothy Maloney Date: Sat, 18 Sep 2021 06:35:17 -0700 Subject: [PATCH 03/11] Moved cache to CrateDebugContext --- compiler/rustc_codegen_llvm/src/debuginfo/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index fbaf8c8bdf63d..3810496aa04e1 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -15,6 +15,7 @@ use crate::llvm::debuginfo::{ DIArray, DIBuilder, DIFile, DIFlags, DILexicalBlock, DILocation, DISPFlags, DIScope, DIType, DIVariable, }; +use crate::debuginfo::utils::debug_context; use crate::value::Value; use rustc_codegen_ssa::debuginfo::type_names; @@ -62,6 +63,7 @@ pub struct CrateDebugContext<'a, 'tcx> { builder: &'a mut DIBuilder<'a>, created_files: RefCell, Option), &'a DIFile>>, created_enum_disr_types: RefCell>, + type_name_cache: RefCell, bool), String>>, type_map: RefCell>, namespace_map: RefCell>, @@ -91,6 +93,7 @@ impl<'a, 'tcx> CrateDebugContext<'a, 'tcx> { builder, created_files: Default::default(), created_enum_disr_types: Default::default(), + type_name_cache: Default::default(), type_map: Default::default(), namespace_map: RefCell::new(Default::default()), composite_types_completed: Default::default(), @@ -435,10 +438,12 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { substs: SubstsRef<'tcx>, name_to_append_suffix_to: &mut String, ) -> &'ll DIArray { + let type_name_cache = &mut *debug_context(cx).type_name_cache.borrow_mut(); type_names::push_generic_params( cx.tcx, cx.tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), substs), name_to_append_suffix_to, + type_name_cache, ); if substs.types().next().is_none() { From 482aedc5a143504dc5635960b063e142cae9746c Mon Sep 17 00:00:00 2001 From: Timothy Maloney Date: Sat, 18 Sep 2021 06:37:23 -0700 Subject: [PATCH 04/11] Added cache argument for compute_debuginfo_type_name --- .../src/debuginfo/metadata.rs | 33 +----- .../src/debuginfo/type_names.rs | 107 ++++++++++++++---- 2 files changed, 88 insertions(+), 52 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index cab753ae970f3..8ec3f1489e2bb 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -152,8 +152,6 @@ pub struct TypeMap<'ll, 'tcx> { type_to_metadata: FxHashMap, &'ll DIType>, /// A map from types to `UniqueTypeId`. This is an N:1 mapping. type_to_unique_id: FxHashMap, UniqueTypeId>, - /// A map from `UniqueTypeId` to it's type-name string. - unique_id_to_type_name: FxHashMap, } impl TypeMap<'ll, 'tcx> { @@ -203,21 +201,6 @@ impl TypeMap<'ll, 'tcx> { } } - /// Adds a `UniqueTypeId` to type-name mapping to the `TypeMap`. The - /// method will fail if the mapping already exists. - fn register_unique_id_with_type_name( - &mut self, - unique_type_id: UniqueTypeId, - type_name: String, - ) { - if self.unique_id_to_type_name.insert(unique_type_id, type_name).is_some() { - bug!( - "type name for unique ID '{}' is already in the `TypeMap`!", - self.get_unique_type_id_as_string(unique_type_id) - ); - } - } - fn find_metadata_for_type(&self, type_: Ty<'tcx>) -> Option<&'ll DIType> { self.type_to_metadata.get(&type_).cloned() } @@ -226,10 +209,6 @@ impl TypeMap<'ll, 'tcx> { self.unique_id_to_metadata.get(&unique_type_id).cloned() } - fn find_type_name_for_unique_id(&self, unique_type_id: UniqueTypeId) -> Option { - self.unique_id_to_type_name.get(&unique_type_id).cloned() - } - /// Gets the string representation of a `UniqueTypeId`. This method will fail if /// the ID is unknown. fn get_unique_type_id_as_string(&self, unique_type_id: UniqueTypeId) -> &str { @@ -396,16 +375,8 @@ macro_rules! return_if_metadata_created_in_meantime { } fn check_type_name_cache(cx: &CodegenCx<'ll, 'tcx>, ty: Ty<'tcx>, qualified: bool) -> String { - let mut type_map = debug_context(cx).type_map.borrow_mut(); - let unique_type_id = type_map.get_unique_type_id_of_type(cx, ty); - match type_map.find_type_name_for_unique_id(unique_type_id) { - Some(type_name) => type_name, - None => { - let type_name = compute_debuginfo_type_name(cx.tcx, ty, qualified); - type_map.register_unique_id_with_type_name(unique_type_id, type_name.clone()); - type_name - } - } + let type_name_cache = &mut *debug_context(cx).type_name_cache.borrow_mut(); + compute_debuginfo_type_name(cx.tcx, ty, qualified, type_name_cache) } fn fixed_vec_metadata( diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index e842f5e9391c8..c0b1b807ac1dd 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -11,7 +11,7 @@ // within the brackets). // * `"` is treated as the start of a string. -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_hir as hir; use rustc_hir::def_id::DefId; @@ -33,12 +33,18 @@ pub fn compute_debuginfo_type_name<'tcx>( tcx: TyCtxt<'tcx>, t: Ty<'tcx>, qualified: bool, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, ) -> String { let _prof = tcx.prof.generic_activity("compute_debuginfo_type_name"); + // Check if we have seen this type and qualifier before. + if let Some(type_name) = type_name_cache.get(&(&t, qualified)) { + return type_name.clone(); + } + let mut result = String::with_capacity(64); let mut visited = FxHashSet::default(); - push_debuginfo_type_name(tcx, t, qualified, &mut result, &mut visited); + push_debuginfo_type_name(tcx, t, qualified, &mut result, &mut visited, type_name_cache); result } @@ -50,6 +56,7 @@ fn push_debuginfo_type_name<'tcx>( qualified: bool, output: &mut String, visited: &mut FxHashSet>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, ) { // When targeting MSVC, emit C++ style type names for compatibility with // .natvis visualizers (and perhaps other existing native debuggers?) @@ -72,10 +79,10 @@ fn push_debuginfo_type_name<'tcx>( ty::Foreign(def_id) => push_item_name(tcx, def_id, qualified, output), ty::Adt(def, substs) => { if def.is_enum() && cpp_like_names { - msvc_enum_fallback(tcx, t, def, substs, output, visited); + msvc_enum_fallback(tcx, t, def, substs, output, visited, type_name_cache); } else { push_item_name(tcx, def.did, qualified, output); - push_generic_params_internal(tcx, substs, output, visited); + push_generic_params_internal(tcx, substs, output, visited, type_name_cache); } } ty::Tuple(component_types) => { @@ -86,7 +93,14 @@ fn push_debuginfo_type_name<'tcx>( } for component_type in component_types { - push_debuginfo_type_name(tcx, component_type.expect_ty(), true, output, visited); + push_debuginfo_type_name( + tcx, + component_type.expect_ty(), + true, + output, + visited, + type_name_cache, + ); push_arg_separator(cpp_like_names, output); } if !component_types.is_empty() { @@ -113,7 +127,7 @@ fn push_debuginfo_type_name<'tcx>( } } - push_debuginfo_type_name(tcx, inner_type, qualified, output, visited); + push_debuginfo_type_name(tcx, inner_type, qualified, output, visited, type_name_cache); if cpp_like_names { push_close_angle_bracket(cpp_like_names, output); @@ -139,7 +153,7 @@ fn push_debuginfo_type_name<'tcx>( } } - push_debuginfo_type_name(tcx, inner_type, qualified, output, visited); + push_debuginfo_type_name(tcx, inner_type, qualified, output, visited, type_name_cache); if cpp_like_names && !is_slice_or_str { push_close_angle_bracket(cpp_like_names, output); @@ -148,7 +162,7 @@ fn push_debuginfo_type_name<'tcx>( ty::Array(inner_type, len) => { if cpp_like_names { output.push_str("array$<"); - push_debuginfo_type_name(tcx, inner_type, true, output, visited); + push_debuginfo_type_name(tcx, inner_type, true, output, visited, type_name_cache); match len.val { ty::ConstKind::Param(param) => write!(output, ",{}>", param.name).unwrap(), _ => write!(output, ",{}>", len.eval_usize(tcx, ty::ParamEnv::reveal_all())) @@ -156,7 +170,7 @@ fn push_debuginfo_type_name<'tcx>( } } else { output.push('['); - push_debuginfo_type_name(tcx, inner_type, true, output, visited); + push_debuginfo_type_name(tcx, inner_type, true, output, visited, type_name_cache); match len.val { ty::ConstKind::Param(param) => write!(output, "; {}]", param.name).unwrap(), _ => write!(output, "; {}]", len.eval_usize(tcx, ty::ParamEnv::reveal_all())) @@ -171,7 +185,7 @@ fn push_debuginfo_type_name<'tcx>( output.push('['); } - push_debuginfo_type_name(tcx, inner_type, true, output, visited); + push_debuginfo_type_name(tcx, inner_type, true, output, visited, type_name_cache); if cpp_like_names { push_close_angle_bracket(cpp_like_names, output); @@ -200,8 +214,13 @@ fn push_debuginfo_type_name<'tcx>( let principal = tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), principal); push_item_name(tcx, principal.def_id, qualified, output); - let principal_has_generic_params = - push_generic_params_internal(tcx, principal.substs, output, visited); + let principal_has_generic_params = push_generic_params_internal( + tcx, + principal.substs, + output, + visited, + type_name_cache, + ); let projection_bounds: SmallVec<[_; 4]> = trait_data .projection_bounds() @@ -225,12 +244,26 @@ fn push_debuginfo_type_name<'tcx>( output.push_str("assoc$<"); push_item_name(tcx, item_def_id, false, output); push_arg_separator(cpp_like_names, output); - push_debuginfo_type_name(tcx, ty, true, output, visited); + push_debuginfo_type_name( + tcx, + ty, + true, + output, + visited, + type_name_cache, + ); push_close_angle_bracket(cpp_like_names, output); } else { push_item_name(tcx, item_def_id, false, output); output.push('='); - push_debuginfo_type_name(tcx, ty, true, output, visited); + push_debuginfo_type_name( + tcx, + ty, + true, + output, + visited, + type_name_cache, + ); } } @@ -298,7 +331,14 @@ fn push_debuginfo_type_name<'tcx>( if sig.output().is_unit() { output.push_str("void"); } else { - push_debuginfo_type_name(tcx, sig.output(), true, output, visited); + push_debuginfo_type_name( + tcx, + sig.output(), + true, + output, + visited, + type_name_cache, + ); } output.push_str(" (*)("); } else { @@ -315,7 +355,14 @@ fn push_debuginfo_type_name<'tcx>( if !sig.inputs().is_empty() { for ¶meter_type in sig.inputs() { - push_debuginfo_type_name(tcx, parameter_type, true, output, visited); + push_debuginfo_type_name( + tcx, + parameter_type, + true, + output, + visited, + type_name_cache, + ); push_arg_separator(cpp_like_names, output); } pop_arg_separator(output); @@ -333,7 +380,7 @@ fn push_debuginfo_type_name<'tcx>( if !cpp_like_names && !sig.output().is_unit() { output.push_str(" -> "); - push_debuginfo_type_name(tcx, sig.output(), true, output, visited); + push_debuginfo_type_name(tcx, sig.output(), true, output, visited, type_name_cache); } // We only keep the type in 'visited' @@ -375,6 +422,10 @@ fn push_debuginfo_type_name<'tcx>( } } + if type_name_cache.insert((&t, qualified), output.clone()).is_some() { + bug!("type name is already in the type name cache!"); + } + /// MSVC names enums differently than other platforms so that the debugging visualization // format (natvis) is able to understand enums and render the active variant correctly in the // debugger. For more information, look in `src/etc/natvis/intrinsic.natvis` and @@ -386,12 +437,13 @@ fn push_debuginfo_type_name<'tcx>( substs: SubstsRef<'tcx>, output: &mut String, visited: &mut FxHashSet>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, ) { let layout = tcx.layout_of(tcx.param_env(def.did).and(ty)).expect("layout error"); output.push_str("enum$<"); push_item_name(tcx, def.did, true, output); - push_generic_params_internal(tcx, substs, output, visited); + push_generic_params_internal(tcx, substs, output, visited, type_name_cache); if let Variants::Multiple { tag_encoding: TagEncoding::Niche { dataful_variant, .. }, @@ -503,6 +555,7 @@ fn push_generic_params_internal<'tcx>( substs: SubstsRef<'tcx>, output: &mut String, visited: &mut FxHashSet>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, ) -> bool { if substs.non_erasable_generics().next().is_none() { return false; @@ -517,7 +570,14 @@ fn push_generic_params_internal<'tcx>( for type_parameter in substs.non_erasable_generics() { match type_parameter { GenericArgKind::Type(type_parameter) => { - push_debuginfo_type_name(tcx, type_parameter, true, output, visited); + push_debuginfo_type_name( + tcx, + type_parameter, + true, + output, + visited, + type_name_cache, + ); } GenericArgKind::Const(ct) => { push_const_param(tcx, ct, output); @@ -578,10 +638,15 @@ fn push_const_param<'tcx>(tcx: TyCtxt<'tcx>, ct: &'tcx ty::Const<'tcx>, output: .unwrap(); } -pub fn push_generic_params<'tcx>(tcx: TyCtxt<'tcx>, substs: SubstsRef<'tcx>, output: &mut String) { +pub fn push_generic_params<'tcx>( + tcx: TyCtxt<'tcx>, + substs: SubstsRef<'tcx>, + output: &mut String, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, +) { let _prof = tcx.prof.generic_activity("compute_debuginfo_type_name"); let mut visited = FxHashSet::default(); - push_generic_params_internal(tcx, substs, output, &mut visited); + push_generic_params_internal(tcx, substs, output, &mut visited, type_name_cache); } fn push_close_angle_bracket(cpp_like_names: bool, output: &mut String) { From 95dd85a13472a0303dd5804f6b33ca57f95c75a4 Mon Sep 17 00:00:00 2001 From: Timothy Maloney Date: Sat, 18 Sep 2021 07:11:47 -0700 Subject: [PATCH 05/11] Moved cache check to push_debuginfo_type_name --- .../rustc_codegen_ssa/src/debuginfo/type_names.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index c0b1b807ac1dd..7ce847ab0361d 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -37,11 +37,6 @@ pub fn compute_debuginfo_type_name<'tcx>( ) -> String { let _prof = tcx.prof.generic_activity("compute_debuginfo_type_name"); - // Check if we have seen this type and qualifier before. - if let Some(type_name) = type_name_cache.get(&(&t, qualified)) { - return type_name.clone(); - } - let mut result = String::with_capacity(64); let mut visited = FxHashSet::default(); push_debuginfo_type_name(tcx, t, qualified, &mut result, &mut visited, type_name_cache); @@ -58,6 +53,12 @@ fn push_debuginfo_type_name<'tcx>( visited: &mut FxHashSet>, type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, ) { + // Check if we have seen this type and qualifier before. + if let Some(type_name) = type_name_cache.get(&(&t, qualified)) { + output.push_str(&type_name.clone()); + return; + } + // When targeting MSVC, emit C++ style type names for compatibility with // .natvis visualizers (and perhaps other existing native debuggers?) let cpp_like_names = cpp_like_names(tcx); From 9425877bdca89d40238704f2fc0a9b72f1301c39 Mon Sep 17 00:00:00 2001 From: Timothy Maloney Date: Sat, 18 Sep 2021 20:27:36 -0700 Subject: [PATCH 06/11] Changed &mut to RefCell --- .../rustc_codegen_llvm/src/debuginfo/metadata.rs | 3 +-- compiler/rustc_codegen_llvm/src/debuginfo/mod.rs | 3 +-- .../rustc_codegen_ssa/src/debuginfo/type_names.rs | 15 ++++++++------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 8ec3f1489e2bb..21b7beb659d94 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -375,8 +375,7 @@ macro_rules! return_if_metadata_created_in_meantime { } fn check_type_name_cache(cx: &CodegenCx<'ll, 'tcx>, ty: Ty<'tcx>, qualified: bool) -> String { - let type_name_cache = &mut *debug_context(cx).type_name_cache.borrow_mut(); - compute_debuginfo_type_name(cx.tcx, ty, qualified, type_name_cache) + compute_debuginfo_type_name(cx.tcx, ty, qualified, &debug_context(cx).type_name_cache) } fn fixed_vec_metadata( diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index 3810496aa04e1..c1f0281777887 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -438,12 +438,11 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { substs: SubstsRef<'tcx>, name_to_append_suffix_to: &mut String, ) -> &'ll DIArray { - let type_name_cache = &mut *debug_context(cx).type_name_cache.borrow_mut(); type_names::push_generic_params( cx.tcx, cx.tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), substs), name_to_append_suffix_to, - type_name_cache, + &debug_context(cx).type_name_cache, ); if substs.types().next().is_none() { diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index 7ce847ab0361d..525e37ea677a6 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -24,6 +24,7 @@ use rustc_target::abi::{Integer, TagEncoding, Variants}; use smallvec::SmallVec; use std::fmt::Write; +use std::cell::RefCell; // Compute the name of the type as it should be stored in debuginfo. Does not do // any caching, i.e., calling the function twice with the same type will also do @@ -33,7 +34,7 @@ pub fn compute_debuginfo_type_name<'tcx>( tcx: TyCtxt<'tcx>, t: Ty<'tcx>, qualified: bool, - type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, + type_name_cache: &RefCell, bool), String>>, ) -> String { let _prof = tcx.prof.generic_activity("compute_debuginfo_type_name"); @@ -51,10 +52,10 @@ fn push_debuginfo_type_name<'tcx>( qualified: bool, output: &mut String, visited: &mut FxHashSet>, - type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, + type_name_cache: &RefCell, bool), String>>, ) { // Check if we have seen this type and qualifier before. - if let Some(type_name) = type_name_cache.get(&(&t, qualified)) { + if let Some(type_name) = type_name_cache.borrow().get(&(&t, qualified)) { output.push_str(&type_name.clone()); return; } @@ -423,7 +424,7 @@ fn push_debuginfo_type_name<'tcx>( } } - if type_name_cache.insert((&t, qualified), output.clone()).is_some() { + if type_name_cache.borrow_mut().insert((&t, qualified), output.clone()).is_some() { bug!("type name is already in the type name cache!"); } @@ -438,7 +439,7 @@ fn push_debuginfo_type_name<'tcx>( substs: SubstsRef<'tcx>, output: &mut String, visited: &mut FxHashSet>, - type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, + type_name_cache: &RefCell, bool), String>>, ) { let layout = tcx.layout_of(tcx.param_env(def.did).and(ty)).expect("layout error"); @@ -556,7 +557,7 @@ fn push_generic_params_internal<'tcx>( substs: SubstsRef<'tcx>, output: &mut String, visited: &mut FxHashSet>, - type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, + type_name_cache: &RefCell, bool), String>>, ) -> bool { if substs.non_erasable_generics().next().is_none() { return false; @@ -643,7 +644,7 @@ pub fn push_generic_params<'tcx>( tcx: TyCtxt<'tcx>, substs: SubstsRef<'tcx>, output: &mut String, - type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, + type_name_cache: &RefCell, bool), String>>, ) { let _prof = tcx.prof.generic_activity("compute_debuginfo_type_name"); let mut visited = FxHashSet::default(); From 6412f3f4ecae64fb4703184aecb7f02e4280831c Mon Sep 17 00:00:00 2001 From: Timothy Maloney Date: Wed, 22 Sep 2021 08:47:08 -0700 Subject: [PATCH 07/11] Appeased almighty tidy --- compiler/rustc_codegen_llvm/src/debuginfo/mod.rs | 2 +- compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index c1f0281777887..0e439a1390fcf 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -10,12 +10,12 @@ use self::utils::{create_DIArray, is_node_local_to_unit, DIB}; use crate::abi::FnAbi; use crate::builder::Builder; use crate::common::CodegenCx; +use crate::debuginfo::utils::debug_context; use crate::llvm; use crate::llvm::debuginfo::{ DIArray, DIBuilder, DIFile, DIFlags, DILexicalBlock, DILocation, DISPFlags, DIScope, DIType, DIVariable, }; -use crate::debuginfo::utils::debug_context; use crate::value::Value; use rustc_codegen_ssa::debuginfo::type_names; diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index 525e37ea677a6..a883e9f525c53 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -23,8 +23,8 @@ use rustc_middle::ty::{self, AdtDef, ExistentialProjection, Ty, TyCtxt}; use rustc_target::abi::{Integer, TagEncoding, Variants}; use smallvec::SmallVec; -use std::fmt::Write; use std::cell::RefCell; +use std::fmt::Write; // Compute the name of the type as it should be stored in debuginfo. Does not do // any caching, i.e., calling the function twice with the same type will also do @@ -52,7 +52,7 @@ fn push_debuginfo_type_name<'tcx>( qualified: bool, output: &mut String, visited: &mut FxHashSet>, - type_name_cache: &RefCell, bool), String>>, + type_name_cache: &RefCell, bool), String>>, ) { // Check if we have seen this type and qualifier before. if let Some(type_name) = type_name_cache.borrow().get(&(&t, qualified)) { @@ -439,7 +439,7 @@ fn push_debuginfo_type_name<'tcx>( substs: SubstsRef<'tcx>, output: &mut String, visited: &mut FxHashSet>, - type_name_cache: &RefCell, bool), String>>, + type_name_cache: &RefCell, bool), String>>, ) { let layout = tcx.layout_of(tcx.param_env(def.did).and(ty)).expect("layout error"); From c53c12c1e101366abb3093e01b1764816de00a4f Mon Sep 17 00:00:00 2001 From: Timothy Maloney Date: Thu, 23 Sep 2021 10:48:27 -0700 Subject: [PATCH 08/11] Changed argument type; moved cache insertion. --- .../src/debuginfo/type_names.rs | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index a883e9f525c53..684abcd1305e3 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -23,7 +23,6 @@ use rustc_middle::ty::{self, AdtDef, ExistentialProjection, Ty, TyCtxt}; use rustc_target::abi::{Integer, TagEncoding, Variants}; use smallvec::SmallVec; -use std::cell::RefCell; use std::fmt::Write; // Compute the name of the type as it should be stored in debuginfo. Does not do @@ -34,13 +33,16 @@ pub fn compute_debuginfo_type_name<'tcx>( tcx: TyCtxt<'tcx>, t: Ty<'tcx>, qualified: bool, - type_name_cache: &RefCell, bool), String>>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, ) -> String { let _prof = tcx.prof.generic_activity("compute_debuginfo_type_name"); let mut result = String::with_capacity(64); let mut visited = FxHashSet::default(); push_debuginfo_type_name(tcx, t, qualified, &mut result, &mut visited, type_name_cache); + if type_name_cache.insert((t, qualified), result.clone()).is_some() { + bug!("type name is already in the type name cache!"); + } result } @@ -52,11 +54,11 @@ fn push_debuginfo_type_name<'tcx>( qualified: bool, output: &mut String, visited: &mut FxHashSet>, - type_name_cache: &RefCell, bool), String>>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, ) { // Check if we have seen this type and qualifier before. - if let Some(type_name) = type_name_cache.borrow().get(&(&t, qualified)) { - output.push_str(&type_name.clone()); + if let Some(type_name) = type_name_cache.get(&(t, qualified)) { + output.push_str(&type_name[..]); return; } @@ -424,10 +426,6 @@ fn push_debuginfo_type_name<'tcx>( } } - if type_name_cache.borrow_mut().insert((&t, qualified), output.clone()).is_some() { - bug!("type name is already in the type name cache!"); - } - /// MSVC names enums differently than other platforms so that the debugging visualization // format (natvis) is able to understand enums and render the active variant correctly in the // debugger. For more information, look in `src/etc/natvis/intrinsic.natvis` and @@ -439,7 +437,7 @@ fn push_debuginfo_type_name<'tcx>( substs: SubstsRef<'tcx>, output: &mut String, visited: &mut FxHashSet>, - type_name_cache: &RefCell, bool), String>>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, ) { let layout = tcx.layout_of(tcx.param_env(def.did).and(ty)).expect("layout error"); @@ -557,7 +555,7 @@ fn push_generic_params_internal<'tcx>( substs: SubstsRef<'tcx>, output: &mut String, visited: &mut FxHashSet>, - type_name_cache: &RefCell, bool), String>>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, ) -> bool { if substs.non_erasable_generics().next().is_none() { return false; @@ -644,7 +642,7 @@ pub fn push_generic_params<'tcx>( tcx: TyCtxt<'tcx>, substs: SubstsRef<'tcx>, output: &mut String, - type_name_cache: &RefCell, bool), String>>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, ) { let _prof = tcx.prof.generic_activity("compute_debuginfo_type_name"); let mut visited = FxHashSet::default(); From fdd211a5d12344013061ba97da23a65cdca53c33 Mon Sep 17 00:00:00 2001 From: Timothy Maloney Date: Thu, 23 Sep 2021 10:49:32 -0700 Subject: [PATCH 09/11] Changed args Appeased almighty tidy Changed args --- compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs | 7 ++++++- compiler/rustc_codegen_llvm/src/debuginfo/mod.rs | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 21b7beb659d94..cc0ddf3132dac 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -375,7 +375,12 @@ macro_rules! return_if_metadata_created_in_meantime { } fn check_type_name_cache(cx: &CodegenCx<'ll, 'tcx>, ty: Ty<'tcx>, qualified: bool) -> String { - compute_debuginfo_type_name(cx.tcx, ty, qualified, &debug_context(cx).type_name_cache) + compute_debuginfo_type_name( + cx.tcx, + ty, + qualified, + &mut debug_context(cx).type_name_cache.borrow_mut(), + ) } fn fixed_vec_metadata( diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index 0e439a1390fcf..fd6977dfc7ca6 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -442,7 +442,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { cx.tcx, cx.tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), substs), name_to_append_suffix_to, - &debug_context(cx).type_name_cache, + &mut debug_context(cx).type_name_cache.borrow_mut(), ); if substs.types().next().is_none() { From 13a502d2e8380c4baaf4bc67bf3d67fa03635567 Mon Sep 17 00:00:00 2001 From: Timothy Maloney Date: Mon, 4 Oct 2021 11:23:19 -0700 Subject: [PATCH 10/11] Changed String to Rc and added extra cache check --- .../rustc_codegen_llvm/src/debuginfo/mod.rs | 3 ++- .../src/debuginfo/type_names.rs | 18 +++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index fd6977dfc7ca6..d5e7e299135a3 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -38,6 +38,7 @@ use rustc_target::abi::{Primitive, Size}; use libc::c_uint; use smallvec::SmallVec; use std::cell::RefCell; +use std::rc::Rc; use std::iter; use tracing::debug; @@ -63,7 +64,7 @@ pub struct CrateDebugContext<'a, 'tcx> { builder: &'a mut DIBuilder<'a>, created_files: RefCell, Option), &'a DIFile>>, created_enum_disr_types: RefCell>, - type_name_cache: RefCell, bool), String>>, + type_name_cache: RefCell, bool), Rc>>, type_map: RefCell>, namespace_map: RefCell>, diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index 684abcd1305e3..3f273e28833b9 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -24,6 +24,7 @@ use rustc_target::abi::{Integer, TagEncoding, Variants}; use smallvec::SmallVec; use std::fmt::Write; +use std::rc::Rc; // Compute the name of the type as it should be stored in debuginfo. Does not do // any caching, i.e., calling the function twice with the same type will also do @@ -33,14 +34,18 @@ pub fn compute_debuginfo_type_name<'tcx>( tcx: TyCtxt<'tcx>, t: Ty<'tcx>, qualified: bool, - type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), Rc>, ) -> String { let _prof = tcx.prof.generic_activity("compute_debuginfo_type_name"); + if let Some(type_name) = type_name_cache.get(&(t, qualified)) { + return String::from(&type_name[..]); + } + let mut result = String::with_capacity(64); let mut visited = FxHashSet::default(); push_debuginfo_type_name(tcx, t, qualified, &mut result, &mut visited, type_name_cache); - if type_name_cache.insert((t, qualified), result.clone()).is_some() { + if type_name_cache.insert((t, qualified), Rc::from(&*result)).is_some() { bug!("type name is already in the type name cache!"); } result @@ -54,9 +59,8 @@ fn push_debuginfo_type_name<'tcx>( qualified: bool, output: &mut String, visited: &mut FxHashSet>, - type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), Rc>, ) { - // Check if we have seen this type and qualifier before. if let Some(type_name) = type_name_cache.get(&(t, qualified)) { output.push_str(&type_name[..]); return; @@ -437,7 +441,7 @@ fn push_debuginfo_type_name<'tcx>( substs: SubstsRef<'tcx>, output: &mut String, visited: &mut FxHashSet>, - type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), Rc>, ) { let layout = tcx.layout_of(tcx.param_env(def.did).and(ty)).expect("layout error"); @@ -555,7 +559,7 @@ fn push_generic_params_internal<'tcx>( substs: SubstsRef<'tcx>, output: &mut String, visited: &mut FxHashSet>, - type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), Rc>, ) -> bool { if substs.non_erasable_generics().next().is_none() { return false; @@ -642,7 +646,7 @@ pub fn push_generic_params<'tcx>( tcx: TyCtxt<'tcx>, substs: SubstsRef<'tcx>, output: &mut String, - type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), String>, + type_name_cache: &mut FxHashMap<(Ty<'tcx>, bool), Rc>, ) { let _prof = tcx.prof.generic_activity("compute_debuginfo_type_name"); let mut visited = FxHashSet::default(); From 353e42e8a3d79173c4ea91395c54624413d623c8 Mon Sep 17 00:00:00 2001 From: Timothy Maloney Date: Mon, 4 Oct 2021 11:42:53 -0700 Subject: [PATCH 11/11] Changed formatting --- compiler/rustc_codegen_llvm/src/debuginfo/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index d5e7e299135a3..705420e4075e4 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -38,8 +38,8 @@ use rustc_target::abi::{Primitive, Size}; use libc::c_uint; use smallvec::SmallVec; use std::cell::RefCell; -use std::rc::Rc; use std::iter; +use std::rc::Rc; use tracing::debug; mod create_scope_map;