From 63e91fb01ef333aa4a5688c4e89e5d3b01835d65 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 8 Dec 2020 22:15:48 -0500 Subject: [PATCH 01/20] Use 32-byte nodes in the editor IR --- editor/src/ast.rs | 148 ++++++-------- editor/src/bucket.rs | 457 ------------------------------------------- editor/src/pool.rs | 333 +++++++++++++++++++++++++++++++ 3 files changed, 395 insertions(+), 543 deletions(-) delete mode 100644 editor/src/bucket.rs create mode 100644 editor/src/pool.rs diff --git a/editor/src/ast.rs b/editor/src/ast.rs index 9d2dc3dbeb..35aa12bd04 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -1,10 +1,12 @@ -use crate::bucket::{BucketId, BucketList, BucketSlot, BucketStr, NodeId}; -use arraystring::{typenum::U14, ArrayString}; +use crate::bucket::{BucketList, BucketStr, NodeId}; +use arraystring::{typenum::U30, ArrayString}; use roc_can::def::Annotation; use roc_can::expr::{Field, Recursive}; use roc_module::ident::Lowercase; use roc_module::low_level::LowLevel; +use roc_module::operator::CalledVia; use roc_module::symbol::Symbol; +use roc_problem::can::RuntimeError; use roc_types::subs::Variable; use roc_types::types::Alias; @@ -23,8 +25,8 @@ pub enum IntStyle { Binary, } -/// An Expr that fits in 16B. -/// It has a 1B discriminant and variants which hold payloads of at most 15B. +/// An Expr that fits in 32B. +/// It has a 1B discriminant and variants which hold payloads of at most 31B. #[derive(Debug)] pub enum Expr2 { /// A number literal (without a dot) containing no underscores @@ -40,28 +42,25 @@ pub enum Expr2 { }, /// A number literal (without a dot) containing underscores NumWithUnderscores { - number: i64, // 8B - var: Variable, // 4B - text_id: BucketId, // 2B - text_sl: BucketSlot, // 1B + number: i64, // 8B + var: Variable, // 4B + text: NodeId, // 8B }, /// A float literal (with a dot) containing underscores FloatWithUnderscores { - number: f64, // 8B - var: Variable, // 4B - text_id: BucketId, // 2B - text_sl: BucketSlot, // 1B + number: f64, // 8B + var: Variable, // 4B + text: NodeId, // 8B }, - /// string literals of length up to 14B - SmallStr(ArrayString), // 15B + /// string literals of length up to 30B + SmallStr(ArrayString), // 31B /// string literals of length up to 4094B MedStr { - str_id: BucketId, - str_sl: BucketSlot, - }, // 4B + str: NodeId, // 8B + }, /// string literals of length over 4094B, but requires calling malloc/free BigStr { - pointer: *const u8, // 8B on 64-bit systems + pointer: *const u8, // 8B len: u32, // 4B, meaning maximum string literal size of 4GB. Could theoretically fit 7B here, which would get closer to the full isize::MAX }, // Lookups @@ -76,100 +75,82 @@ pub enum Expr2 { List { list_var: Variable, // 4B - required for uniqueness of the list elem_var: Variable, // 4B - elems: BucketList, // 4B + elems: BucketList, // 9B }, If { cond_var: Variable, // 4B expr_var: Variable, // 4B - branches: BucketList<(Expr2, Expr2)>, // 4B - final_else_id: BucketId, // 2B - final_else_sl: BucketSlot, // 1B + branches: BucketList<(Expr2, Expr2)>, // 9B + final_else: NodeId, // 8B }, When { cond_var: Variable, // 4B expr_var: Variable, // 4B - branches: BucketList, // 4B - cond_id: BucketId, // 2B - cond_sl: BucketSlot, // 1B + branches: BucketList, // 9B + cond: NodeId, // 8B }, LetRec { // TODO need to make this Alias type here bucket-friendly, which will be hard! - aliases: BucketList<(Symbol, Alias)>, // 4B - defs: BucketList, // 4B + aliases: BucketList<(Symbol, Alias)>, // 9B + defs: BucketList, // 9B body_var: Variable, // 4B - body_id: BucketId, // 2B - body_sl: BucketSlot, // 1B + body_id: NodeId, // 8B }, LetNonRec { // TODO need to make this Alias type here bucket-friendly, which will be hard! - aliases: BucketList<(Symbol, Alias)>, // 4B - def_id: BucketId, // 2B - def_sl: BucketSlot, // 1B - body_id: BucketId, // 2B - body_sl: BucketSlot, // 1B + aliases: BucketList<(Symbol, Alias)>, // 9B + def_id: NodeId, // 8B + body_id: NodeId, // 8B body_var: Variable, // 4B }, Call { /// NOTE: the first elem in this list is the expression and its variable. /// The others are arguments. This is because we didn't have room for /// both the expr and its variable otherwise. - expr_and_args: BucketList<(Variable, NodeId)>, // 4B + expr_and_args: BucketList<(Variable, NodeId)>, // 9B fn_var: Variable, // 4B closure_var: Variable, // 4B /// Cached outside expr_and_args so we don't have to potentially /// traverse that whole linked list chain to count all the args. - arity: u16, // 2B - called_via: CalledVia2, // 1B + arity: usize, // 8B - could make this smaller if need be + called_via: CalledVia, // 2B }, RunLowLevel { op: LowLevel, // 1B - args: BucketList<(Variable, NodeId)>, // 4B + args: BucketList<(Variable, NodeId)>, // 9B ret_var: Variable, // 4B }, Closure { - /// NOTE: the first elem in this list is the function's name Symbol, plus Variable::NONE - /// - /// This is not ideal, but there's no room for an 8-byte Symbol - /// in a 16B node that already needs to hold this much other data. - captured_symbols: BucketList<(Symbol, Variable)>, // 4B - args: BucketList<(Variable, NodeId)>, // 4B - recursive: Recursive, // 1B - body_id: BucketId, // 2B - body_sl: BucketSlot, // 1B - vars_id: BucketId, // 2B - vars_sl: BucketSlot, // 1B + captured_symbols: BucketList<(Symbol, Variable)>, // 9B + args: BucketList<(Variable, NodeId)>, // 9B + recursive: Recursive, // 1B + extra: NodeId, // 8B }, // Product Types Record { record_var: Variable, // 4B - fields: BucketList<(BucketStr, Variable, NodeId)>, // 4B + fields: BucketList<(BucketStr, Variable, NodeId)>, // 9B }, /// Empty record constant EmptyRecord, /// Look up exactly one field on a record, e.g. (expr).foo. Access { - field_id: BucketId, // 3B - field_sl: BucketSlot, // 3B - expr_id: BucketId, // 2B - expr_sl: BucketSlot, // 1B - vars_id: BucketId, // 2B - vars_sl: BucketSlot, // 1B + field: NodeId, // 8B + expr: NodeId, // 8B + vars: NodeId, // 8B }, /// field accessor as a function, e.g. (.foo) expr Accessor { - record_vars_id: BucketId, // 3B - record_vars_sl: BucketSlot, // 3B - function_var: Variable, // 4B - closure_var: Variable, // 4B - field_id: BucketId, // 2B - field_sl: BucketSlot, // 1B + record_vars_id: NodeId, // 8B + function_var: Variable, // 4B + closure_var: Variable, // 4B + field_id: NodeId, // 8B }, Update { symbol: Symbol, // 8B - updates: BucketList<(Lowercase, Field)>, // 4B - vars_id: BucketId, // 2B - vars_sl: BucketSlot, // 1B + updates: BucketList<(Lowercase, Field)>, // 9B + vars_id: NodeId, // 8B }, // Sum Types @@ -177,11 +158,10 @@ pub enum Expr2 { // NOTE: A BucketStr node is a 2B length and then 14B bytes, // plus more bytes in adjacent nodes if necessary. Thus we have // a hard cap of 4094 bytes as the maximum length of tags and fields. - name_id: BucketId, // 2B - name_sl: BucketSlot, // 1B - variant_var: Variable, // 4B - ext_var: Variable, // 4B - arguments: BucketList<(Variable, BucketId, BucketSlot)>, // 4B + name_id: NodeId, // 8B + variant_var: Variable, // 4B + ext_var: Variable, // 4B + arguments: BucketList<(Variable, NodeId)>, // 9B }, // Compiles, but will crash if reached @@ -258,13 +238,15 @@ pub struct AccessVars { field_var: Variable, // 4B } -/// This is 16B, so it fits in a Node slot. +/// This is 32B, so it fits in a Node slot. #[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub struct ClosureVars { - function_type: Variable, - closure_type: Variable, - closure_ext_var: Variable, - return_type: Variable, +pub struct ClosureExtra { + name: Symbol, // 8B + body: NodeId, // 8B + function_type: Variable, // 4B + closure_type: Variable, // 4B + closure_ext_var: Variable, // 4B + return_type: Variable, // 4B } #[derive(Debug)] @@ -321,19 +303,13 @@ pub struct Exprs { pub buckets: Vec, } -// Each bucket has 256 slots. Each slot holds one 16B node +// Each bucket has 128 slots. Each slot holds one 32B node // This means each bucket is 4096B, which is the size of a memory page // on typical systems where the compiler will be run. // -// Because each bucket has 256 slots, and arrays of nodes must fit inside -// a single bucket, this implies that nodes which contain arrays of nodes -// (e.g. If, When, Record, Tag, Call, Closure) can only contain at most -// 255 nodes. So functions can have at most 255 arguments, records can have -// at most 255 fields, etc. -// // Nice things about this system include: // * Allocating a new bucket is as simple as asking the OS for a memory page. -// * Since each node is 16B, each node's memory address will be a multiple of 16. +// * Since each node is 32B, each node's memory address will be a multiple of 16. // * Thanks to the free lists and our consistent chunk sizes, we should // end up with very little fragmentation. // * Finding a slot for a given node should be very fast: see if the relevant @@ -350,7 +326,7 @@ pub struct Exprs { // usize pointers, which would be too big for us to have 16B nodes. // On the plus side, we could be okay with higher memory usage early on, // and then later use the Mesh strategy to reduce long-running memory usage. -type ExprBucketSlots = [Expr2; 256]; +type ExprBucketSlots = [Expr2; 128]; #[test] fn size_of_expr_bucket() { @@ -382,7 +358,7 @@ pub struct ExprBucketSlot(u8); #[test] fn size_of_expr() { - assert_eq!(std::mem::size_of::(), 16); + assert_eq!(std::mem::size_of::(), 32); } #[test] diff --git a/editor/src/bucket.rs b/editor/src/bucket.rs deleted file mode 100644 index 33b6cd7a4c..0000000000 --- a/editor/src/bucket.rs +++ /dev/null @@ -1,457 +0,0 @@ -/// A bucket of 16-byte nodes. The node value 0 is reserved for the bucket's -/// use, and valid nodes may never have that value. -/// -/// By design, each bucket is 4096 bytes large. When you make a bucket, it -/// uses mmap to reserve one anonymous memory page in which to store nodes. -/// Since nodes are 16 bytes, one bucket can store 256 nodes; you can access -/// a particular node by its BucketSlot, which is an opaque wrapper around a u8. -/// -/// Buckets also use the node value 0 (all 0 bits) to mark slots as unoccupied. -/// This is important for performance. -use libc::{c_void, calloc, free, mmap, munmap, MAP_ANONYMOUS, MAP_PRIVATE, PROT_READ, PROT_WRITE}; -use std::marker::PhantomData; -use std::mem::size_of; -use std::ptr::null; - -pub const BUCKET_BYTES: usize = 4096; - -#[derive(Debug)] -pub struct NodeId { - pub bucket_id: BucketId, - pub slot: BucketSlot, -} - -#[test] -fn size_of_node_id() { - assert_eq!(std::mem::size_of::>(), 4); -} - -impl Clone for NodeId { - fn clone(&self) -> Self { - *self - } -} - -impl Copy for NodeId {} - -impl NodeId { - fn next_slot(&self) -> Self { - NodeId { - bucket_id: self.bucket_id, - slot: self.slot.increment(), - } - } -} - -impl PartialEq for NodeId { - fn eq(&self, other: &Self) -> bool { - self.bucket_id == other.bucket_id && self.slot == other.slot - } -} - -impl Eq for NodeId {} - -#[derive(Debug)] -#[repr(transparent)] -pub struct BucketId { - value: u16, - _phantom: PhantomData, -} - -#[test] -fn size_of_bucket_id() { - assert_eq!(std::mem::size_of::>(), 2); -} - -impl Clone for BucketId { - fn clone(&self) -> Self { - *self - } -} - -impl Copy for BucketId {} - -impl PartialEq for BucketId { - fn eq(&self, other: &Self) -> bool { - self.value == other.value - } -} - -impl Eq for BucketId {} - -impl BucketId { - fn from_u16(value: u16) -> Self { - BucketId { - value, - _phantom: PhantomData::default(), - } - } -} - -#[derive(Debug)] -#[repr(transparent)] -pub struct BucketSlot { - value: u8, - _phantom: PhantomData, -} - -#[test] -fn size_of_bucket_slot() { - assert_eq!(std::mem::size_of::>(), 1); -} - -impl Clone for BucketSlot { - fn clone(&self) -> Self { - *self - } -} - -impl Copy for BucketSlot {} - -impl PartialEq for BucketSlot { - fn eq(&self, other: &Self) -> bool { - self.value == other.value - } -} - -impl Eq for BucketSlot {} - -impl BucketSlot { - #[allow(dead_code)] - fn from_u8(value: u8) -> Self { - BucketSlot { - value, - _phantom: PhantomData::default(), - } - } - - fn increment(&self) -> Self { - BucketSlot { - value: self.value + 1, - _phantom: PhantomData::default(), - } - } -} - -pub struct Buckets { - buckets: Vec, - // free_1node_slots: Vec>, -} - -impl Buckets { - // fn find_space_for(&mut self, nodes: u8) -> Result, ()> {} - - pub fn add(&mut self) -> Result, ()> { - let num_buckets = self.buckets.len(); - - if num_buckets <= u16::MAX as usize { - let bucket_id = BucketId::from_u16(num_buckets as u16); - let bucket = Bucket::default(); - - self.buckets.push(bucket); - - Ok(bucket_id) - } else { - Err(()) - } - } - - fn get_unchecked<'a, T: Sized>(&'a self, node_id: NodeId) -> &'a T { - unsafe { - self.buckets - .get(node_id.bucket_id.value as usize) - .unwrap() - .get_unchecked(node_id.slot.value) - } - } - - pub fn get<'a, T: Sized>(&'a self, node_id: NodeId) -> Option<&'a T> { - self.buckets - .get(node_id.bucket_id.value as usize) - .and_then(|bucket| bucket.get(node_id.slot)) - } -} - -struct Bucket { - #[allow(dead_code)] - next_unused_slot: u16, - first_slot: *mut [u8; 16], -} - -impl Bucket { - /// If there's room left in the bucket, adds the item and returns - /// the slot where it was put. If there was no room left, returns Err(()). - #[allow(dead_code)] - pub fn add(&mut self, node: T) -> Result, ()> { - // It's only safe to store this as a *const T if T is 16 bytes. - // This is designed to be used exclusively with 16-byte nodes! - debug_assert_eq!(size_of::(), 16); - - // Once next_unused_slot exceeds u8::MAX, we have no room left. - if self.next_unused_slot <= u8::MAX as u16 { - let chosen_slot = self.next_unused_slot as u8; - - unsafe { self.put_unchecked(node, chosen_slot) }; - self.next_unused_slot += 1; - - Ok(BucketSlot::from_u8(chosen_slot)) - } else { - // No room left! - Err(()) - } - } - - /// If the given slot is available, inserts the given node into it. - /// Otherwise, returns the node that was in the already-occupied slot. - #[allow(dead_code)] - pub fn insert(&mut self, node: T, slot: BucketSlot) -> Result<(), &T> { - // It's only safe to use this if T is 16 bytes. - // This is designed to be used exclusively with 16-byte nodes! - debug_assert_eq!(size_of::(), 16); - - let slot = slot.value; - - unsafe { - if self.is_available(slot) { - self.put_unchecked(node, slot); - - Ok(()) - } else { - Err(self.get_unchecked(slot)) - } - } - } - - pub fn get<'a, T: Sized>(&'a self, slot: BucketSlot) -> Option<&'a T> { - // It's only safe to store this as a *const T if T is 16 bytes. - // This is designed to be used exclusively with 16-byte nodes! - debug_assert_eq!(size_of::(), 16); - - unsafe { - let slot_ptr = self.first_slot.offset(slot.value as isize) as *const T; - let value: &[u8; 16] = &*(slot_ptr as *const [u8; 16]); - - if *value != [0; 16] { - Some(&*(value as *const [u8; 16] as *const T)) - } else { - None - } - } - } - - unsafe fn put_unchecked(&mut self, node: T, slot: u8) { - // It's only safe to store this as a *const T if T is 16 bytes. - // This is designed to be used exclusively with 16-byte nodes! - debug_assert_eq!(size_of::(), 16); - - let slot_ptr = self.first_slot.offset(slot as isize) as *mut T; - - *slot_ptr = node; - } - - unsafe fn get_unchecked(&self, slot: u8) -> &T { - &*(self.first_slot.offset(slot as isize) as *const T) - } - - // A slot is available iff its bytes are all zeroes - unsafe fn is_available(&self, slot: u8) -> bool { - let slot_ptr = self.first_slot.offset(slot as isize) as *const [u8; 16]; - - *slot_ptr == [0; 16] - } -} - -impl Default for Bucket { - fn default() -> Self { - let first_slot = if page_size::get() == 4096 { - unsafe { - // mmap exactly one memory page (4096 bytes) - mmap( - null::() as *mut c_void, - BUCKET_BYTES, - PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS, - 0, - 0, - ) - } - } else { - // Somehow the page size is not 4096 bytes, so fall back on calloc. - // (We use calloc over malloc because we rely on the bytes having - // been zeroed to tell which slots are available.) - unsafe { calloc(1, BUCKET_BYTES) } - } as *mut [u8; 16]; - - Bucket { - next_unused_slot: 0, - first_slot, - } - } -} - -impl Drop for Bucket { - fn drop(&mut self) { - if page_size::get() == 4096 { - unsafe { - munmap(self.first_slot as *mut c_void, BUCKET_BYTES); - } - } else { - unsafe { - free(self.first_slot as *mut c_void); - } - } - } -} - -#[derive(Debug)] -pub struct BucketStr { - first_node_id: NodeId<()>, - first_segment_len: u8, -} - -#[test] -fn size_of_bucket_str() { - assert_eq!(std::mem::size_of::>(), 4); -} - -/// A non-empty list inside a bucket. It takes 4B of memory. -/// -/// This is internally represented as an array of at most 255 nodes, which -/// can grow to 256+ nodes by having the last nodeent be a linked list Cons -/// cell which points to another such backing array which has more nodes. -/// -/// In practice, these will almost be far below 256 nodes, but in theory -/// they can be enormous in length thanks to the linked list fallback. -/// -/// Since these are non-empty lists, we need separate variants for collections -/// that can be empty, e.g. EmptyRecord and EmptyList. In contrast, we don't -/// need an EmptyList or EmptyWhen, since although those use BucketList -/// to store their branches, having zero branches is syntactically invalid. -/// Same with Call and Closure, since all functions must have 1+ arguments. -#[derive(Debug)] -pub struct BucketList { - first_node_id: BucketId, - first_node_sl: BucketSlot, - first_segment_len: u8, -} - -#[test] -fn size_of_bucket_list() { - assert_eq!(std::mem::size_of::>(), 4); -} - -impl<'a, T: 'a + Sized> BucketList { - /// If given a first_segment_len of 0, that means this is a BucketList - /// consisting of 256+ nodes. The first 255 are stored in the usual - /// array, and then there's one more nodeent at the end which continues - /// the list with a new length and NodeId value. BucketList iterators - /// automatically do these jumps behind the scenes when necessary. - pub fn new(first_node_id: NodeId, first_segment_len: u8) -> Self { - BucketList { - first_segment_len, - first_node_id: first_node_id.bucket_id, - first_node_sl: first_node_id.slot, - } - } - - pub fn into_iter(self, buckets: &'a Buckets) -> impl Iterator { - self.bucket_list_iter(buckets) - } - - /// Private version of into_iter which exposes the implementation detail - /// of BucketListIter. We don't want that struct to be public, but we - /// actually do want to have this separate function for code reuse - /// in the iterator's next() method. - fn bucket_list_iter(&self, buckets: &'a Buckets) -> BucketListIter<'a, T> { - let first_segment_len = self.first_segment_len; - let continues_with_cons = first_segment_len == 0; - let len_remaining = if continues_with_cons { - // We have 255 nodes followed by a Cons cell continuing the list. - u8::MAX - } else { - first_segment_len - }; - - BucketListIter { - continues_with_cons, - len_remaining, - bucket_id: self.first_node_id, - slot: self.first_node_sl, - buckets, - } - } -} - -struct BucketListIter<'a, T: Sized> { - bucket_id: BucketId, - slot: BucketSlot, - len_remaining: u8, - continues_with_cons: bool, - buckets: &'a Buckets, -} - -impl<'a, T: Sized> Iterator for BucketListIter<'a, T> -where - T: 'a, -{ - type Item = &'a T; - - fn next(&mut self) -> Option { - match self.len_remaining { - 0 => match self.continues_with_cons { - // We're done! This is by far the most common case, so we put - // it first to avoid branch mispredictions. - false => None, - // We need to continue with a Cons cell. - true => { - let node_id = NodeId { - bucket_id: self.bucket_id, - slot: self.slot, - } - .next_slot(); - - // Since we have continues_with_cons set, the next slot - // will definitely be occupied with a BucketList struct. - let node = self.buckets.get_unchecked(node_id); - let next_list = unsafe { &*(node as *const T as *const BucketList) }; - - // Replace the current iterator with an iterator into that - // list, and then continue with next() on that iterator. - let next_iter = next_list.bucket_list_iter(self.buckets); - - self.bucket_id = next_iter.bucket_id; - self.slot = next_iter.slot; - self.len_remaining = next_iter.len_remaining; - self.continues_with_cons = next_iter.continues_with_cons; - - self.next() - } - }, - 1 => { - self.len_remaining = 0; - - // Don't advance the node pointer's slot, because that might - // advance past the end of the bucket! - - Some(self.buckets.get_unchecked(NodeId { - bucket_id: self.bucket_id, - slot: self.slot, - })) - } - len_remaining => { - // Get the current node - let node_id = NodeId { - bucket_id: self.bucket_id, - slot: self.slot, - }; - let node = self.buckets.get_unchecked(node_id); - - // Advance the node pointer to the next slot in the current bucket - self.slot = self.slot.increment(); - self.len_remaining = len_remaining - 1; - - Some(node) - } - } - } -} diff --git a/editor/src/pool.rs b/editor/src/pool.rs new file mode 100644 index 0000000000..28a2a44e07 --- /dev/null +++ b/editor/src/pool.rs @@ -0,0 +1,333 @@ +/// A pool of 32-byte nodes. The node value 0 is reserved for the pool's +/// use, and valid nodes may never have that value. +/// +/// Internally, the pool is divided into pages of 4096 bytes. It stores nodes +/// into one page at a time, and when it runs out, it uses mmap to reserve an +/// anonymous memory page in which to store nodes. +/// +/// Since nodes are 32 bytes, one page can store 128 nodes; you can access a +/// particular node by its NodeId, which is an opaque wrapper around a pointer. +/// +/// Pages also use the node value 0 (all 0 bits) to mark nodes as unoccupied. +/// This is important for performance. +use libc::{c_void, calloc, free, mmap, munmap, MAP_ANONYMOUS, MAP_PRIVATE, PROT_READ, PROT_WRITE}; +use std::mem::size_of; +use std::ptr::null; + +pub const NODE_SIZE: usize = 32; + +// Pages are an internal concept which never leave this module. +const PAGE_BYTES: usize = 4096; +const NODES_PER_PAGE: usize = PAGE_BYTES / NODE_SIZE; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct NodeId(*const T); + +pub struct Pool { + pages: Vec, + // free_1node_slots: Vec>, +} + +impl Pool { + // fn find_space_for(&mut self, nodes: u8) -> Result, ()> {} + + pub fn add(&mut self) -> Result, ()> { + let num_pages = self.buckets.len(); + + match self.pages.last() {} + + if self.next_unused_node.offset_from(self.first_node) < NODES_PER_PAGE { + let bucket = Page::default(); + + self.buckets.push(bucket); + + Ok(NodeId(bucket.first_node as *const T)) + } else { + Err(()) + } + } + + fn get_unchecked<'a, T: Sized>(&'a self, node_id: NodeId) -> &'a T { + unsafe { + self.buckets + .get(node_id.bucket_id.value as usize) + .unwrap() + .get_unchecked(node_id.node.value) + } + } + + pub fn get<'a, T: Sized>(&'a self, node_id: NodeId) -> Option<&'a T> { + self.buckets + .get(node_id.bucket_id.value as usize) + .and_then(|bucket| bucket.get(node_id.node)) + } +} + +struct Page { + #[allow(dead_code)] + next_unused_node: *const [u8; NODE_SIZE], + first_node: *mut [u8; NODE_SIZE], +} + +impl Page { + /// If there's room left in the bucket, adds the item and returns + /// the node where it was put. If there was no room left, returns Err(()). + #[allow(dead_code)] + pub fn add(&mut self, node: T) -> Result, ()> { + // It's only safe to store this as a *const T if T is the size of a node. + debug_assert_eq!(size_of::(), NODE_SIZE); + + // Once next_unused_node exceeds NODES_PER_PAGE, we have no room left. + if self.next_unused_node <= NODES_PER_PAGE { + let chosen_node = self.next_unused_node; + + unsafe { *chosen_node = node }; + self.next_unused_node = self.next_unused_node.add(1); + + Ok(NodeId(chosen_node)) + } else { + // No room left! + Err(()) + } + } + + /// If the given node is available, inserts the given node into it. + /// Otherwise, returns the node that was in the already-occupied node. + #[allow(dead_code)] + pub fn insert(&mut self, node: T, node: NodeId) -> Result<(), &T> { + // It's only safe to store this as a *const T if T is the size of a node. + debug_assert_eq!(size_of::(), NODE_SIZE); + + let node = node.0; + + unsafe { + if self.is_available(node) { + self.put_unchecked(node, node); + + Ok(()) + } else { + Err(self.get_unchecked(node)) + } + } + } + + pub fn get<'a, T: Sized>(&'a self, node: NodeId) -> Option<&'a T> { + // It's only safe to store this as a *const T if T is the size of a node. + debug_assert_eq!(size_of::(), NODE_SIZE); + + unsafe { + let node_ptr = self.first_node.offset(node.value as isize) as *const T; + let value: &[u8; NODE_SIZE] = &*(node_ptr as *const [u8; NODE_SIZE]); + + if *value != [0; NODE_SIZE] { + Some(&*(value as *const [u8; NODE_SIZE] as *const T)) + } else { + None + } + } + } + + unsafe fn get_unchecked(&self, node: u8) -> &T { + &*(self.first_node.offset(node as isize) as *const T) + } + + // A node is available iff its bytes are all zeroes + unsafe fn is_available(&self, node_id: NodeId) -> bool { + debug_assert_eq!(size_of::(), NODE_SIZE); + + *node_id.0 == [0; NODE_SIZE] + } +} + +impl Default for Page { + fn default() -> Self { + let first_node = if page_size::get() == 4096 { + unsafe { + // mmap exactly one memory page (4096 bytes) + mmap( + null::() as *mut c_void, + PAGE_BYTES, + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, + 0, + 0, + ) + } + } else { + // Somehow the page size is not 4096 bytes, so fall back on calloc. + // (We use calloc over malloc because we rely on the bytes having + // been zeroed to tell which nodes are available.) + unsafe { calloc(1, PAGE_BYTES) } + } as *mut [u8; NODE_SIZE]; + + Page { + next_unused_node: first_node, + first_node, + } + } +} + +impl Drop for Page { + fn drop(&mut self) { + if page_size::get() == 4096 { + unsafe { + munmap(self.first_node as *mut c_void, PAGE_BYTES); + } + } else { + unsafe { + free(self.first_node as *mut c_void); + } + } + } +} + +#[derive(Debug)] +pub struct PageStr { + first_node_id: NodeId<()>, + first_segment_len: u8, +} + +#[test] +fn size_of_bucket_str() { + assert_eq!(std::mem::size_of::>(), 4); +} + +/// A non-empty list inside a bucket. It takes 4B of memory. +/// +/// This is internally represented as an array of at most 255 nodes, which +/// can grow to 256+ nodes by having the last nodeent be a linked list Cons +/// cell which points to another such backing array which has more nodes. +/// +/// In practice, these will almost be far below 256 nodes, but in theory +/// they can be enormous in length thanks to the linked list fallback. +/// +/// Since these are non-empty lists, we need separate variants for collections +/// that can be empty, e.g. EmptyRecord and EmptyList. In contrast, we don't +/// need an EmptyList or EmptyWhen, since although those use PageList +/// to store their branches, having zero branches is syntactically invalid. +/// Same with Call and Closure, since all functions must have 1+ arguments. +#[derive(Debug)] +pub struct PageList { + first_node_id: NodeId, + first_segment_len: u8, +} + +#[test] +fn size_of_bucket_list() { + assert_eq!(std::mem::size_of::>(), 4); +} + +impl<'a, T: 'a + Sized> PageList { + /// If given a first_segment_len of 0, that means this is a PageList + /// consisting of 256+ nodes. The first 255 are stored in the usual + /// array, and then there's one more nodeent at the end which continues + /// the list with a new length and NodeId value. PageList iterators + /// automatically do these jumps behind the scenes when necessary. + pub fn new(first_node_id: NodeId, first_segment_len: u8) -> Self { + PageList { + first_segment_len, + first_node_id: first_node_id.bucket_id, + first_node_sl: first_node_id.node, + } + } + + pub fn into_iter(self, buckets: &'a Pages) -> impl Iterator { + self.bucket_list_iter(buckets) + } + + /// Private version of into_iter which exposes the implementation detail + /// of PageListIter. We don't want that struct to be public, but we + /// actually do want to have this separate function for code reuse + /// in the iterator's next() method. + fn bucket_list_iter(&self, buckets: &'a Pages) -> PageListIter<'a, T> { + let first_segment_len = self.first_segment_len; + let continues_with_cons = first_segment_len == 0; + let len_remaining = if continues_with_cons { + // We have 255 nodes followed by a Cons cell continuing the list. + u8::MAX + } else { + first_segment_len + }; + + PageListIter { + continues_with_cons, + len_remaining, + bucket_id: self.first_node_id, + node: self.first_node_sl, + buckets, + } + } +} + +struct PageListIter<'a, T: Sized> { + current_node_id: NodeId, + len_remaining: u8, + continues_with_cons: bool, + buckets: &'a Pages, +} + +impl<'a, T: Sized> Iterator for PageListIter<'a, T> +where + T: 'a, +{ + type Item = &'a T; + + fn next(&mut self) -> Option { + match self.len_remaining { + 0 => match self.continues_with_cons { + // We're done! This is by far the most common case, so we put + // it first to avoid branch mispredictions. + false => None, + // We need to continue with a Cons cell. + true => { + let node_id = NodeId { + bucket_id: self.bucket_id, + node: self.node, + } + .next_node(); + + // Since we have continues_with_cons set, the next node + // will definitely be occupied with a PageList struct. + let node = self.buckets.get_unchecked(node_id); + let next_list = unsafe { &*(node as *const T as *const PageList) }; + + // Replace the current iterator with an iterator into that + // list, and then continue with next() on that iterator. + let next_iter = next_list.bucket_list_iter(self.buckets); + + self.bucket_id = next_iter.bucket_id; + self.node = next_iter.node; + self.len_remaining = next_iter.len_remaining; + self.continues_with_cons = next_iter.continues_with_cons; + + self.next() + } + }, + 1 => { + self.len_remaining = 0; + + // Don't advance the node pointer's node, because that might + // advance past the end of the bucket! + + Some(self.buckets.get_unchecked(NodeId { + bucket_id: self.bucket_id, + node: self.node, + })) + } + len_remaining => { + // Get the current node + let node_id = NodeId { + bucket_id: self.bucket_id, + node: self.node, + }; + let node = self.buckets.get_unchecked(node_id); + + // Advance the node pointer to the next node in the current bucket + self.node = self.node.increment(); + self.len_remaining = len_remaining - 1; + + Some(node) + } + } + } +} From ef45e77a35a2f4f70cbc638340fd42698911c6bf Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 8 Dec 2020 22:16:14 -0500 Subject: [PATCH 02/20] Add editor::pool --- editor/src/ast.rs | 294 ++++++++++++-------------------- editor/src/lib.rs | 2 +- editor/src/pool.rs | 409 ++++++++++++++++++++++----------------------- 3 files changed, 312 insertions(+), 393 deletions(-) diff --git a/editor/src/ast.rs b/editor/src/ast.rs index 35aa12bd04..63c58eff93 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -1,4 +1,4 @@ -use crate::bucket::{BucketList, BucketStr, NodeId}; +use crate::pool::{NodeId, PoolStr, PoolVec}; use arraystring::{typenum::U30, ArrayString}; use roc_can::def::Annotation; use roc_can::expr::{Field, Recursive}; @@ -6,7 +6,6 @@ use roc_module::ident::Lowercase; use roc_module::low_level::LowLevel; use roc_module::operator::CalledVia; use roc_module::symbol::Symbol; -use roc_problem::can::RuntimeError; use roc_types::subs::Variable; use roc_types::types::Alias; @@ -42,27 +41,20 @@ pub enum Expr2 { }, /// A number literal (without a dot) containing underscores NumWithUnderscores { - number: i64, // 8B - var: Variable, // 4B - text: NodeId, // 8B + number: i64, // 8B + var: Variable, // 4B + text: NodeId, // 8B }, /// A float literal (with a dot) containing underscores FloatWithUnderscores { - number: f64, // 8B - var: Variable, // 4B - text: NodeId, // 8B + number: f64, // 8B + var: Variable, // 4B + text: NodeId, // 8B }, /// string literals of length up to 30B SmallStr(ArrayString), // 31B - /// string literals of length up to 4094B - MedStr { - str: NodeId, // 8B - }, - /// string literals of length over 4094B, but requires calling malloc/free - BigStr { - pointer: *const u8, // 8B - len: u32, // 4B, meaning maximum string literal size of 4GB. Could theoretically fit 7B here, which would get closer to the full isize::MAX - }, + /// string literals of length 31B or more + Str(NodeId), // 8B // Lookups Var(Symbol), // 8B @@ -73,96 +65,96 @@ pub enum Expr2 { elem_var: Variable, // 4B }, List { - list_var: Variable, // 4B - required for uniqueness of the list - elem_var: Variable, // 4B - elems: BucketList, // 9B + list_var: Variable, // 4B - required for uniqueness of the list + elem_var: Variable, // 4B + first_elem: PoolVec, // 16B }, If { - cond_var: Variable, // 4B - expr_var: Variable, // 4B - branches: BucketList<(Expr2, Expr2)>, // 9B - final_else: NodeId, // 8B - }, - When { - cond_var: Variable, // 4B - expr_var: Variable, // 4B - branches: BucketList, // 9B - cond: NodeId, // 8B - }, - LetRec { - // TODO need to make this Alias type here bucket-friendly, which will be hard! - aliases: BucketList<(Symbol, Alias)>, // 9B - defs: BucketList, // 9B - body_var: Variable, // 4B - body_id: NodeId, // 8B - }, - LetNonRec { - // TODO need to make this Alias type here bucket-friendly, which will be hard! - aliases: BucketList<(Symbol, Alias)>, // 9B - def_id: NodeId, // 8B - body_id: NodeId, // 8B - body_var: Variable, // 4B - }, - Call { - /// NOTE: the first elem in this list is the expression and its variable. - /// The others are arguments. This is because we didn't have room for - /// both the expr and its variable otherwise. - expr_and_args: BucketList<(Variable, NodeId)>, // 9B - fn_var: Variable, // 4B - closure_var: Variable, // 4B - /// Cached outside expr_and_args so we don't have to potentially - /// traverse that whole linked list chain to count all the args. - arity: usize, // 8B - could make this smaller if need be - called_via: CalledVia, // 2B - }, - RunLowLevel { - op: LowLevel, // 1B - args: BucketList<(Variable, NodeId)>, // 9B - ret_var: Variable, // 4B - }, - Closure { - captured_symbols: BucketList<(Symbol, Variable)>, // 9B - args: BucketList<(Variable, NodeId)>, // 9B - recursive: Recursive, // 1B - extra: NodeId, // 8B + cond_var: Variable, // 4B + expr_var: Variable, // 4B + branches: PoolVec<(Expr2, Expr2)>, // 16B + final_else: NodeId, // 8B }, + // When { + // cond_var: Variable, // 4B + // expr_var: Variable, // 4B + // branches: PoolVec, // 9B + // cond: NodeId, // 8B + // }, + // LetRec { + // // TODO need to make this Alias type here page-friendly, which will be hard! + // aliases: PoolVec<(Symbol, Alias)>, // 9B + // defs: PoolVec, // 9B + // body_var: Variable, // 4B + // body_id: NodeId, // 8B + // }, + // LetNonRec { + // // TODO need to make this Alias type here page-friendly, which will be hard! + // aliases: PoolVec<(Symbol, Alias)>, // 9B + // def_id: NodeId, // 8B + // body_id: NodeId, // 8B + // body_var: Variable, // 4B + // }, + // Call { + // /// NOTE: the first elem in this list is the expression and its variable. + // /// The others are arguments. This is because we didn't have room for + // /// both the expr and its variable otherwise. + // expr_and_args: PoolVec<(Variable, NodeId)>, // 9B + // fn_var: Variable, // 4B + // closure_var: Variable, // 4B + // /// Cached outside expr_and_args so we don't have to potentially + // /// traverse that whole linked list chain to count all the args. + // arity: usize, // 8B - could make this smaller if need be + // called_via: CalledVia, // 2B + // }, + // RunLowLevel { + // op: LowLevel, // 1B + // args: PoolVec<(Variable, NodeId)>, // 9B + // ret_var: Variable, // 4B + // }, + // Closure { + // captured_symbols: PoolVec<(Symbol, Variable)>, // 9B + // args: PoolVec<(Variable, NodeId)>, // 9B + // recursive: Recursive, // 1B + // extra: NodeId, // 8B + // }, // Product Types - Record { - record_var: Variable, // 4B - fields: BucketList<(BucketStr, Variable, NodeId)>, // 9B - }, + // Record { + // record_var: Variable, // 4B + // fields: PoolVec<(PoolStr, Variable, NodeId)>, // 9B + // }, /// Empty record constant - EmptyRecord, - /// Look up exactly one field on a record, e.g. (expr).foo. - Access { - field: NodeId, // 8B - expr: NodeId, // 8B - vars: NodeId, // 8B - }, + // EmptyRecord, + // /// Look up exactly one field on a record, e.g. (expr).foo. + // Access { + // field: NodeId, // 8B + // expr: NodeId, // 8B + // vars: NodeId, // 8B + // }, - /// field accessor as a function, e.g. (.foo) expr - Accessor { - record_vars_id: NodeId, // 8B - function_var: Variable, // 4B - closure_var: Variable, // 4B - field_id: NodeId, // 8B - }, - Update { - symbol: Symbol, // 8B - updates: BucketList<(Lowercase, Field)>, // 9B - vars_id: NodeId, // 8B - }, + // /// field accessor as a function, e.g. (.foo) expr + // Accessor { + // record_vars_id: NodeId, // 8B + // function_var: Variable, // 4B + // closure_var: Variable, // 4B + // field_id: NodeId, // 8B + // }, + // Update { + // symbol: Symbol, // 8B + // updates: PoolVec<(Lowercase, Field)>, // 9B + // vars_id: NodeId, // 8B + // }, // Sum Types - Tag { - // NOTE: A BucketStr node is a 2B length and then 14B bytes, - // plus more bytes in adjacent nodes if necessary. Thus we have - // a hard cap of 4094 bytes as the maximum length of tags and fields. - name_id: NodeId, // 8B - variant_var: Variable, // 4B - ext_var: Variable, // 4B - arguments: BucketList<(Variable, NodeId)>, // 9B - }, + // Tag { + // // NOTE: A PoolStr node is a 2B length and then 14B bytes, + // // plus more bytes in adjacent nodes if necessary. Thus we have + // // a hard cap of 4094 bytes as the maximum length of tags and fields. + // name_id: NodeId, // 8B + // variant_var: Variable, // 4B + // ext_var: Variable, // 4B + // arguments: PoolVec<(Variable, NodeId)>, // 9B + // }, // Compiles, but will crash if reached RuntimeError(/* TODO make a version of RuntimeError that fits in 15B */), @@ -206,8 +198,8 @@ pub struct Def { pub pattern: NodeId, // 3B pub expr: NodeId, // 3B // TODO maybe need to combine these vars behind a pointer? - pub expr_var: Variable, // 4B - pub pattern_vars: BucketList<(Symbol, Variable)>, // 4B + pub expr_var: Variable, // 4B + pub pattern_vars: PoolVec<(Symbol, Variable)>, // 4B // TODO how big is an annotation? What about an Option? pub annotation: Option, // ??? } @@ -239,7 +231,7 @@ pub struct AccessVars { } /// This is 32B, so it fits in a Node slot. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[derive(Debug)] pub struct ClosureExtra { name: Symbol, // 8B body: NodeId, // 8B @@ -251,110 +243,38 @@ pub struct ClosureExtra { #[derive(Debug)] pub struct WhenBranch { - pub patterns: BucketList, // 4B + pub patterns: PoolVec, // 4B pub body: NodeId, // 3B pub guard: Option>, // 4B } #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct PatternId { - /// TODO: PatternBucketId - bucket_id: ExprBucketId, - /// TODO: PatternBucketSlot - slot: ExprBucketSlot, -} - -// Each bucket has metadata and slots. -// The metadata determines things like which slots are free. -#[derive(Debug)] -pub struct ExprBucket { - // We can store this as a u8 because whenever we create a bucket, we - // always fill at least one slot. So there will never be 256 unused slots - // remaining; the most there will ever be will be 255. - // - // Note that there can be "holes" in this as we remove nodes; those - // are recorded in the containing struct, not here. - // - // Also note that we can derive from this the next unused slot. - unused_slots_remaining: u8, - slots: Box, -} - -pub struct Exprs { - // Whenever we free a slot of a particular size, we make a note of it - // here, so we can reuse it later. This can lead to poor data locality - // over time, but the alternative is memory fragmentation and ever-growing - // memory usage. We could in theory go up to free_128node_slots, but in - // practice it seems unlikely that it would be worth the bookkeeping - // effort to go that high. - // - // TODO: this could be refactored Into `free_slots: [5; Vec]` - // where (2 ^ index) is the size node in that slot. It's less - // self-documenting but might allow for better code reuse. - pub free_1node_slots: Vec, - pub free_2node_slots: Vec, - pub free_4node_slots: Vec, - pub free_8node_slots: Vec, - pub free_16node_slots: Vec, - // Note that empty_buckets is equivalent to free_256node_slots - it means - // the entire bucket is empty, at which point we can fill it with - // whatever we please. - pub empty_buckets: Vec, - pub buckets: Vec, -} - -// Each bucket has 128 slots. Each slot holds one 32B node -// This means each bucket is 4096B, which is the size of a memory page -// on typical systems where the compiler will be run. -// -// Nice things about this system include: -// * Allocating a new bucket is as simple as asking the OS for a memory page. -// * Since each node is 32B, each node's memory address will be a multiple of 16. -// * Thanks to the free lists and our consistent chunk sizes, we should -// end up with very little fragmentation. -// * Finding a slot for a given node should be very fast: see if the relevant -// free list has any openings; if not, try the next size up. -// -// Less nice things include: -// * This system makes it very hard to ever give a page back to the OS. -// We could try doing the Mesh Allocator strategy: whenever we allocate -// something, assign it to a random slot in the bucket, and then periodically -// try to merge two pages into one (by locking and remapping them in the OS) -// and then returning the redundant physical page back to the OS. This should -// work in theory, but is pretty complicated, and we'd need to schedule it. -// Keep in mind that we can't use the Mesh Allocator itself because it returns -// usize pointers, which would be too big for us to have 16B nodes. -// On the plus side, we could be okay with higher memory usage early on, -// and then later use the Mesh strategy to reduce long-running memory usage. -type ExprBucketSlots = [Expr2; 128]; - -#[test] -fn size_of_expr_bucket() { - assert_eq!( - std::mem::size_of::(), - crate::bucket::BUCKET_BYTES - ); + /// TODO: PatternPoolId + page_id: ExprPoolId, + /// TODO: PatternPoolSlot + slot: ExprPoolSlot, } #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct PatId { - bucket_id: ExprBucketId, // TODO PatBucketId - slot: ExprBucketSlot, // TODO PatBucketSlot + page_id: ExprPoolId, // TODO PatPoolId + slot: ExprPoolSlot, // TODO PatPoolSlot } #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct ExprId { - bucket_id: ExprBucketId, - slot: ExprBucketSlot, + page_id: ExprPoolId, + slot: ExprPoolSlot, } -// We have a maximum of 65K buckets. +// We have a maximum of 65K pages. #[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub struct ExprBucketId(u16); +pub struct ExprPoolId(u16); -/// Each of these is the index of one 16B node inside a bucket's 4096B +/// Each of these is the index of one 16B node inside a page's 4096B #[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub struct ExprBucketSlot(u8); +pub struct ExprPoolSlot(u8); #[test] fn size_of_expr() { diff --git a/editor/src/lib.rs b/editor/src/lib.rs index 9d921bd8fa..10b2fca8ec 100644 --- a/editor/src/lib.rs +++ b/editor/src/lib.rs @@ -27,7 +27,7 @@ use winit::event::{Event, ModifiersState}; use winit::event_loop::ControlFlow; pub mod ast; -pub mod bucket; +pub mod pool; mod buffer; pub mod file; mod keyboard_input; diff --git a/editor/src/pool.rs b/editor/src/pool.rs index 28a2a44e07..7ba8651b7b 100644 --- a/editor/src/pool.rs +++ b/editor/src/pool.rs @@ -10,7 +10,7 @@ /// /// Pages also use the node value 0 (all 0 bits) to mark nodes as unoccupied. /// This is important for performance. -use libc::{c_void, calloc, free, mmap, munmap, MAP_ANONYMOUS, MAP_PRIVATE, PROT_READ, PROT_WRITE}; +use libc::{c_void, MAP_ANONYMOUS, MAP_PRIVATE, PROT_READ, PROT_WRITE}; use std::mem::size_of; use std::ptr::null; @@ -18,7 +18,31 @@ pub const NODE_SIZE: usize = 32; // Pages are an internal concept which never leave this module. const PAGE_BYTES: usize = 4096; -const NODES_PER_PAGE: usize = PAGE_BYTES / NODE_SIZE; +const NODES_PER_PAGE: u8 = (PAGE_BYTES / NODE_SIZE) as u8; + +// Each page has 128 slots. Each slot holds one 32B node +// This means each page is 4096B, which is the size of a memory page +// on typical systems where the compiler will be run. +// +// Nice things about this system include: +// * Allocating a new page is as simple as asking the OS for a memory page. +// * Since each node is 32B, each node's memory address will be a multiple of 16. +// * Thanks to the free lists and our consistent chunk sizes, we should +// end up with very little fragmentation. +// * Finding a slot for a given node should be very fast: see if the relevant +// free list has any openings; if not, try the next size up. +// +// Less nice things include: +// * This system makes it very hard to ever give a page back to the OS. +// We could try doing the Mesh Allocator strategy: whenever we allocate +// something, assign it to a random slot in the page, and then periodically +// try to merge two pages into one (by locking and remapping them in the OS) +// and then returning the redundant physical page back to the OS. This should +// work in theory, but is pretty complicated, and we'd need to schedule it. +// Keep in mind that we can't use the Mesh Allocator itself because it returns +// usize pointers, which would be too big for us to have 16B nodes. +// On the plus side, we could be okay with higher memory usage early on, +// and then later use the Mesh strategy to reduce long-running memory usage. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct NodeId(*const T); @@ -29,114 +53,67 @@ pub struct Pool { } impl Pool { + /// Returns a pool with a capacity equal to the given number of 4096-byte pages. + // pub fn with_pages(pages: usize) { + // todo!(); + // } + // fn find_space_for(&mut self, nodes: u8) -> Result, ()> {} - pub fn add(&mut self) -> Result, ()> { - let num_pages = self.buckets.len(); + pub fn add(&mut self, node: T) -> NodeId { + // It's only safe to store this as a *mut T if T is the size of a node. + debug_assert_eq!(size_of::(), NODE_SIZE); - match self.pages.last() {} + match self.pages.last_mut() { + Some(page) if page.node_count < NODES_PER_PAGE => Pool::add_to_page(page, node), + _ => { + // This page is either full or doesn't exist, so create a new one. + let mut page = Page::default(); + let node_id = Pool::add_to_page(&mut page, node); - if self.next_unused_node.offset_from(self.first_node) < NODES_PER_PAGE { - let bucket = Page::default(); + self.pages.push(page); - self.buckets.push(bucket); - - Ok(NodeId(bucket.first_node as *const T)) - } else { - Err(()) + node_id + } } } - fn get_unchecked<'a, T: Sized>(&'a self, node_id: NodeId) -> &'a T { + /// Reserves the given number of contiguous node slots, and returns + /// the NodeId of the first one. We only allow reserving 2^32 in a row. + fn reserve(&mut self, _nodes: u32) -> NodeId { + todo!("Implement Pool::reserve"); + } + + fn add_to_page(page: &mut Page, node: T) -> NodeId { unsafe { - self.buckets - .get(node_id.bucket_id.value as usize) - .unwrap() - .get_unchecked(node_id.node.value) + let node_ptr = (page.first_node as *const T).offset(page.node_count as isize) as *mut T; + + *node_ptr = node; + + page.node_count += 1; + + NodeId(node_ptr) } } - pub fn get<'a, T: Sized>(&'a self, node_id: NodeId) -> Option<&'a T> { - self.buckets - .get(node_id.bucket_id.value as usize) - .and_then(|bucket| bucket.get(node_id.node)) + pub fn get<'a, T: Sized>(&'a self, node_id: NodeId) -> &'a T { + unsafe { &*node_id.0 } + } + + // A node is available iff its bytes are all zeroes + #[allow(dead_code)] + unsafe fn is_available(&self, node_id: NodeId) -> bool { + debug_assert_eq!(size_of::(), NODE_SIZE); + + let ptr = node_id.0 as *const [u8; NODE_SIZE]; + + *ptr == [0; NODE_SIZE] } } struct Page { - #[allow(dead_code)] - next_unused_node: *const [u8; NODE_SIZE], - first_node: *mut [u8; NODE_SIZE], -} - -impl Page { - /// If there's room left in the bucket, adds the item and returns - /// the node where it was put. If there was no room left, returns Err(()). - #[allow(dead_code)] - pub fn add(&mut self, node: T) -> Result, ()> { - // It's only safe to store this as a *const T if T is the size of a node. - debug_assert_eq!(size_of::(), NODE_SIZE); - - // Once next_unused_node exceeds NODES_PER_PAGE, we have no room left. - if self.next_unused_node <= NODES_PER_PAGE { - let chosen_node = self.next_unused_node; - - unsafe { *chosen_node = node }; - self.next_unused_node = self.next_unused_node.add(1); - - Ok(NodeId(chosen_node)) - } else { - // No room left! - Err(()) - } - } - - /// If the given node is available, inserts the given node into it. - /// Otherwise, returns the node that was in the already-occupied node. - #[allow(dead_code)] - pub fn insert(&mut self, node: T, node: NodeId) -> Result<(), &T> { - // It's only safe to store this as a *const T if T is the size of a node. - debug_assert_eq!(size_of::(), NODE_SIZE); - - let node = node.0; - - unsafe { - if self.is_available(node) { - self.put_unchecked(node, node); - - Ok(()) - } else { - Err(self.get_unchecked(node)) - } - } - } - - pub fn get<'a, T: Sized>(&'a self, node: NodeId) -> Option<&'a T> { - // It's only safe to store this as a *const T if T is the size of a node. - debug_assert_eq!(size_of::(), NODE_SIZE); - - unsafe { - let node_ptr = self.first_node.offset(node.value as isize) as *const T; - let value: &[u8; NODE_SIZE] = &*(node_ptr as *const [u8; NODE_SIZE]); - - if *value != [0; NODE_SIZE] { - Some(&*(value as *const [u8; NODE_SIZE] as *const T)) - } else { - None - } - } - } - - unsafe fn get_unchecked(&self, node: u8) -> &T { - &*(self.first_node.offset(node as isize) as *const T) - } - - // A node is available iff its bytes are all zeroes - unsafe fn is_available(&self, node_id: NodeId) -> bool { - debug_assert_eq!(size_of::(), NODE_SIZE); - - *node_id.0 == [0; NODE_SIZE] - } + first_node: *const [u8; NODE_SIZE], + node_count: u8, } impl Default for Page { @@ -144,7 +121,7 @@ impl Default for Page { let first_node = if page_size::get() == 4096 { unsafe { // mmap exactly one memory page (4096 bytes) - mmap( + libc::mmap( null::() as *mut c_void, PAGE_BYTES, PROT_READ | PROT_WRITE, @@ -157,12 +134,12 @@ impl Default for Page { // Somehow the page size is not 4096 bytes, so fall back on calloc. // (We use calloc over malloc because we rely on the bytes having // been zeroed to tell which nodes are available.) - unsafe { calloc(1, PAGE_BYTES) } + unsafe { libc::calloc(1, PAGE_BYTES) } } as *mut [u8; NODE_SIZE]; Page { - next_unused_node: first_node, first_node, + node_count: 0, } } } @@ -171,163 +148,185 @@ impl Drop for Page { fn drop(&mut self) { if page_size::get() == 4096 { unsafe { - munmap(self.first_node as *mut c_void, PAGE_BYTES); + libc::munmap(self.first_node as *mut c_void, PAGE_BYTES); } } else { unsafe { - free(self.first_node as *mut c_void); + libc::free(self.first_node as *mut c_void); } } } } +/// A string of at most 2^32 bytes, allocated in a pool if it fits in a Page, +/// or using malloc as a fallback if not. Like std::str::String, this has +/// both a length and capacity. #[derive(Debug)] -pub struct PageStr { +pub struct PoolStr { first_node_id: NodeId<()>, - first_segment_len: u8, + len: u32, + cap: u32, } #[test] -fn size_of_bucket_str() { - assert_eq!(std::mem::size_of::>(), 4); +fn pool_str_size() { + assert_eq!(size_of::(), size_of::() + 8); } -/// A non-empty list inside a bucket. It takes 4B of memory. -/// -/// This is internally represented as an array of at most 255 nodes, which -/// can grow to 256+ nodes by having the last nodeent be a linked list Cons -/// cell which points to another such backing array which has more nodes. -/// -/// In practice, these will almost be far below 256 nodes, but in theory -/// they can be enormous in length thanks to the linked list fallback. -/// -/// Since these are non-empty lists, we need separate variants for collections -/// that can be empty, e.g. EmptyRecord and EmptyList. In contrast, we don't -/// need an EmptyList or EmptyWhen, since although those use PageList -/// to store their branches, having zero branches is syntactically invalid. -/// Same with Call and Closure, since all functions must have 1+ arguments. +/// An array of at most 2^32 elements, allocated in a pool if it fits in a Page, +/// or using malloc as a fallback if not. Like std::vec::Vec, this has both +/// a length and capacity. #[derive(Debug)] -pub struct PageList { +pub struct PoolVec { first_node_id: NodeId, - first_segment_len: u8, + len: u32, + cap: u32, } #[test] -fn size_of_bucket_list() { - assert_eq!(std::mem::size_of::>(), 4); +fn pool_vec_size() { + assert_eq!(size_of::>(), size_of::() + 8); } -impl<'a, T: 'a + Sized> PageList { - /// If given a first_segment_len of 0, that means this is a PageList - /// consisting of 256+ nodes. The first 255 are stored in the usual - /// array, and then there's one more nodeent at the end which continues - /// the list with a new length and NodeId value. PageList iterators - /// automatically do these jumps behind the scenes when necessary. - pub fn new(first_node_id: NodeId, first_segment_len: u8) -> Self { - PageList { - first_segment_len, - first_node_id: first_node_id.bucket_id, - first_node_sl: first_node_id.node, +impl<'a, T: 'a + Sized> PoolVec { + /// If given a slice of length > 128, the first 128 nodes will be stored in + /// the usual array, and then there's one more node at the end which + /// continues the list with a new length and NodeId value. PoolVec + /// iterators automatically do these jumps behind the scenes when necessary. + pub fn new>(nodes: I, pool: &mut Pool) -> Self { + debug_assert!(nodes.len() <= u32::MAX as usize); + debug_assert!(size_of::() <= NODE_SIZE); + + let len = nodes.len() as u32; + + if len > 0 { + if len <= NODES_PER_PAGE as u32 { + let first_node_id = pool.reserve(len); + let mut next_node_ptr = first_node_id.0 as *mut T; + + for node in nodes { + unsafe { + *next_node_ptr = node; + + next_node_ptr = next_node_ptr.offset(1); + } + } + + PoolVec { + first_node_id, + len, + cap: len, + } + } else { + let first_node_ptr = unsafe { + // mmap enough memory to hold it + libc::mmap( + null::() as *mut c_void, + len as usize, + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, + 0, + 0, + ) + }; + + PoolVec { + first_node_id: NodeId(first_node_ptr as *const T), + len, + cap: len, + } + } + } else { + PoolVec { + first_node_id: NodeId(std::ptr::null()), + len: 0, + cap: 0, + } } } - pub fn into_iter(self, buckets: &'a Pages) -> impl Iterator { - self.bucket_list_iter(buckets) + pub fn iter(self, pool: &'a Pool) -> impl ExactSizeIterator { + self.pool_list_iter(pool) } /// Private version of into_iter which exposes the implementation detail - /// of PageListIter. We don't want that struct to be public, but we + /// of PoolVecIter. We don't want that struct to be public, but we /// actually do want to have this separate function for code reuse /// in the iterator's next() method. - fn bucket_list_iter(&self, buckets: &'a Pages) -> PageListIter<'a, T> { - let first_segment_len = self.first_segment_len; - let continues_with_cons = first_segment_len == 0; - let len_remaining = if continues_with_cons { - // We have 255 nodes followed by a Cons cell continuing the list. - u8::MAX - } else { - first_segment_len - }; + #[inline(always)] + fn pool_list_iter(&self, pool: &'a Pool) -> PoolVecIter<'a, T> { + PoolVecIter { + _pool: pool, + current_node_id: NodeId(self.first_node_id.0), + len_remaining: self.len, + } + } - PageListIter { - continues_with_cons, - len_remaining, - bucket_id: self.first_node_id, - node: self.first_node_sl, - buckets, + pub fn free(self) { + if self.len <= NODES_PER_PAGE as u32 { + // If this was small enough to fit in a Page, then zero it out. + unsafe { + libc::memset( + self.first_node_id.0 as *mut c_void, + 0, + self.len as usize * NODE_SIZE, + ); + } + + // TODO insert it into the pool's free list + } else { + // This was bigger than a Page, so we mmap'd it. Now we free it! + unsafe { + libc::munmap(self.first_node_id.0 as *mut c_void, self.len as usize); + } } } } -struct PageListIter<'a, T: Sized> { +struct PoolVecIter<'a, T: Sized> { + /// This iterator returns elements which have the same lifetime as the pool + _pool: &'a Pool, current_node_id: NodeId, - len_remaining: u8, - continues_with_cons: bool, - buckets: &'a Pages, + len_remaining: u32, } -impl<'a, T: Sized> Iterator for PageListIter<'a, T> +impl<'a, T: Sized> ExactSizeIterator for PoolVecIter<'a, T> +where + T: 'a, +{ + fn len(&self) -> usize { + self.len_remaining as usize + } +} + +impl<'a, T: Sized> Iterator for PoolVecIter<'a, T> where T: 'a, { type Item = &'a T; fn next(&mut self) -> Option { - match self.len_remaining { - 0 => match self.continues_with_cons { - // We're done! This is by far the most common case, so we put - // it first to avoid branch mispredictions. - false => None, - // We need to continue with a Cons cell. - true => { - let node_id = NodeId { - bucket_id: self.bucket_id, - node: self.node, - } - .next_node(); + let len_remaining = self.len_remaining; - // Since we have continues_with_cons set, the next node - // will definitely be occupied with a PageList struct. - let node = self.buckets.get_unchecked(node_id); - let next_list = unsafe { &*(node as *const T as *const PageList) }; + if len_remaining > 1 { + // Get the current node + let node_ptr = self.current_node_id.0; - // Replace the current iterator with an iterator into that - // list, and then continue with next() on that iterator. - let next_iter = next_list.bucket_list_iter(self.buckets); + // Advance the node pointer to the next node in the current page + self.current_node_id = NodeId(unsafe { node_ptr.offset(1) }); + self.len_remaining = len_remaining - 1; - self.bucket_id = next_iter.bucket_id; - self.node = next_iter.node; - self.len_remaining = next_iter.len_remaining; - self.continues_with_cons = next_iter.continues_with_cons; + Some(unsafe { &*node_ptr }) + } else if len_remaining == 1 { + self.len_remaining = 0; - self.next() - } - }, - 1 => { - self.len_remaining = 0; + // Don't advance the node pointer's node, because that might + // advance past the end of the page! - // Don't advance the node pointer's node, because that might - // advance past the end of the bucket! - - Some(self.buckets.get_unchecked(NodeId { - bucket_id: self.bucket_id, - node: self.node, - })) - } - len_remaining => { - // Get the current node - let node_id = NodeId { - bucket_id: self.bucket_id, - node: self.node, - }; - let node = self.buckets.get_unchecked(node_id); - - // Advance the node pointer to the next node in the current bucket - self.node = self.node.increment(); - self.len_remaining = len_remaining - 1; - - Some(node) - } + Some(unsafe { &*self.current_node_id.0 }) + } else { + // len_remaining was 0 + None } } } From fd47d6ee7133378dac7b72c4f3da282e5ea25215 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 8 Dec 2020 22:16:31 -0500 Subject: [PATCH 03/20] Have NodeId be an index, not a pointer --- editor/src/ast.rs | 26 ++-- editor/src/pool.rs | 293 +++++++++++++++++++++------------------------ 2 files changed, 148 insertions(+), 171 deletions(-) diff --git a/editor/src/ast.rs b/editor/src/ast.rs index 63c58eff93..da28a70b59 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -75,19 +75,19 @@ pub enum Expr2 { branches: PoolVec<(Expr2, Expr2)>, // 16B final_else: NodeId, // 8B }, - // When { - // cond_var: Variable, // 4B - // expr_var: Variable, // 4B - // branches: PoolVec, // 9B - // cond: NodeId, // 8B - // }, - // LetRec { - // // TODO need to make this Alias type here page-friendly, which will be hard! - // aliases: PoolVec<(Symbol, Alias)>, // 9B - // defs: PoolVec, // 9B - // body_var: Variable, // 4B - // body_id: NodeId, // 8B - // }, + When { + cond_var: Variable, // 4B + expr_var: Variable, // 4B + branches: PoolVec, // 9B + cond: NodeId, // 8B + }, + LetRec { + // TODO need to make this Alias type here page-friendly, which will be hard! + aliases: PoolVec<(Symbol, Alias)>, // 9B + defs: PoolVec, // 9B + body_var: Variable, // 4B + body_id: NodeId, // 8B + }, // LetNonRec { // // TODO need to make this Alias type here page-friendly, which will be hard! // aliases: PoolVec<(Symbol, Alias)>, // 9B diff --git a/editor/src/pool.rs b/editor/src/pool.rs index 7ba8651b7b..1c7d13e3fb 100644 --- a/editor/src/pool.rs +++ b/editor/src/pool.rs @@ -11,15 +11,10 @@ /// Pages also use the node value 0 (all 0 bits) to mark nodes as unoccupied. /// This is important for performance. use libc::{c_void, MAP_ANONYMOUS, MAP_PRIVATE, PROT_READ, PROT_WRITE}; +use std::marker::PhantomData; use std::mem::size_of; use std::ptr::null; -pub const NODE_SIZE: usize = 32; - -// Pages are an internal concept which never leave this module. -const PAGE_BYTES: usize = 4096; -const NODES_PER_PAGE: u8 = (PAGE_BYTES / NODE_SIZE) as u8; - // Each page has 128 slots. Each slot holds one 32B node // This means each page is 4096B, which is the size of a memory page // on typical systems where the compiler will be run. @@ -44,148 +39,152 @@ const NODES_PER_PAGE: u8 = (PAGE_BYTES / NODE_SIZE) as u8; // On the plus side, we could be okay with higher memory usage early on, // and then later use the Mesh strategy to reduce long-running memory usage. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct NodeId(*const T); +#[derive(Debug, PartialEq, Eq)] +pub struct NodeId { + index: u32, + _phantom: PhantomData, +} -pub struct Pool { - pages: Vec, +impl Clone for NodeId { + fn clone(&self) -> Self { + NodeId { + index: self.index, + _phantom: PhantomData::default(), + } + } +} + +impl Copy for NodeId {} + +/// S is a slot size; e.g. Pool<[u8; 32]> for a pool of 32-bit slots +pub struct Pool { + nodes: *mut S, + num_nodes: u32, + capacity: u32, // free_1node_slots: Vec>, } -impl Pool { - /// Returns a pool with a capacity equal to the given number of 4096-byte pages. - // pub fn with_pages(pages: usize) { - // todo!(); - // } +impl Pool { + pub fn with_capacity(&mut self, nodes: u32) -> Self { + // round up number of nodes requested to nearest page size in bytes + let bytes_per_page = page_size::get(); + let node_bytes = size_of::() * nodes as usize; + let leftover = node_bytes % bytes_per_page; + let bytes_to_mmap = if leftover == 0 { + node_bytes + } else { + node_bytes + bytes_per_page - leftover + }; - // fn find_space_for(&mut self, nodes: u8) -> Result, ()> {} + let nodes = unsafe { + // mmap anonymous memory pages - that is, contiguous virtual memory + // addresses from the OS which will be lazily translated into + // physical memory one 4096-byte page at a time, once we actually + // try to read or write in that page's address range. + libc::mmap( + null::() as *mut c_void, + bytes_to_mmap, + PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, + 0, + 0, + ) + } as *mut S; - pub fn add(&mut self, node: T) -> NodeId { - // It's only safe to store this as a *mut T if T is the size of a node. - debug_assert_eq!(size_of::(), NODE_SIZE); + // This is our actual capacity, in nodes. + // It might be higher than the requested capacity due to rounding up + // to nearest page size. + let capacity = (bytes_to_mmap / size_of::()) as u32; - match self.pages.last_mut() { - Some(page) if page.node_count < NODES_PER_PAGE => Pool::add_to_page(page, node), - _ => { - // This page is either full or doesn't exist, so create a new one. - let mut page = Page::default(); - let node_id = Pool::add_to_page(&mut page, node); + Pool { + nodes, + num_nodes: 0, + capacity, + } + } - self.pages.push(page); + pub fn add(&mut self, node: T) -> NodeId { + // It's only safe to store this if T is the same size as S. + debug_assert_eq!(size_of::(), size_of::()); - node_id + let index = self.num_nodes; + + if index < self.capacity { + let node_ptr = unsafe { self.nodes.offset(index as isize) } as *mut T; + + unsafe { *node_ptr = node }; + + self.num_nodes = index + 1; + + NodeId { + index, + _phantom: PhantomData::default(), } + } else { + todo!("pool ran out of capacity. TODO reallocate the nodes pointer to map to a bigger space. Can use mremap on Linux, but must memcpy lots of bytes on macOS and Windows."); } } /// Reserves the given number of contiguous node slots, and returns /// the NodeId of the first one. We only allow reserving 2^32 in a row. - fn reserve(&mut self, _nodes: u32) -> NodeId { + fn reserve(&mut self, _nodes: u32) -> NodeId { todo!("Implement Pool::reserve"); } - fn add_to_page(page: &mut Page, node: T) -> NodeId { + pub fn get<'a, T>(&'a self, node_id: NodeId) -> &'a T { unsafe { - let node_ptr = (page.first_node as *const T).offset(page.node_count as isize) as *mut T; + let node_ptr = self.nodes.offset(node_id.index as isize) as *mut T; - *node_ptr = node; - - page.node_count += 1; - - NodeId(node_ptr) + &*node_ptr } } - pub fn get<'a, T: Sized>(&'a self, node_id: NodeId) -> &'a T { - unsafe { &*node_id.0 } - } - // A node is available iff its bytes are all zeroes #[allow(dead_code)] unsafe fn is_available(&self, node_id: NodeId) -> bool { - debug_assert_eq!(size_of::(), NODE_SIZE); + debug_assert_eq!(size_of::(), size_of::()); - let ptr = node_id.0 as *const [u8; NODE_SIZE]; + unsafe { + let node_ptr = self.nodes.offset(node_id.index as isize) as *const [u8; size_of::()]; - *ptr == [0; NODE_SIZE] - } -} - -struct Page { - first_node: *const [u8; NODE_SIZE], - node_count: u8, -} - -impl Default for Page { - fn default() -> Self { - let first_node = if page_size::get() == 4096 { - unsafe { - // mmap exactly one memory page (4096 bytes) - libc::mmap( - null::() as *mut c_void, - PAGE_BYTES, - PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS, - 0, - 0, - ) - } - } else { - // Somehow the page size is not 4096 bytes, so fall back on calloc. - // (We use calloc over malloc because we rely on the bytes having - // been zeroed to tell which nodes are available.) - unsafe { libc::calloc(1, PAGE_BYTES) } - } as *mut [u8; NODE_SIZE]; - - Page { - first_node, - node_count: 0, + *node_ptr == [0; size_of::()] } } } -impl Drop for Page { +impl Drop for Pool { fn drop(&mut self) { - if page_size::get() == 4096 { - unsafe { - libc::munmap(self.first_node as *mut c_void, PAGE_BYTES); - } - } else { - unsafe { - libc::free(self.first_node as *mut c_void); - } + unsafe { + libc::munmap( + self.nodes as *mut c_void, + size_of::() * self.capacity as usize, + ); } } } -/// A string of at most 2^32 bytes, allocated in a pool if it fits in a Page, -/// or using malloc as a fallback if not. Like std::str::String, this has -/// both a length and capacity. +/// A string containing at most 2^32 pool-allocated bytes. #[derive(Debug)] pub struct PoolStr { first_node_id: NodeId<()>, len: u32, - cap: u32, } #[test] fn pool_str_size() { - assert_eq!(size_of::(), size_of::() + 8); + assert_eq!(size_of::(), 8); } -/// An array of at most 2^32 elements, allocated in a pool if it fits in a Page, -/// or using malloc as a fallback if not. Like std::vec::Vec, this has both -/// a length and capacity. +/// An array of at most 2^32 pool-allocated nodes. #[derive(Debug)] -pub struct PoolVec { +pub struct PoolVec { first_node_id: NodeId, len: u32, - cap: u32, } #[test] fn pool_vec_size() { - assert_eq!(size_of::>(), size_of::() + 8); + assert_eq!(size_of::>(), 8); } impl<'a, T: 'a + Sized> PoolVec { @@ -193,59 +192,38 @@ impl<'a, T: 'a + Sized> PoolVec { /// the usual array, and then there's one more node at the end which /// continues the list with a new length and NodeId value. PoolVec /// iterators automatically do these jumps behind the scenes when necessary. - pub fn new>(nodes: I, pool: &mut Pool) -> Self { + pub fn new, S>(nodes: I, pool: &mut Pool) -> Self { debug_assert!(nodes.len() <= u32::MAX as usize); - debug_assert!(size_of::() <= NODE_SIZE); + debug_assert!(size_of::() <= size_of::()); let len = nodes.len() as u32; if len > 0 { - if len <= NODES_PER_PAGE as u32 { - let first_node_id = pool.reserve(len); - let mut next_node_ptr = first_node_id.0 as *mut T; + let first_node_id = pool.reserve(len); + let index = first_node_id.index as isize; + let mut next_node_ptr = unsafe { pool.nodes.offset(index) } as *mut T; - for node in nodes { - unsafe { - *next_node_ptr = node; + for node in nodes { + unsafe { + *next_node_ptr = node; - next_node_ptr = next_node_ptr.offset(1); - } - } - - PoolVec { - first_node_id, - len, - cap: len, - } - } else { - let first_node_ptr = unsafe { - // mmap enough memory to hold it - libc::mmap( - null::() as *mut c_void, - len as usize, - PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS, - 0, - 0, - ) - }; - - PoolVec { - first_node_id: NodeId(first_node_ptr as *const T), - len, - cap: len, + next_node_ptr = next_node_ptr.offset(1); } } + + PoolVec { first_node_id, len } } else { PoolVec { - first_node_id: NodeId(std::ptr::null()), + first_node_id: NodeId { + index: 0, + _phantom: PhantomData::default(), + }, len: 0, - cap: 0, } } } - pub fn iter(self, pool: &'a Pool) -> impl ExactSizeIterator { + pub fn iter(self, pool: &'a Pool) -> impl ExactSizeIterator { self.pool_list_iter(pool) } @@ -254,43 +232,35 @@ impl<'a, T: 'a + Sized> PoolVec { /// actually do want to have this separate function for code reuse /// in the iterator's next() method. #[inline(always)] - fn pool_list_iter(&self, pool: &'a Pool) -> PoolVecIter<'a, T> { + fn pool_list_iter(&self, pool: &'a Pool) -> PoolVecIter<'a, S, T> { PoolVecIter { - _pool: pool, - current_node_id: NodeId(self.first_node_id.0), + pool, + current_node_id: self.first_node_id, len_remaining: self.len, } } - pub fn free(self) { - if self.len <= NODES_PER_PAGE as u32 { - // If this was small enough to fit in a Page, then zero it out. - unsafe { - libc::memset( - self.first_node_id.0 as *mut c_void, - 0, - self.len as usize * NODE_SIZE, - ); - } + pub fn free(self, pool: &'a mut Pool) { + // zero out the memory + unsafe { + let index = self.first_node_id.index as isize; + let node_ptr = pool.nodes.offset(index) as *mut c_void; + let bytes = self.len as usize * size_of::(); + + libc::memset(node_ptr, 0, bytes); + } // TODO insert it into the pool's free list - } else { - // This was bigger than a Page, so we mmap'd it. Now we free it! - unsafe { - libc::munmap(self.first_node_id.0 as *mut c_void, self.len as usize); - } - } } } -struct PoolVecIter<'a, T: Sized> { - /// This iterator returns elements which have the same lifetime as the pool - _pool: &'a Pool, +struct PoolVecIter<'a, S, T> { + pool: &'a Pool, current_node_id: NodeId, len_remaining: u32, } -impl<'a, T: Sized> ExactSizeIterator for PoolVecIter<'a, T> +impl<'a, S, T> ExactSizeIterator for PoolVecIter<'a, S, T> where T: 'a, { @@ -299,7 +269,7 @@ where } } -impl<'a, T: Sized> Iterator for PoolVecIter<'a, T> +impl<'a, S, T> Iterator for PoolVecIter<'a, S, T> where T: 'a, { @@ -310,10 +280,14 @@ where if len_remaining > 1 { // Get the current node - let node_ptr = self.current_node_id.0; + let index = self.current_node_id.index; + let node_ptr = unsafe { self.pool.nodes.offset(index as isize) } as *const T; // Advance the node pointer to the next node in the current page - self.current_node_id = NodeId(unsafe { node_ptr.offset(1) }); + self.current_node_id = NodeId { + index: index + 1, + _phantom: PhantomData::default(), + }; self.len_remaining = len_remaining - 1; Some(unsafe { &*node_ptr }) @@ -323,7 +297,10 @@ where // Don't advance the node pointer's node, because that might // advance past the end of the page! - Some(unsafe { &*self.current_node_id.0 }) + let index = self.current_node_id.index; + let node_ptr = unsafe { self.pool.nodes.offset(index as isize) } as *const T; + + Some(unsafe { &*node_ptr }) } else { // len_remaining was 0 None From c1356f0b68bd3d850f4d971837bb06277988f0b4 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 8 Dec 2020 22:14:59 -0500 Subject: [PATCH 04/20] Fix Pool implementation --- editor/src/ast.rs | 241 ++++++++++++++++++++------------------------- editor/src/pool.rs | 49 ++++----- 2 files changed, 133 insertions(+), 157 deletions(-) diff --git a/editor/src/ast.rs b/editor/src/ast.rs index da28a70b59..9a70d2a7f0 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -28,33 +28,45 @@ pub enum IntStyle { /// It has a 1B discriminant and variants which hold payloads of at most 31B. #[derive(Debug)] pub enum Expr2 { - /// A number literal (without a dot) containing no underscores - Num { + /// A negative number literal without a dot + I64 { number: i64, // 8B var: Variable, // 4B style: IntStyle, // 1B + text: PoolStr, // 8B }, - /// A floating-point literal (with a dot) containing no underscores + /// A nonnegative number literal without a dot + U64 { + number: u64, // 8B + var: Variable, // 4B + style: IntStyle, // 1B + text: PoolStr, // 8B + }, + /// A large (over 64-bit) negative number literal without a dot. + /// This only comes up for literals that won't fit in 64-bit integers. + I128 { + number: i128, // 16B + var: Variable, // 4B + style: IntStyle, // 1B + text: PoolStr, // 8B + }, + /// A large (over 64-bit) nonnegative number literal without a dot + /// This only comes up for literals that won't fit in 64-bit integers. + U128 { + number: u128, // 16B + var: Variable, // 4B + style: IntStyle, // 1B + text: PoolStr, // 8B + }, + /// A floating-point literal (with a dot) Float { number: f64, // 8B var: Variable, // 4B }, - /// A number literal (without a dot) containing underscores - NumWithUnderscores { - number: i64, // 8B - var: Variable, // 4B - text: NodeId, // 8B - }, - /// A float literal (with a dot) containing underscores - FloatWithUnderscores { - number: f64, // 8B - var: Variable, // 4B - text: NodeId, // 8B - }, /// string literals of length up to 30B SmallStr(ArrayString), // 31B /// string literals of length 31B or more - Str(NodeId), // 8B + Str(PoolStr), // 8B // Lookups Var(Symbol), // 8B @@ -67,132 +79,101 @@ pub enum Expr2 { List { list_var: Variable, // 4B - required for uniqueness of the list elem_var: Variable, // 4B - first_elem: PoolVec, // 16B + first_elem: PoolVec, // 8B }, If { cond_var: Variable, // 4B expr_var: Variable, // 4B - branches: PoolVec<(Expr2, Expr2)>, // 16B - final_else: NodeId, // 8B + branches: PoolVec<(Expr2, Expr2)>, // 8B + final_else: NodeId, // 4B }, When { cond_var: Variable, // 4B expr_var: Variable, // 4B - branches: PoolVec, // 9B - cond: NodeId, // 8B + branches: PoolVec, // 8B + cond: NodeId, // 4B }, LetRec { // TODO need to make this Alias type here page-friendly, which will be hard! - aliases: PoolVec<(Symbol, Alias)>, // 9B - defs: PoolVec, // 9B - body_var: Variable, // 4B - body_id: NodeId, // 8B + aliases: PoolVec<(Symbol, Alias)>, // 8B + defs: PoolVec, // 8B + body_var: Variable, // 8B + body_id: NodeId, // 4B + }, + LetNonRec { + // TODO need to make this Alias type here page-friendly, which will be hard! + aliases: PoolVec<(Symbol, Alias)>, // 8B + def_id: NodeId, // 4B + body_id: NodeId, // 4B + body_var: Variable, // 4B + }, + Call { + /// NOTE: the first elem in this list is the expression and its variable. + /// The others are arguments. This is because we didn't have room for + /// both the expr and its variable otherwise. + expr_and_args: PoolVec<(Variable, NodeId)>, // 8B + fn_var: Variable, // 4B + closure_var: Variable, // 4B + /// Cached outside expr_and_args so we don't have to potentially + /// traverse that whole linked list chain to count all the args. + arity: usize, // 8B - could make this smaller if need be + called_via: CalledVia, // 2B + }, + RunLowLevel { + op: LowLevel, // 1B + args: PoolVec<(Variable, NodeId)>, // 8B + ret_var: Variable, // 4B + }, + Closure { + args: PoolVec<(Variable, NodeId)>, // 8B + name: Symbol, // 8B + body: NodeId, // 4B + function_type: Variable, // 4B + recursive: Recursive, // 1B + extra: NodeId, // 4B }, - // LetNonRec { - // // TODO need to make this Alias type here page-friendly, which will be hard! - // aliases: PoolVec<(Symbol, Alias)>, // 9B - // def_id: NodeId, // 8B - // body_id: NodeId, // 8B - // body_var: Variable, // 4B - // }, - // Call { - // /// NOTE: the first elem in this list is the expression and its variable. - // /// The others are arguments. This is because we didn't have room for - // /// both the expr and its variable otherwise. - // expr_and_args: PoolVec<(Variable, NodeId)>, // 9B - // fn_var: Variable, // 4B - // closure_var: Variable, // 4B - // /// Cached outside expr_and_args so we don't have to potentially - // /// traverse that whole linked list chain to count all the args. - // arity: usize, // 8B - could make this smaller if need be - // called_via: CalledVia, // 2B - // }, - // RunLowLevel { - // op: LowLevel, // 1B - // args: PoolVec<(Variable, NodeId)>, // 9B - // ret_var: Variable, // 4B - // }, - // Closure { - // captured_symbols: PoolVec<(Symbol, Variable)>, // 9B - // args: PoolVec<(Variable, NodeId)>, // 9B - // recursive: Recursive, // 1B - // extra: NodeId, // 8B - // }, // Product Types - // Record { - // record_var: Variable, // 4B - // fields: PoolVec<(PoolStr, Variable, NodeId)>, // 9B - // }, + Record { + record_var: Variable, // 4B + fields: PoolVec<(PoolStr, Variable, NodeId)>, // 8B + }, /// Empty record constant - // EmptyRecord, - // /// Look up exactly one field on a record, e.g. (expr).foo. - // Access { - // field: NodeId, // 8B - // expr: NodeId, // 8B - // vars: NodeId, // 8B - // }, + EmptyRecord, + /// Look up exactly one field on a record, e.g. (expr).foo. + Access { + field: NodeId, // 4B + expr: NodeId, // 4B + vars: NodeId, // 4B + }, - // /// field accessor as a function, e.g. (.foo) expr - // Accessor { - // record_vars_id: NodeId, // 8B - // function_var: Variable, // 4B - // closure_var: Variable, // 4B - // field_id: NodeId, // 8B - // }, - // Update { - // symbol: Symbol, // 8B - // updates: PoolVec<(Lowercase, Field)>, // 9B - // vars_id: NodeId, // 8B - // }, + /// field accessor as a function, e.g. (.foo) expr + Accessor { + record_vars_id: NodeId, // 4B + function_var: Variable, // 4B + closure_var: Variable, // 4B + field_id: NodeId, // 4B + }, + Update { + symbol: Symbol, // 8B + updates: PoolVec<(Lowercase, Field)>, // 8B + vars_id: NodeId, // 4B + }, // Sum Types - // Tag { - // // NOTE: A PoolStr node is a 2B length and then 14B bytes, - // // plus more bytes in adjacent nodes if necessary. Thus we have - // // a hard cap of 4094 bytes as the maximum length of tags and fields. - // name_id: NodeId, // 8B - // variant_var: Variable, // 4B - // ext_var: Variable, // 4B - // arguments: PoolVec<(Variable, NodeId)>, // 9B - // }, + Tag { + // NOTE: A PoolStr node is a 2B length and then 14B bytes, + // plus more bytes in adjacent nodes if necessary. Thus we have + // a hard cap of 4094 bytes as the maximum length of tags and fields. + name_id: NodeId, // 4B + variant_var: Variable, // 4B + ext_var: Variable, // 4B + arguments: PoolVec<(Variable, NodeId)>, // 8B + }, // Compiles, but will crash if reached RuntimeError(/* TODO make a version of RuntimeError that fits in 15B */), } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -/// It's critical that this fit in 1 byte. If it takes 2B, Expr::Call is too big. -/// That's why we have all the variants in here, instead of having separate -/// UnaryOp and Binary -pub enum CalledVia2 { - /// Calling with space, e.g. (foo bar) - Space, - - /// (-), e.g. (-x) - Negate, - /// (!), e.g. (!x) - Not, - - // highest precedence binary op - Caret, - Star, - Slash, - DoubleSlash, - Percent, - DoublePercent, - Plus, - Minus, - Equals, - NotEquals, - LessThan, - GreaterThan, - LessThanOrEq, - GreaterThanOrEq, - And, - Or, - Pizza, // lowest precedence binary op -} - #[derive(Debug)] pub struct Def { pub pattern: NodeId, // 3B @@ -230,15 +211,14 @@ pub struct AccessVars { field_var: Variable, // 4B } -/// This is 32B, so it fits in a Node slot. +/// This is overflow data from a Closure variant, which needs to store +/// more than 32B of total data #[derive(Debug)] pub struct ClosureExtra { - name: Symbol, // 8B - body: NodeId, // 8B - function_type: Variable, // 4B - closure_type: Variable, // 4B - closure_ext_var: Variable, // 4B - return_type: Variable, // 4B + return_type: Variable, // 4B + captured_symbols: PoolVec<(Symbol, Variable)>, // 8B + closure_type: Variable, // 4B + closure_ext_var: Variable, // 4B } #[derive(Debug)] @@ -278,10 +258,5 @@ pub struct ExprPoolSlot(u8); #[test] fn size_of_expr() { - assert_eq!(std::mem::size_of::(), 32); -} - -#[test] -fn size_of_called_via() { - assert_eq!(std::mem::size_of::(), 1); + assert_eq!(std::mem::size_of::(), crate::pool::NODE_BYTES); } diff --git a/editor/src/pool.rs b/editor/src/pool.rs index 1c7d13e3fb..91dfde3789 100644 --- a/editor/src/pool.rs +++ b/editor/src/pool.rs @@ -15,6 +15,8 @@ use std::marker::PhantomData; use std::mem::size_of; use std::ptr::null; +pub const NODE_BYTES: usize = 32; + // Each page has 128 slots. Each slot holds one 32B node // This means each page is 4096B, which is the size of a memory page // on typical systems where the compiler will be run. @@ -56,19 +58,18 @@ impl Clone for NodeId { impl Copy for NodeId {} -/// S is a slot size; e.g. Pool<[u8; 32]> for a pool of 32-bit slots -pub struct Pool { - nodes: *mut S, +pub struct Pool { + nodes: *mut [u8; NODE_BYTES], num_nodes: u32, capacity: u32, // free_1node_slots: Vec>, } -impl Pool { +impl Pool { pub fn with_capacity(&mut self, nodes: u32) -> Self { // round up number of nodes requested to nearest page size in bytes let bytes_per_page = page_size::get(); - let node_bytes = size_of::() * nodes as usize; + let node_bytes = NODE_BYTES * nodes as usize; let leftover = node_bytes % bytes_per_page; let bytes_to_mmap = if leftover == 0 { node_bytes @@ -89,12 +90,12 @@ impl Pool { 0, 0, ) - } as *mut S; + } as *mut [u8; NODE_BYTES]; // This is our actual capacity, in nodes. // It might be higher than the requested capacity due to rounding up // to nearest page size. - let capacity = (bytes_to_mmap / size_of::()) as u32; + let capacity = (bytes_to_mmap / NODE_BYTES) as u32; Pool { nodes, @@ -105,7 +106,7 @@ impl Pool { pub fn add(&mut self, node: T) -> NodeId { // It's only safe to store this if T is the same size as S. - debug_assert_eq!(size_of::(), size_of::()); + debug_assert_eq!(size_of::(), NODE_BYTES); let index = self.num_nodes; @@ -141,23 +142,23 @@ impl Pool { // A node is available iff its bytes are all zeroes #[allow(dead_code)] - unsafe fn is_available(&self, node_id: NodeId) -> bool { - debug_assert_eq!(size_of::(), size_of::()); + fn is_available(&self, node_id: NodeId) -> bool { + debug_assert_eq!(size_of::(), NODE_BYTES); unsafe { - let node_ptr = self.nodes.offset(node_id.index as isize) as *const [u8; size_of::()]; + let node_ptr = self.nodes.offset(node_id.index as isize) as *const [u8; NODE_BYTES]; - *node_ptr == [0; size_of::()] + *node_ptr == [0; NODE_BYTES] } } } -impl Drop for Pool { +impl Drop for Pool { fn drop(&mut self) { unsafe { libc::munmap( self.nodes as *mut c_void, - size_of::() * self.capacity as usize, + NODE_BYTES * self.capacity as usize, ); } } @@ -192,9 +193,9 @@ impl<'a, T: 'a + Sized> PoolVec { /// the usual array, and then there's one more node at the end which /// continues the list with a new length and NodeId value. PoolVec /// iterators automatically do these jumps behind the scenes when necessary. - pub fn new, S>(nodes: I, pool: &mut Pool) -> Self { + pub fn new, S>(nodes: I, pool: &mut Pool) -> Self { debug_assert!(nodes.len() <= u32::MAX as usize); - debug_assert!(size_of::() <= size_of::()); + debug_assert!(size_of::() <= NODE_BYTES); let len = nodes.len() as u32; @@ -223,7 +224,7 @@ impl<'a, T: 'a + Sized> PoolVec { } } - pub fn iter(self, pool: &'a Pool) -> impl ExactSizeIterator { + pub fn iter(self, pool: &'a Pool) -> impl ExactSizeIterator { self.pool_list_iter(pool) } @@ -232,7 +233,7 @@ impl<'a, T: 'a + Sized> PoolVec { /// actually do want to have this separate function for code reuse /// in the iterator's next() method. #[inline(always)] - fn pool_list_iter(&self, pool: &'a Pool) -> PoolVecIter<'a, S, T> { + fn pool_list_iter(&self, pool: &'a Pool) -> PoolVecIter<'a, T> { PoolVecIter { pool, current_node_id: self.first_node_id, @@ -240,12 +241,12 @@ impl<'a, T: 'a + Sized> PoolVec { } } - pub fn free(self, pool: &'a mut Pool) { + pub fn free(self, pool: &'a mut Pool) { // zero out the memory unsafe { let index = self.first_node_id.index as isize; let node_ptr = pool.nodes.offset(index) as *mut c_void; - let bytes = self.len as usize * size_of::(); + let bytes = self.len as usize * NODE_BYTES; libc::memset(node_ptr, 0, bytes); } @@ -254,13 +255,13 @@ impl<'a, T: 'a + Sized> PoolVec { } } -struct PoolVecIter<'a, S, T> { - pool: &'a Pool, +struct PoolVecIter<'a, T> { + pool: &'a Pool, current_node_id: NodeId, len_remaining: u32, } -impl<'a, S, T> ExactSizeIterator for PoolVecIter<'a, S, T> +impl<'a, T> ExactSizeIterator for PoolVecIter<'a, T> where T: 'a, { @@ -269,7 +270,7 @@ where } } -impl<'a, S, T> Iterator for PoolVecIter<'a, S, T> +impl<'a, T> Iterator for PoolVecIter<'a, T> where T: 'a, { From bdf08ea5a6862eeb13dab4abaf2be9cfd7a14494 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 8 Dec 2020 23:01:10 -0500 Subject: [PATCH 05/20] Remove unused argument from with_capacity --- editor/src/pool.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor/src/pool.rs b/editor/src/pool.rs index 91dfde3789..d5bc2bc1a7 100644 --- a/editor/src/pool.rs +++ b/editor/src/pool.rs @@ -66,7 +66,7 @@ pub struct Pool { } impl Pool { - pub fn with_capacity(&mut self, nodes: u32) -> Self { + pub fn with_capacity(nodes: u32) -> Self { // round up number of nodes requested to nearest page size in bytes let bytes_per_page = page_size::get(); let node_bytes = NODE_BYTES * nodes as usize; From a46a8d076bdfd83cbe481921fb7ade68d63837dd Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 8 Dec 2020 23:01:16 -0500 Subject: [PATCH 06/20] Add a mmap benchmark --- editor/src/pool.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/editor/src/pool.rs b/editor/src/pool.rs index d5bc2bc1a7..1128923bab 100644 --- a/editor/src/pool.rs +++ b/editor/src/pool.rs @@ -308,3 +308,29 @@ where } } } + +#[test] +fn mmap_benchmark() { + use std::time::SystemTime; + + let start_time = SystemTime::now(); + let pool = Pool::with_capacity(1234567890); + let end_time = SystemTime::now(); + let elapsed = end_time.duration_since(start_time).unwrap(); + + println!("Time to init pool: {:?}", elapsed); + + // touch a bunch of pages to see how long they take to fault in + for i in 1..100 { + let start_time = SystemTime::now(); + unsafe { + *pool.nodes.offset(i * 10) = [1; NODE_BYTES]; + }; + let end_time = SystemTime::now(); + let elapsed = end_time.duration_since(start_time).unwrap(); + + println!("Time to touch page {}: {:?}", i, elapsed); + } + + assert_eq!(pool.capacity, 128); +} From 48dd79c5f408fe6e76b048b2e51c85f3544836b4 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 8 Dec 2020 23:01:19 -0500 Subject: [PATCH 07/20] Revert "Add a mmap benchmark" This reverts commit 89f213d58f92f16227b3b0542e5a99e10874e91a. --- editor/src/pool.rs | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/editor/src/pool.rs b/editor/src/pool.rs index 1128923bab..d5bc2bc1a7 100644 --- a/editor/src/pool.rs +++ b/editor/src/pool.rs @@ -308,29 +308,3 @@ where } } } - -#[test] -fn mmap_benchmark() { - use std::time::SystemTime; - - let start_time = SystemTime::now(); - let pool = Pool::with_capacity(1234567890); - let end_time = SystemTime::now(); - let elapsed = end_time.duration_since(start_time).unwrap(); - - println!("Time to init pool: {:?}", elapsed); - - // touch a bunch of pages to see how long they take to fault in - for i in 1..100 { - let start_time = SystemTime::now(); - unsafe { - *pool.nodes.offset(i * 10) = [1; NODE_BYTES]; - }; - let end_time = SystemTime::now(); - let elapsed = end_time.duration_since(start_time).unwrap(); - - println!("Time to touch page {}: {:?}", i, elapsed); - } - - assert_eq!(pool.capacity, 128); -} From 91c3cf25b6c4621316110d68a4999267b34f4597 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 8 Dec 2020 23:15:20 -0500 Subject: [PATCH 08/20] Revise how numbers are stored in editor IR --- editor/src/ast.rs | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/editor/src/ast.rs b/editor/src/ast.rs index 9a70d2a7f0..d8c95208a1 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -24,26 +24,43 @@ pub enum IntStyle { Binary, } +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum IntVal { + I64(i64), + U64(u64), + I32(i32), + U32(u32), + I16(i16), + U16(u16), + I8(i8), + U8(u8), +} + +#[derive(Debug, Copy, Clone, PartialEq)] +pub enum FloatVal { + F64(f64), + F32(f32), +} + +#[test] +fn size_of_intval() { + assert_eq!(std::mem::size_of::(), 16); +} + /// An Expr that fits in 32B. /// It has a 1B discriminant and variants which hold payloads of at most 31B. #[derive(Debug)] pub enum Expr2 { /// A negative number literal without a dot - I64 { - number: i64, // 8B - var: Variable, // 4B - style: IntStyle, // 1B - text: PoolStr, // 8B - }, - /// A nonnegative number literal without a dot - U64 { - number: u64, // 8B + SmallInt { + number: IntVal, // 16B var: Variable, // 4B style: IntStyle, // 1B text: PoolStr, // 8B }, /// A large (over 64-bit) negative number literal without a dot. - /// This only comes up for literals that won't fit in 64-bit integers. + /// This variant can't use IntVal because if IntVal stored 128-bit + /// integers, it would be 32B on its own because of alignment. I128 { number: i128, // 16B var: Variable, // 4B @@ -51,7 +68,8 @@ pub enum Expr2 { text: PoolStr, // 8B }, /// A large (over 64-bit) nonnegative number literal without a dot - /// This only comes up for literals that won't fit in 64-bit integers. + /// This variant can't use IntVal because if IntVal stored 128-bit + /// integers, it would be 32B on its own because of alignment. U128 { number: u128, // 16B var: Variable, // 4B @@ -60,8 +78,8 @@ pub enum Expr2 { }, /// A floating-point literal (with a dot) Float { - number: f64, // 8B - var: Variable, // 4B + number: FloatVal, // 16B + var: Variable, // 4B }, /// string literals of length up to 30B SmallStr(ArrayString), // 31B From 68cf9602e1fd4622a6f624b021065c1e279984a7 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 8 Dec 2020 23:15:47 -0500 Subject: [PATCH 09/20] cargo fmt --- editor/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor/src/lib.rs b/editor/src/lib.rs index 10b2fca8ec..70e6d8c77b 100644 --- a/editor/src/lib.rs +++ b/editor/src/lib.rs @@ -27,10 +27,10 @@ use winit::event::{Event, ModifiersState}; use winit::event_loop::ControlFlow; pub mod ast; -pub mod pool; mod buffer; pub mod file; mod keyboard_input; +pub mod pool; mod rect; pub mod text; mod util; From 61300f782da10bef81b8d6f35007574f33f997b0 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 8 Dec 2020 23:19:05 -0500 Subject: [PATCH 10/20] Use cmp, per clippy --- editor/src/pool.rs | 47 +++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/editor/src/pool.rs b/editor/src/pool.rs index d5bc2bc1a7..05e3655b9e 100644 --- a/editor/src/pool.rs +++ b/editor/src/pool.rs @@ -11,6 +11,7 @@ /// Pages also use the node value 0 (all 0 bits) to mark nodes as unoccupied. /// This is important for performance. use libc::{c_void, MAP_ANONYMOUS, MAP_PRIVATE, PROT_READ, PROT_WRITE}; +use std::cmp::Ordering; use std::marker::PhantomData; use std::mem::size_of; use std::ptr::null; @@ -279,32 +280,36 @@ where fn next(&mut self) -> Option { let len_remaining = self.len_remaining; - if len_remaining > 1 { - // Get the current node - let index = self.current_node_id.index; - let node_ptr = unsafe { self.pool.nodes.offset(index as isize) } as *const T; + match len_remaining.cmp(&1) { + Ordering::Greater => { + // Get the current node + let index = self.current_node_id.index; + let node_ptr = unsafe { self.pool.nodes.offset(index as isize) } as *const T; - // Advance the node pointer to the next node in the current page - self.current_node_id = NodeId { - index: index + 1, - _phantom: PhantomData::default(), - }; - self.len_remaining = len_remaining - 1; + // Advance the node pointer to the next node in the current page + self.current_node_id = NodeId { + index: index + 1, + _phantom: PhantomData::default(), + }; + self.len_remaining = len_remaining - 1; - Some(unsafe { &*node_ptr }) - } else if len_remaining == 1 { - self.len_remaining = 0; + Some(unsafe { &*node_ptr }) + } + Ordering::Equal => { + self.len_remaining = 0; - // Don't advance the node pointer's node, because that might - // advance past the end of the page! + // Don't advance the node pointer's node, because that might + // advance past the end of the page! - let index = self.current_node_id.index; - let node_ptr = unsafe { self.pool.nodes.offset(index as isize) } as *const T; + let index = self.current_node_id.index; + let node_ptr = unsafe { self.pool.nodes.offset(index as isize) } as *const T; - Some(unsafe { &*node_ptr }) - } else { - // len_remaining was 0 - None + Some(unsafe { &*node_ptr }) + } + Ordering::Less => { + // len_remaining was 0 + None + } } } } From acb2bc70921cd7bc4242441a7ed21a65ab8538c6 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 9 Dec 2020 07:53:42 -0500 Subject: [PATCH 11/20] fix field name --- editor/src/ast.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor/src/ast.rs b/editor/src/ast.rs index d8c95208a1..b9188bb04b 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -97,7 +97,7 @@ pub enum Expr2 { List { list_var: Variable, // 4B - required for uniqueness of the list elem_var: Variable, // 4B - first_elem: PoolVec, // 8B + elems: PoolVec, // 8B }, If { cond_var: Variable, // 4B From 0d0a9e677c07ae4c355e7c75939f3a51926efc13 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 9 Dec 2020 08:02:21 -0500 Subject: [PATCH 12/20] Remove obsolete expr_and_args --- editor/src/ast.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/editor/src/ast.rs b/editor/src/ast.rs index b9188bb04b..ca056c0bdf 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -126,16 +126,12 @@ pub enum Expr2 { body_var: Variable, // 4B }, Call { - /// NOTE: the first elem in this list is the expression and its variable. - /// The others are arguments. This is because we didn't have room for - /// both the expr and its variable otherwise. - expr_and_args: PoolVec<(Variable, NodeId)>, // 8B - fn_var: Variable, // 4B - closure_var: Variable, // 4B - /// Cached outside expr_and_args so we don't have to potentially - /// traverse that whole linked list chain to count all the args. - arity: usize, // 8B - could make this smaller if need be - called_via: CalledVia, // 2B + args: PoolVec<(Variable, NodeId)>, // 8B + expr: NodeId, // 4B + expr_var: Variable, // 4B + fn_var: Variable, // 4B + closure_var: Variable, // 4B + called_via: CalledVia, // 2B }, RunLowLevel { op: LowLevel, // 1B From c0a74ab10ef4a319ca63ea7414227f7448b82b60 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 9 Dec 2020 08:03:58 -0500 Subject: [PATCH 13/20] Drop oboslete comment --- editor/src/ast.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/editor/src/ast.rs b/editor/src/ast.rs index ca056c0bdf..c3fc7dede2 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -175,9 +175,6 @@ pub enum Expr2 { // Sum Types Tag { - // NOTE: A PoolStr node is a 2B length and then 14B bytes, - // plus more bytes in adjacent nodes if necessary. Thus we have - // a hard cap of 4094 bytes as the maximum length of tags and fields. name_id: NodeId, // 4B variant_var: Variable, // 4B ext_var: Variable, // 4B From fdd3fd14e982a603cfb5e624fe05fc5977f0e9c0 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 9 Dec 2020 08:06:17 -0500 Subject: [PATCH 14/20] cargo fmt --- editor/src/ast.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/editor/src/ast.rs b/editor/src/ast.rs index c3fc7dede2..811a9a51bb 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -95,8 +95,8 @@ pub enum Expr2 { elem_var: Variable, // 4B }, List { - list_var: Variable, // 4B - required for uniqueness of the list - elem_var: Variable, // 4B + list_var: Variable, // 4B - required for uniqueness of the list + elem_var: Variable, // 4B elems: PoolVec, // 8B }, If { From 22369e523c054db6a69eb8bfca39617e61f5a004 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 9 Dec 2020 08:06:24 -0500 Subject: [PATCH 15/20] Use PoolStr for tag and field names --- editor/src/ast.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/editor/src/ast.rs b/editor/src/ast.rs index 811a9a51bb..f2d1f1ed77 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -155,7 +155,7 @@ pub enum Expr2 { EmptyRecord, /// Look up exactly one field on a record, e.g. (expr).foo. Access { - field: NodeId, // 4B + field: PoolStr, // 4B expr: NodeId, // 4B vars: NodeId, // 4B }, @@ -165,7 +165,7 @@ pub enum Expr2 { record_vars_id: NodeId, // 4B function_var: Variable, // 4B closure_var: Variable, // 4B - field_id: NodeId, // 4B + field: PoolStr, // 4B }, Update { symbol: Symbol, // 8B @@ -175,7 +175,7 @@ pub enum Expr2 { // Sum Types Tag { - name_id: NodeId, // 4B + name: PoolStr, // 4B variant_var: Variable, // 4B ext_var: Variable, // 4B arguments: PoolVec<(Variable, NodeId)>, // 8B From b48a6dee28203620e27b9b5e678196194cf9078d Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 9 Dec 2020 08:08:46 -0500 Subject: [PATCH 16/20] Drop obsolete UpdateVars --- editor/src/ast.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/editor/src/ast.rs b/editor/src/ast.rs index f2d1f1ed77..df95817156 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -170,7 +170,8 @@ pub enum Expr2 { Update { symbol: Symbol, // 8B updates: PoolVec<(Lowercase, Field)>, // 8B - vars_id: NodeId, // 4B + record_var: Variable, // 4B + ext_var: Variable, // 4B }, // Sum Types @@ -201,12 +202,6 @@ pub enum Pat2 { Todo, } -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub struct UpdateVars { - record_var: Variable, // 4B - ext_var: Variable, // 4B -} - #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct RecordVars { record_var: Variable, // 4B From ed4e223b03ea2cb0b7b7767a992a50583e74ba7d Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 9 Dec 2020 08:09:20 -0500 Subject: [PATCH 17/20] Drop obsolete RecordVars --- editor/src/ast.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/editor/src/ast.rs b/editor/src/ast.rs index df95817156..973e310a4b 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -162,10 +162,12 @@ pub enum Expr2 { /// field accessor as a function, e.g. (.foo) expr Accessor { - record_vars_id: NodeId, // 4B - function_var: Variable, // 4B - closure_var: Variable, // 4B - field: PoolStr, // 4B + function_var: Variable, // 4B + closure_var: Variable, // 4B + field: PoolStr, // 4B + record_var: Variable, // 4B + ext_var: Variable, // 4B + field_var: Variable, // 4B }, Update { symbol: Symbol, // 8B @@ -202,13 +204,6 @@ pub enum Pat2 { Todo, } -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub struct RecordVars { - record_var: Variable, // 4B - ext_var: Variable, // 4B - field_var: Variable, // 4B -} - /// This is 15B, so it fits in a Node slot. #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct AccessVars { From 7cad3eb68c04b3e7a5f66099fca3bb1904aeac89 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 9 Dec 2020 08:09:45 -0500 Subject: [PATCH 18/20] Drop obsolete AccessVars --- editor/src/ast.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/editor/src/ast.rs b/editor/src/ast.rs index 973e310a4b..38e5d9fc14 100644 --- a/editor/src/ast.rs +++ b/editor/src/ast.rs @@ -155,9 +155,11 @@ pub enum Expr2 { EmptyRecord, /// Look up exactly one field on a record, e.g. (expr).foo. Access { - field: PoolStr, // 4B - expr: NodeId, // 4B - vars: NodeId, // 4B + field: PoolStr, // 4B + expr: NodeId, // 4B + record_var: Variable, // 4B + ext_var: Variable, // 4B + field_var: Variable, // 4B }, /// field accessor as a function, e.g. (.foo) expr @@ -204,14 +206,6 @@ pub enum Pat2 { Todo, } -/// This is 15B, so it fits in a Node slot. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub struct AccessVars { - record_var: Variable, // 4B - ext_var: Variable, // 4B - field_var: Variable, // 4B -} - /// This is overflow data from a Closure variant, which needs to store /// more than 32B of total data #[derive(Debug)] From 53534f5b098fe04671d69ac5ba6b1d8e69a58184 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 9 Dec 2020 19:05:19 -0500 Subject: [PATCH 19/20] Drop obsolete comment --- editor/src/pool.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/editor/src/pool.rs b/editor/src/pool.rs index 05e3655b9e..25eafe8d8e 100644 --- a/editor/src/pool.rs +++ b/editor/src/pool.rs @@ -190,10 +190,6 @@ fn pool_vec_size() { } impl<'a, T: 'a + Sized> PoolVec { - /// If given a slice of length > 128, the first 128 nodes will be stored in - /// the usual array, and then there's one more node at the end which - /// continues the list with a new length and NodeId value. PoolVec - /// iterators automatically do these jumps behind the scenes when necessary. pub fn new, S>(nodes: I, pool: &mut Pool) -> Self { debug_assert!(nodes.len() <= u32::MAX as usize); debug_assert!(size_of::() <= NODE_BYTES); From d420beb56f9f6bf4982958f29c8d695eea32871a Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 9 Dec 2020 20:34:41 -0500 Subject: [PATCH 20/20] Implement Pool::reserve --- editor/src/pool.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/editor/src/pool.rs b/editor/src/pool.rs index 25eafe8d8e..c862c1b99c 100644 --- a/editor/src/pool.rs +++ b/editor/src/pool.rs @@ -109,14 +109,22 @@ impl Pool { // It's only safe to store this if T is the same size as S. debug_assert_eq!(size_of::(), NODE_BYTES); + let node_id = self.reserve(1); + let node_ptr = unsafe { self.nodes.offset(node_id.index as isize) } as *mut T; + + unsafe { *node_ptr = node }; + + node_id + } + + /// Reserves the given number of contiguous node slots, and returns + /// the NodeId of the first one. We only allow reserving 2^32 in a row. + fn reserve(&mut self, nodes: u32) -> NodeId { + // TODO once we have a free list, look in there for an open slot first! let index = self.num_nodes; if index < self.capacity { - let node_ptr = unsafe { self.nodes.offset(index as isize) } as *mut T; - - unsafe { *node_ptr = node }; - - self.num_nodes = index + 1; + self.num_nodes = index + nodes; NodeId { index, @@ -127,12 +135,6 @@ impl Pool { } } - /// Reserves the given number of contiguous node slots, and returns - /// the NodeId of the first one. We only allow reserving 2^32 in a row. - fn reserve(&mut self, _nodes: u32) -> NodeId { - todo!("Implement Pool::reserve"); - } - pub fn get<'a, T>(&'a self, node_id: NodeId) -> &'a T { unsafe { let node_ptr = self.nodes.offset(node_id.index as isize) as *mut T;