Skip to content

Refactor compression cache in v0 symbol mangler #87583

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 30, 2021
Merged

Refactor compression cache in v0 symbol mangler #87583

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8307072
Remove redundant option around compression caches
tmiasko Jul 27, 2021
0ce8001
Flatten compression caches into symbol mangler
tmiasko Jul 29, 2021
0eabbf8
Implement `Printer` for `&mut SymbolMangler`
tmiasko Jul 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 32 additions & 45 deletions compiler/rustc_symbol_mangling/src/v0.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,12 @@ pub(super) fn mangle(
let substs = tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), instance.substs);

let prefix = "_R";
let mut cx = SymbolMangler {
let mut cx = &mut SymbolMangler {
tcx,
compress: Some(Box::new(CompressionCaches {
start_offset: prefix.len(),

paths: FxHashMap::default(),
types: FxHashMap::default(),
consts: FxHashMap::default(),
})),
start_offset: prefix.len(),
paths: FxHashMap::default(),
types: FxHashMap::default(),
consts: FxHashMap::default(),
binders: vec![],
out: String::from(prefix),
};
Expand All @@ -52,17 +49,7 @@ pub(super) fn mangle(
if let Some(instantiating_crate) = instantiating_crate {
cx = cx.print_def_path(instantiating_crate.as_def_id(), &[]).unwrap();
}
cx.out
}

struct CompressionCaches<'tcx> {
// The length of the prefix in `out` (e.g. 2 for `_R`).
start_offset: usize,

// The values are start positions in `out`, in bytes.
paths: FxHashMap<(DefId, &'tcx [GenericArg<'tcx>]), usize>,
types: FxHashMap<Ty<'tcx>, usize>,
consts: FxHashMap<&'tcx ty::Const<'tcx>, usize>,
std::mem::take(&mut cx.out)
}

struct BinderLevel {
Expand All @@ -81,9 +68,15 @@ struct BinderLevel {

struct SymbolMangler<'tcx> {
tcx: TyCtxt<'tcx>,
compress: Option<Box<CompressionCaches<'tcx>>>,
binders: Vec<BinderLevel>,
out: String,

/// The length of the prefix in `out` (e.g. 2 for `_R`).
start_offset: usize,
/// The values are start positions in `out`, in bytes.
Copy link
Member

@eddyb eddyb Jul 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple nitpicks (feel free to ignore I guess):

  • comment was not doc comment because it applies to all 3 fields
  • the names of the fields should've probably been adjusted (or the separate struct kept), since now they've lost a bit of context

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. Will keep this in mind when making further changes there.

I was looking into the idea of using only shortening back-references, but differences seems to be negligible. For cargo, total symbol size reduction of -0.016% (nm target/debug/cargo | grep -o '_R.*' | wc --bytes). Could be more useful with const generics, I suppose.

paths: FxHashMap<(DefId, &'tcx [GenericArg<'tcx>]), usize>,
types: FxHashMap<Ty<'tcx>, usize>,
consts: FxHashMap<&'tcx ty::Const<'tcx>, usize>,
}

impl SymbolMangler<'tcx> {
Expand Down Expand Up @@ -160,13 +153,13 @@ impl SymbolMangler<'tcx> {
self.push(ident);
}

fn path_append_ns(
mut self,
print_prefix: impl FnOnce(Self) -> Result<Self, !>,
fn path_append_ns<'a>(
mut self: &'a mut Self,
print_prefix: impl FnOnce(&'a mut Self) -> Result<&'a mut Self, !>,
ns: char,
disambiguator: u64,
name: &str,
) -> Result<Self, !> {
) -> Result<&'a mut Self, !> {
self.push("N");
self.out.push(ns);
self = print_prefix(self)?;
Expand All @@ -175,17 +168,17 @@ impl SymbolMangler<'tcx> {
Ok(self)
}

