Merge pull request #2999 from rtfeldman/symbol-nonzero-u32

Symbol nonzero u32
This commit is contained in:
Ayaz 2022-05-05 13:29:40 -04:00 committed by GitHub
commit 8bcd64d502
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 88 additions and 48 deletions

View File

@ -19,6 +19,8 @@ use crate::mem_pool::shallow_clone::ShallowClone;
pub type TypeId = NodeId<Type2>;
const TYPE2_SIZE: () = assert!(std::mem::size_of::<Type2>() == 3 * 8 + 4);
#[derive(Debug)]
pub enum Type2 {
Variable(Variable), // 4B
@ -47,11 +49,6 @@ pub enum Type2 {
Erroneous(Problem2), // 24B
}
#[test]
fn type2_size() {
assert_eq!(std::mem::size_of::<Type2>(), 32); // 24B + pad
}
#[derive(Debug)]
pub enum Problem2 {
CanonicalizationProblem,

View File

@ -63,7 +63,7 @@ pub enum TagName {
}
roc_error_macros::assert_sizeof_aarch64!(TagName, 24);
roc_error_macros::assert_sizeof_wasm!(TagName, 16);
roc_error_macros::assert_sizeof_wasm!(TagName, 12);
roc_error_macros::assert_sizeof_default!(TagName, 24);
impl TagName {

View File

@ -5,11 +5,40 @@ use roc_ident::IdentStr;
use roc_region::all::Region;
use snafu::OptionExt;
use std::collections::HashMap;
use std::num::NonZeroU32;
use std::{fmt, u32};
// TODO: benchmark this as { ident_id: u32, module_id: u32 } and see if perf stays the same
// the packed(4) is needed for faster equality comparisons. With it, the structure is
// treated as a single u64, and comparison is one instruction
//
// example::eq_sym64:
// cmp rdi, rsi
// sete al
// ret
//
// while without it we get 2 extra instructions
//
// example::eq_sym64:
// xor edi, edx
// xor esi, ecx
// or esi, edi
// sete al
// ret
//
// #[repr(packed)] gives you #[repr(packed(1))], and then all your reads are unaligned
// so we set the alignment to (the natural) 4
#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct Symbol(u64);
#[repr(packed(4))]
pub struct Symbol {
ident_id: u32,
module_id: NonZeroU32,
}
/// An Option<Symbol> will use the 0 that is not used by the NonZeroU32 module_id field to encode
/// the Nothing case. An Option<Symbol> hence takes no more space than a Symbol.
#[allow(dead_code)]
const SYMBOL_HAS_NICHE: () =
assert!(std::mem::size_of::<Symbol>() == std::mem::size_of::<Option<Symbol>>());
// When this is `true` (which it normally should be), Symbol's Debug::fmt implementation
// attempts to pretty print debug symbols using interns recorded using
@ -28,7 +57,7 @@ impl Symbol {
// e.g. pub const NUM_NUM: Symbol = …
pub const fn new(module_id: ModuleId, ident_id: IdentId) -> Symbol {
// The bit layout of the u64 inside a Symbol is:
// The bit layout of the inside of a Symbol is:
//
// |------ 32 bits -----|------ 32 bits -----|
// | ident_id | module_id |
@ -37,20 +66,22 @@ impl Symbol {
// module_id comes second because we need to query it more often,
// and this way we can get it by truncating the u64 to u32,
// whereas accessing the first slot requires a bit shift first.
let bits = ((ident_id.0 as u64) << 32) | (module_id.0 as u64);
Symbol(bits)
Self {
module_id: module_id.0,
ident_id: ident_id.0,
}
}
pub fn module_id(self) -> ModuleId {
ModuleId(self.0 as u32)
pub const fn module_id(self) -> ModuleId {
ModuleId(self.module_id)
}
pub fn ident_id(self) -> IdentId {
IdentId((self.0 >> 32) as u32)
pub const fn ident_id(self) -> IdentId {
IdentId(self.ident_id)
}
pub fn is_builtin(self) -> bool {
pub const fn is_builtin(self) -> bool {
self.module_id().is_builtin()
}
@ -88,8 +119,8 @@ impl Symbol {
})
}
pub fn as_u64(self) -> u64 {
self.0
pub const fn as_u64(self) -> u64 {
u64::from_ne_bytes(self.to_ne_bytes())
}
pub fn fully_qualified(self, interns: &Interns, home: ModuleId) -> ModuleName {
@ -109,7 +140,7 @@ impl Symbol {
}
pub const fn to_ne_bytes(self) -> [u8; 8] {
self.0.to_ne_bytes()
unsafe { std::mem::transmute(self) }
}
#[cfg(debug_assertions)]
@ -133,7 +164,7 @@ impl fmt::Debug for Symbol {
let ident_id = self.ident_id();
match DEBUG_IDENT_IDS_BY_MODULE_ID.lock() {
Ok(names) => match &names.get(&module_id.0) {
Ok(names) => match &names.get(&(module_id.to_zero_indexed() as u32)) {
Some(ident_ids) => match ident_ids.get_name(ident_id) {
Some(ident_str) => write!(f, "`{:?}.{}`", module_id, ident_str),
None => fallback_debug_fmt(*self, f),
@ -174,7 +205,7 @@ impl fmt::Display for Symbol {
impl From<Symbol> for u64 {
fn from(symbol: Symbol) -> Self {
symbol.0
symbol.as_u64()
}
}
@ -292,18 +323,31 @@ lazy_static! {
/// A globally unique ID that gets assigned to each module as it is loaded.
#[derive(Copy, Clone, PartialEq, Eq, Hash)]
pub struct ModuleId(u32);
pub struct ModuleId(NonZeroU32);
impl ModuleId {
// NOTE: the define_builtins! macro adds a bunch of constants to this impl,
//
// e.g. pub const NUM: ModuleId = …
const fn from_zero_indexed(mut id: usize) -> Self {
id += 1;
// only happens on overflow
debug_assert!(id != 0);
ModuleId(unsafe { NonZeroU32::new_unchecked(id as u32) })
}
const fn to_zero_indexed(self) -> usize {
(self.0.get() - 1) as usize
}
#[cfg(debug_assertions)]
pub fn register_debug_idents(self, ident_ids: &IdentIds) {
let mut all = DEBUG_IDENT_IDS_BY_MODULE_ID.lock().expect("Failed to acquire lock for Debug interning into DEBUG_MODULE_ID_NAMES, presumably because a thread panicked.");
all.insert(self.0, ident_ids.clone());
all.insert(self.to_zero_indexed() as u32, ident_ids.clone());
}
#[cfg(not(debug_assertions))]
@ -336,7 +380,7 @@ impl fmt::Debug for ModuleId {
.expect("Failed to acquire lock for Debug reading from DEBUG_MODULE_ID_NAMES, presumably because a thread panicked.");
if PRETTY_PRINT_DEBUG_SYMBOLS {
match names.get(&self.0) {
match names.get(&(self.to_zero_indexed() as u32)) {
Some(str_ref) => write!(f, "{}", str_ref.clone()),
None => {
panic!(
@ -394,7 +438,7 @@ impl<'a> PackageModuleIds<'a> {
Some(id) => *id,
None => {
let by_id = &mut self.by_id;
let module_id = ModuleId(by_id.len() as u32);
let module_id = ModuleId::from_zero_indexed(by_id.len());
by_id.push(module_name.clone());
@ -430,7 +474,7 @@ impl<'a> PackageModuleIds<'a> {
let mut names = DEBUG_MODULE_ID_NAMES.lock().expect("Failed to acquire lock for Debug interning into DEBUG_MODULE_ID_NAMES, presumably because a thread panicked.");
names
.entry(module_id.0)
.entry(module_id.to_zero_indexed() as u32)
.or_insert_with(|| match module_name {
PQModuleName::Unqualified(module) => module.as_str().into(),
PQModuleName::Qualified(package, module) => {
@ -450,7 +494,7 @@ impl<'a> PackageModuleIds<'a> {
}
pub fn get_name(&self, id: ModuleId) -> Option<&PQModuleName> {
self.by_id.get(id.0 as usize)
self.by_id.get(id.to_zero_indexed())
}
pub fn available_modules(&self) -> impl Iterator<Item = &PQModuleName> {
@ -475,7 +519,7 @@ impl ModuleIds {
Some(id) => *id,
None => {
let by_id = &mut self.by_id;
let module_id = ModuleId(by_id.len() as u32);
let module_id = ModuleId::from_zero_indexed(by_id.len());
by_id.push(module_name.clone());
@ -496,7 +540,7 @@ impl ModuleIds {
// TODO make sure modules are never added more than once!
names
.entry(module_id.0)
.entry(module_id.to_zero_indexed() as u32)
.or_insert_with(|| module_name.as_str().to_string().into());
}
@ -510,7 +554,7 @@ impl ModuleIds {
}
pub fn get_name(&self, id: ModuleId) -> Option<&ModuleName> {
self.by_id.get(id.0 as usize)
self.by_id.get(id.to_zero_indexed())
}
pub fn available_modules(&self) -> impl Iterator<Item = &ModuleName> {
@ -736,7 +780,8 @@ macro_rules! define_builtins {
let mut exposed_idents_by_module = VecMap::with_capacity(extra_capacity + $total);
$(
debug_assert!(!exposed_idents_by_module.contains_key(&ModuleId($module_id)), r"Error setting up Builtins: when setting up module {} {:?} - the module ID {} is already present in the map. Check the map for duplicate module IDs!", $module_id, $module_name, $module_id);
let module_id = ModuleId::$module_const;
debug_assert!(!exposed_idents_by_module.contains_key(&module_id), r"Error setting up Builtins: when setting up module {} {:?} - the module ID {} is already present in the map. Check the map for duplicate module IDs!", $module_id, $module_name, $module_id);
let ident_ids = {
const TOTAL : usize = [ $($ident_name),+ ].len();
@ -780,8 +825,6 @@ macro_rules! define_builtins {
};
if cfg!(debug_assertions) {
let module_id = ModuleId($module_id);
let name = PQModuleName::Unqualified($module_name.into());
PackageModuleIds::insert_debug_name(module_id, &name);
module_id.register_debug_idents(&ident_ids);
@ -789,7 +832,7 @@ macro_rules! define_builtins {
exposed_idents_by_module.insert(
ModuleId($module_id),
module_id,
ident_ids
);
)+
@ -801,15 +844,15 @@ macro_rules! define_builtins {
}
impl ModuleId {
pub fn is_builtin(&self) -> bool {
pub const fn is_builtin(self) -> bool {
// This is a builtin ModuleId iff it's below the
// total number of builtin modules, since they
// take up the first $total ModuleId numbers.
self.0 < $total
self.to_zero_indexed() < $total
}
$(
pub const $module_const: ModuleId = ModuleId($module_id);
pub const $module_const: ModuleId = ModuleId::from_zero_indexed($module_id);
)+
}
@ -833,7 +876,7 @@ macro_rules! define_builtins {
};
$(
insert_both(ModuleId($module_id), $module_name);
insert_both(ModuleId::$module_const, $module_name);
)+
ModuleIds { by_name, by_id }
@ -861,7 +904,7 @@ macro_rules! define_builtins {
};
$(
insert_both(ModuleId($module_id), $module_name);
insert_both(ModuleId::$module_const, $module_name);
)+
PackageModuleIds { by_name, by_id }
@ -871,7 +914,7 @@ macro_rules! define_builtins {
impl Symbol {
$(
$(
pub const $ident_const: Symbol = Symbol::new(ModuleId($module_id), IdentId($ident_id));
pub const $ident_const: Symbol = Symbol::new(ModuleId::$module_const, IdentId($ident_id));
)+
)+
@ -892,7 +935,7 @@ macro_rules! define_builtins {
let $imported = true;
if $imported {
scope.insert($ident_name.into(), (Symbol::new(ModuleId($module_id), IdentId($ident_id)), Region::zero()));
scope.insert($ident_name.into(), (Symbol::new(ModuleId::$module_const, IdentId($ident_id)), Region::zero()));
}
)?
)+

View File

@ -56,11 +56,11 @@ roc_error_macros::assert_sizeof_aarch64!(Call, 7 * 8);
roc_error_macros::assert_sizeof_aarch64!(CallType, 5 * 8);
roc_error_macros::assert_sizeof_wasm!(Literal, 24);
roc_error_macros::assert_sizeof_wasm!(Expr, 56);
roc_error_macros::assert_sizeof_wasm!(Expr, 48);
roc_error_macros::assert_sizeof_wasm!(Stmt, 120);
roc_error_macros::assert_sizeof_wasm!(ProcLayout, 32);
roc_error_macros::assert_sizeof_wasm!(Call, 40);
roc_error_macros::assert_sizeof_wasm!(CallType, 32);
roc_error_macros::assert_sizeof_wasm!(Call, 36);
roc_error_macros::assert_sizeof_wasm!(CallType, 28);
roc_error_macros::assert_sizeof_default!(Literal, 3 * 8);
roc_error_macros::assert_sizeof_default!(Expr, 10 * 8);

View File

@ -13,8 +13,8 @@ use ven_ena::unify::{InPlace, Snapshot, UnificationTable, UnifyKey};
// if your changes cause this number to go down, great!
// please change it to the lower number.
// if it went up, maybe check that the change is really required
roc_error_macros::assert_sizeof_all!(Descriptor, 6 * 8);
roc_error_macros::assert_sizeof_all!(Content, 4 * 8);
roc_error_macros::assert_sizeof_all!(Descriptor, 5 * 8);
roc_error_macros::assert_sizeof_all!(Content, 3 * 8 + 4);
roc_error_macros::assert_sizeof_all!(FlatType, 3 * 8);
roc_error_macros::assert_sizeof_all!(UnionTags, 12);
roc_error_macros::assert_sizeof_all!(RecordFields, 2 * 8);
@ -2026,8 +2026,8 @@ impl From<Content> for Descriptor {
}
}
roc_error_macros::assert_sizeof_all!(Content, 4 * 8);
roc_error_macros::assert_sizeof_all!((Symbol, AliasVariables, Variable), 3 * 8);
roc_error_macros::assert_sizeof_all!(Content, 3 * 8 + 4);
roc_error_macros::assert_sizeof_all!((Symbol, AliasVariables, Variable), 2 * 8 + 4);
roc_error_macros::assert_sizeof_all!(AliasVariables, 8);
roc_error_macros::assert_sizeof_all!(FlatType, 3 * 8);