From 210b0363206dfe4cb58edb7879d4ab53fc6c3ea9 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 7 May 2020 13:56:02 -0700 Subject: [PATCH] peepmatic: Represent various id types with `u16` These ids end up in the automaton, so making them smaller should give us better data cache locality and also smaller serialized sizes. --- cranelift/codegen/src/preopt.serialized | Bin 6088 -> 5536 bytes .../crates/runtime/src/integer_interner.rs | 17 +++++------------ .../peepmatic/crates/runtime/src/linear.rs | 18 +++++++++++------- .../peepmatic/crates/runtime/src/optimizer.rs | 4 ++-- .../peepmatic/crates/runtime/src/paths.rs | 4 ++-- cranelift/peepmatic/src/dot_fmt.rs | 8 +++++--- cranelift/peepmatic/src/linearize.rs | 3 ++- 7 files changed, 27 insertions(+), 27 deletions(-) diff --git a/cranelift/codegen/src/preopt.serialized b/cranelift/codegen/src/preopt.serialized index 035de61c6289a0a3123e2f18e4104627db5cb361..f016c6d32d260ec758239ee2c0bfdb630735d80e 100644 GIT binary patch literal 5536 zcmcJT*OJ>X3`HrIE$O`%Z+4SS@BJzN|5FK$z-xlCMVFayh9(H$f&d6ol8=+OE{|y< zqEOwEHr9yT~G8JwL+{v+EdF+zbo1~-6o26@+I!O02HBXC79jA|(I!Zq?b(sF< zHchK5xRNxqiY#j_*^>1|wvB)CF|+#Nm;L@0qYrVOx&vd(Tz%4>k*N)UQ58$t%;2^H zj^%}n$?n+T9{0ZF^xA-DWdL3@0Ti!mc%A{6x@jA1qScGdtl5&`#9`{#f&;f=@OsvQ zw`5`UurDC2^$t6k{^U_Zlu{_S3wLho0ZLho0ZLho0ZLho0Zpu3zohDZH7MrZ@s#{q6Zj`-c!AgVCEKO;2uk0Q|Znf14=N9mY(A60!Cr`7XtQ!XQxb+Ah=V-)VEXKI1G-g<5xY(P*6HInjhm*Ghx1;`f1;0y zUn3fxhnGf;<-sk)Jj8d2UGosJ+vH)LzDHKqhQ}g_%ih>jvHA(uxzZaJv5jkSyT$zJ zmZ*8PWxb#AUiCJtZ=}7~MI@fJ*9Oz}kT{~vZ9G)$c*t{988Xs5tY_^y)3+ALcn`>i zM&$0Lv?O8fJUE}Mk5!=~{MS!oe6H_Wg%L{zu-m%L*6AChZ``x3)A%*(F(`g$kEM*r z#4DPGLk^jJvp$w36IpYNXDP0@YnCDwtJrO2X`O!Wkp09eh4)+n8?npxd(`RjSAnwP zF?n8o1S}*w@O{8z$G!}BRQ~4ykMsIrz~d|7-GIkEq_=sAxQAb9l1J|C7Xu#8tr#z2 zzJi`v_op-*?VD%O|4%`S$US=AbZP8CYafuIDU^|k=kWp}G~QO8hB+kvtF&zEe;0@O z3VaLFedZGPlqcdtXDol2w^pe2-6rR2{R+HMwXDTAE_ujX(|T)Uqh+nmmix9a%uVa7 z(VrOiuJyRbenWe9ru~=x!Zb?L9-kVkd7ke~LQp>;kq`7~b$oehcpSmqOJe5mxJDm3M%<+_LSwvo% LTjWlO|E>H3!dxw) literal 6088 zcmchb=W-h{41{$~b)4RNo8le!^F-O+L>v0b`@4B0}0lu7Wc~33b4cx=VOrLg+?q;zKU>z`%o5 z;&BzrnoR&V@tM|zGc@glQ5+wC<$u>%6Tu>OtIN*c3mXqh6~-NuG*#q9xB$>VflSJ_ziB^hG&IBL0UHhLkFpJw#x zJ(yQwux&-u?!ChIR@Dgu|5q^}{4d5hE(W=ULB0ze4DfRuj=`E4$E}0uhstpImYZcgrONX?8t{PTK&~J6|4UGJ_73`{dJ6hnEKS~@47fJAnvwS zpN@*x__1l9UW9wAPr}K4@^_$<_Q^4*njDN?pVq~J0jbteTHZ6S%|~|$&zU&A_pzpy zA9t&I6YO*+WnISCJ16goZ(?|@ks250Dqh@U4vCG$WtP}Zbj8-4RTu8C%qcAa^w2zJf4mlXTKCX)c1M=sI{EUxE z;PbDF`=HDFA00%l?}b}@@SMIa#KQ4xKPMe$?^Duo2fR-@em}iUy1kI~+eRk7171p~ zGv6D}laB9HUo&QYD?W{(cMEjBMr3+I&G=%Iomu_j+4Pxs#* z{0r90p?}I0s$Be@W=O`a+Bw$`P#7Wia^)Cdb5_^o;=Qql>+hD7BwXGP*WuEu9&YZZ zaOV+wBZlN)r{3%bezm0fj=;7hxBs}hyR&V)^ P?_uNPBy8?8|NrYRqEs!S diff --git a/cranelift/peepmatic/crates/runtime/src/integer_interner.rs b/cranelift/peepmatic/crates/runtime/src/integer_interner.rs index c2f0830bf0..996100a66d 100644 --- a/cranelift/peepmatic/crates/runtime/src/integer_interner.rs +++ b/cranelift/peepmatic/crates/runtime/src/integer_interner.rs @@ -7,11 +7,11 @@ use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; -use std::num::NonZeroU32; +use std::num::{NonZeroU16, NonZeroU32}; /// An identifier for an interned integer. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub struct IntegerId(#[doc(hidden)] pub NonZeroU32); +pub struct IntegerId(#[doc(hidden)] pub NonZeroU16); /// An interner for integer values. #[derive(Debug, Default, Serialize, Deserialize)] @@ -40,8 +40,8 @@ impl IntegerInterner { return *id; } - assert!((self.values.len() as u64) < (std::u32::MAX as u64)); - let id = IntegerId(unsafe { NonZeroU32::new_unchecked(self.values.len() as u32 + 1) }); + assert!((self.values.len() as u64) < (std::u16::MAX as u64)); + let id = IntegerId(unsafe { NonZeroU16::new_unchecked(self.values.len() as u16 + 1) }); self.values.push(value); self.map.insert(value, id); @@ -65,16 +65,9 @@ impl IntegerInterner { } } -impl From for u32 { - #[inline] - fn from(id: IntegerId) -> u32 { - id.0.get() - } -} - impl From for NonZeroU32 { #[inline] fn from(id: IntegerId) -> NonZeroU32 { - id.0 + id.0.into() } } diff --git a/cranelift/peepmatic/crates/runtime/src/linear.rs b/cranelift/peepmatic/crates/runtime/src/linear.rs index fe98edb897..a51159f708 100644 --- a/cranelift/peepmatic/crates/runtime/src/linear.rs +++ b/cranelift/peepmatic/crates/runtime/src/linear.rs @@ -149,11 +149,11 @@ pub enum MatchOp { /// A canonicalized identifier for a left-hand side value that was bound in a /// pattern. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] -pub struct LhsId(pub u32); +pub struct LhsId(pub u16); /// A canonicalized identifier for a right-hand side value. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub struct RhsId(pub u32); +pub struct RhsId(pub u16); /// An action to perform when transitioning between states in the automata. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] @@ -243,18 +243,22 @@ pub enum Action { mod tests { use super::*; + // These types all end up in the automaton, so we should take care that they + // are small and don't fill up the data cache (or take up too much + // serialized size). + #[test] - fn match_result_is_4_bytes_in_size() { + fn match_result_size() { assert_eq!(std::mem::size_of::(), 4); } #[test] - fn match_op_is_12_bytes_in_size() { - assert_eq!(std::mem::size_of::(), 12); + fn match_op_size() { + assert_eq!(std::mem::size_of::(), 6); } #[test] - fn action_is_20_bytes_in_size() { - assert_eq!(std::mem::size_of::(), 20); + fn action_size() { + assert_eq!(std::mem::size_of::(), 16); } } diff --git a/cranelift/peepmatic/crates/runtime/src/optimizer.rs b/cranelift/peepmatic/crates/runtime/src/optimizer.rs index fc5916f247..2cbf823a4b 100644 --- a/cranelift/peepmatic/crates/runtime/src/optimizer.rs +++ b/cranelift/peepmatic/crates/runtime/src/optimizer.rs @@ -414,7 +414,7 @@ where Part::Constant(c) => { let x = c.as_int().ok_or(Else)?; let id = self.peep_opt.integers.already_interned(x).ok_or(Else)?; - Ok(id.0) + Ok(id.into()) } Part::Instruction(i) => { let c = self @@ -423,7 +423,7 @@ where .ok_or(Else)?; let x = c.as_int().ok_or(Else)?; let id = self.peep_opt.integers.already_interned(x).ok_or(Else)?; - Ok(id.0) + Ok(id.into()) } Part::ConditionCode(_) => unreachable!("IntegerValue on condition code"), } diff --git a/cranelift/peepmatic/crates/runtime/src/paths.rs b/cranelift/peepmatic/crates/runtime/src/paths.rs index a30aa81886..d826ba76bb 100644 --- a/cranelift/peepmatic/crates/runtime/src/paths.rs +++ b/cranelift/peepmatic/crates/runtime/src/paths.rs @@ -46,7 +46,7 @@ impl Path<'_> { /// An identifier for an interned path. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub struct PathId(u32); +pub struct PathId(u16); /// An interner and de-duplicator for `Path`s. /// @@ -93,7 +93,7 @@ impl PathInterner { #[inline(never)] fn intern_new<'a>(&mut self, path: Path<'a>) -> PathId { - let id: u32 = self + let id: u16 = self .paths .len() .try_into() diff --git a/cranelift/peepmatic/src/dot_fmt.rs b/cranelift/peepmatic/src/dot_fmt.rs index 959fdd96d9..1d854d6561 100644 --- a/cranelift/peepmatic/src/dot_fmt.rs +++ b/cranelift/peepmatic/src/dot_fmt.rs @@ -10,9 +10,9 @@ use peepmatic_runtime::{ operator::Operator, paths::{PathId, PathInterner}, }; -use std::convert::TryFrom; +use std::convert::{TryFrom, TryInto}; use std::io::{self, Write}; -use std::num::NonZeroU32; +use std::num::NonZeroU16; #[derive(Debug)] pub(crate) struct PeepholeDotFmt<'a>(pub(crate) &'a PathInterner, pub(crate) &'a IntegerInterner); @@ -39,7 +39,9 @@ impl DotFmt> for Peeph write!(w, "{}", cc) } linear::MatchOp::IntegerValue { .. } => { - let x = self.1.lookup(IntegerId(NonZeroU32::new(x).unwrap())); + let x = self + .1 + .lookup(IntegerId(NonZeroU16::new(x.try_into().unwrap()).unwrap())); write!(w, "{}", x) } _ => write!(w, "Ok({})", x), diff --git a/cranelift/peepmatic/src/linearize.rs b/cranelift/peepmatic/src/linearize.rs index 5a47abcfcf..eb03853fa2 100644 --- a/cranelift/peepmatic/src/linearize.rs +++ b/cranelift/peepmatic/src/linearize.rs @@ -92,6 +92,7 @@ use peepmatic_runtime::{ paths::{Path, PathId, PathInterner}, }; use std::collections::BTreeMap; +use std::convert::TryInto; use std::num::NonZeroU32; use wast::Id; @@ -339,7 +340,7 @@ impl<'a> RhsBuilder<'a> { ) { while let Some(rhs) = self.rhs_post_order.next() { actions.push(self.rhs_to_linear_action(integers, lhs_id_to_path, rhs)); - let id = linear::RhsId(self.rhs_span_to_id.len() as u32); + let id = linear::RhsId(self.rhs_span_to_id.len().try_into().unwrap()); self.rhs_span_to_id.insert(rhs.span(), id); } }