fn print_backref(mut self, i: usize) -> Result<Self, !> {
fn print_backref(&mut self, i: usize) -> Result<&mut Self, !> {
self.push("B");
self.push_integer_62((i - self.compress.as_ref().unwrap().start_offset) as u64);
self.push_integer_62((i - self.start_offset) as u64);
Ok(self)
}

fn in_binder<T>(
mut self,
fn in_binder<'a, T>(
mut self: &'a mut Self,
value: &ty::Binder<'tcx, T>,
print_value: impl FnOnce(Self, &T) -> Result<Self, !>,
) -> Result<Self, !>
print_value: impl FnOnce(&'a mut Self, &T) -> Result<&'a mut Self, !>,
) -> Result<&'a mut Self, !>
where
T: TypeFoldable<'tcx>,
{
Expand Down Expand Up @@ -218,7 +211,7 @@ impl SymbolMangler<'tcx> {
}
}

impl Printer<'tcx> for SymbolMangler<'tcx> {
impl Printer<'tcx> for &mut SymbolMangler<'tcx> {
type Error = !;

type Path = Self;
Expand All @@ -236,7 +229,7 @@ impl Printer<'tcx> for SymbolMangler<'tcx> {
def_id: DefId,
substs: &'tcx [GenericArg<'tcx>],
) -> Result<Self::Path, Self::Error> {
if let Some(&i) = self.compress.as_ref().and_then(|c| c.paths.get(&(def_id, substs))) {
if let Some(&i) = self.paths.get(&(def_id, substs)) {
return self.print_backref(i);
}
let start = self.out.len();
Expand All @@ -246,9 +239,7 @@ impl Printer<'tcx> for SymbolMangler<'tcx> {
// Only cache paths that do not refer to an enclosing
// binder (which would change depending on context).
if !substs.iter().any(|k| k.has_escaping_bound_vars()) {
if let Some(c) = &mut self.compress {
c.paths.insert((def_id, substs), start);
}
self.paths.insert((def_id, substs), start);
}
Ok(self)
}
Expand Down Expand Up @@ -312,7 +303,7 @@ impl Printer<'tcx> for SymbolMangler<'tcx> {
Ok(self)
}

fn print_region(mut self, region: ty::Region<'_>) -> Result<Self::Region, Self::Error> {
fn print_region(self, region: ty::Region<'_>) -> Result<Self::Region, Self::Error> {
let i = match *region {
// Erased lifetimes use the index 0, for a
// shorter mangling of `L_`.
Expand Down Expand Up @@ -367,7 +358,7 @@ impl Printer<'tcx> for SymbolMangler<'tcx> {
return Ok(self);
}

if let Some(&i) = self.compress.as_ref().and_then(|c| c.types.get(&ty)) {
if let Some(&i) = self.types.get(&ty) {
return self.print_backref(i);
}
let start = self.out.len();
Expand Down Expand Up @@ -476,9 +467,7 @@ impl Printer<'tcx> for SymbolMangler<'tcx> {
// Only cache types that do not refer to an enclosing
// binder (which would change depending on context).
if !ty.has_escaping_bound_vars() {
if let Some(c) = &mut self.compress {
c.types.insert(ty, start);
}
self.types.insert(ty, start);
}
Ok(self)
}
Expand Down Expand Up @@ -545,7 +534,7 @@ impl Printer<'tcx> for SymbolMangler<'tcx> {
}

fn print_const(mut self, ct: &'tcx ty::Const<'tcx>) -> Result<Self::Const, Self::Error> {
if let Some(&i) = self.compress.as_ref().and_then(|c| c.consts.get(&ct)) {
if let Some(&i) = self.consts.get(&ct) {
return self.print_backref(i);
}
let start = self.out.len();
Expand Down Expand Up @@ -583,14 +572,12 @@ impl Printer<'tcx> for SymbolMangler<'tcx> {
// Only cache consts that do not refer to an enclosing
// binder (which would change depending on context).
if !ct.has_escaping_bound_vars() {
if let Some(c) = &mut self.compress {
c.consts.insert(ct, start);
}
self.consts.insert(ct, start);
}
Ok(self)
}

fn path_crate(mut self, cnum: CrateNum) -> Result<Self::Path, Self::Error> {
fn path_crate(self, cnum: CrateNum) -> Result<Self::Path, Self::Error> {
self.push("C");
let stable_crate_id = self.tcx.def_path_hash(cnum.as_def_id()).stable_crate_id();
self.push_disambiguator(stable_crate_id.to_u64());
Expand Down