From 938bed05fc71331f11b024030505bcc60f7d5489 Mon Sep 17 00:00:00 2001 From: milo Date: Thu, 22 Feb 2024 18:21:52 -0500 Subject: [PATCH] minor code cleanup particularly in TransTable --- fish/src/bot/node.rs | 53 +++++++++++++++++++++++-------------------- fish/src/bot/trans.rs | 18 +++++++-------- fish/src/find.rs | 5 ++-- fish/src/lib.rs | 4 ++++ mino/src/queue.rs | 13 ++++------- 5 files changed, 49 insertions(+), 44 deletions(-) diff --git a/fish/src/bot/node.rs b/fish/src/bot/node.rs index 52d0d8a..6cd79aa 100644 --- a/fish/src/bot/node.rs +++ b/fish/src/bot/node.rs @@ -99,41 +99,46 @@ impl Node { where E: FnMut(&Mat, Queue<'_>, &Range) -> (bool, i32) + 'a, { + let mut matrix = MatBuf::new(); + let placements = self.queue().reachable().flat_map(|ty| { let locs = find_locations(self.matrix(), ty); locs.map(move |loc| Piece { ty, loc }) }); - let mut matrix = MatBuf::new(); - - placements.filter_map(move |placement| { + let children = placements.map(move |placement| { + // compute new board state from placement matrix.copy_from(self.matrix()); placement.cells().fill(&mut matrix); let cleared = matrix.clear_lines(); let queue = self.queue().remove(placement.ty); - trans - .get_or_insert(&matrix, queue, || { - Node::alloc( - arena, - copy_matrix(arena, &matrix), - // optimization: copy_queue() not necessary! self.queue is already - // allocated on the arena, and this queue just aliases pointers - // into self.queue.next - queue, - evaluate(&matrix, queue, &cleared), - Some(Edge { - placement, - parent: self.into(), - }), - ) - }) - // ignore duplicates found in the transposition table - .map(|n| tracing::trace!("duplicate: {n:?}")) - .err() + // allocate new node unless this state already exists + trans.get_or_insert(&matrix, queue, || { + Node::alloc( + arena, + copy_matrix(arena, &matrix), + // `queue` is already allocated on the arena so don't need to copy it + queue, + evaluate(&matrix, queue, &cleared), + Some(Edge { + placement, + parent: self.into(), + }), + ) + }) + }); - // TODO: when a duplicate node is encountered, reparent it so that it has - // a more preferable initial path. + // exclude duplicate children to avoid redundant search sub-trees + children.filter_map(|child| { + match child { + Ok(new) => Some(new), + Err(dup) => { + // TODO: reparent dup so that it has a more preferable initial path. + tracing::trace!("ignoring duplicate child {dup:?}"); + None + } + } }) } } diff --git a/fish/src/bot/trans.rs b/fish/src/bot/trans.rs index ab0c1dd..4064f00 100644 --- a/fish/src/bot/trans.rs +++ b/fish/src/bot/trans.rs @@ -6,14 +6,12 @@ use mino::srs::{PieceType, Queue}; use mino::Mat; use crate::bot::node::{Node, RawNodePtr}; +use crate::HashMap; pub(crate) struct TransTable { lookup: HashMap>, } -type HashMap = hashbrown::HashMap; -type HashBuilder = core::hash::BuildHasherDefault; - impl TransTable { /// Constructs a new empty transposition table. pub fn new() -> Self { @@ -23,14 +21,14 @@ impl TransTable { } /// Looks up a node in the transposition table matching the given state. If one is - /// found, returns `Ok(existing_node_ptr)`. If not found, a node is created and - /// inserted by calling the given closure, and returns `Err(newly_inserted_node)`. + /// found, returns `Err(existing_node_ptr)`. If not found, a node is created and + /// inserted by calling the given closure, and returns `Ok(newly_inserted_node)`. pub fn get_or_insert<'a>( &mut self, matrix: &Mat, queue: Queue<'_>, mut alloc: impl FnMut() -> &'a Node, - ) -> Result { + ) -> Result<&'a Node, RawNodePtr> { // two-phase lookup: first key off queue info, then matrix info let map = self @@ -44,19 +42,19 @@ impl TransTable { let mat_ptr = unsafe { RawMatPtr::new(matrix) }; if let Some(&node_ptr) = map.get(&mat_ptr) { - return Ok(node_ptr); + return Err(node_ptr); } let node = alloc(); - debug_assert!(*node.matrix() == *matrix); - debug_assert!(node.queue() == queue); + debug_assert_eq!(*node.matrix(), *matrix); + debug_assert_eq!(node.queue(), queue); // SAFETY: as noted above, this matrix ptr is OK to use for insert, since it comes // from the newly allocated node. let mat_ptr = unsafe { RawMatPtr::new(node.matrix()) }; map.insert(mat_ptr, RawNodePtr::from(node)); - Err(node) + Ok(node) } } diff --git a/fish/src/find.rs b/fish/src/find.rs index 95ea9d0..3c82533 100644 --- a/fish/src/find.rs +++ b/fish/src/find.rs @@ -1,12 +1,13 @@ //! Find locations. use alloc::vec::Vec; -use hashbrown::hash_set::HashSet; use mino::input::Kicks; use mino::piece::{Shape, Spawn}; use mino::srs::PieceType; use mino::{input, Loc, Mat, Movement, Piece}; +use crate::HashSet; + static ALL_INPUTS: &[Movement] = &[ Movement::LEFT, Movement::RIGHT, @@ -31,7 +32,7 @@ pub struct FindLocations<'m, T> { piece_ty: T, // TODO: allow these two collections to be extracted and reused stack: Vec, - visited: HashSet>, + visited: HashSet, } impl<'m, 'c, 'k, T> FindLocations<'m, T> diff --git a/fish/src/lib.rs b/fish/src/lib.rs index ca00560..8d7310b 100644 --- a/fish/src/lib.rs +++ b/fish/src/lib.rs @@ -5,3 +5,7 @@ extern crate alloc; pub mod bot; pub mod eval; pub mod find; + +type HashMap = hashbrown::HashMap; +type HashSet = hashbrown::HashSet; +type HashBuilder = core::hash::BuildHasherDefault; diff --git a/mino/src/queue.rs b/mino/src/queue.rs index a2d64c8..3d544ed 100644 --- a/mino/src/queue.rs +++ b/mino/src/queue.rs @@ -47,7 +47,7 @@ impl<'a, T: Copy> Queue<'a, T> { /// be empty, and may yield at most 2 elements. pub fn reachable(&self) -> impl Iterator { let mut hold = self.hold; - let mut front = self.next.get(0).copied(); + let mut front = self.next.first().copied(); core::iter::from_fn(move || hold.take().or_else(|| front.take())) } } @@ -107,14 +107,11 @@ mod test { let idx = ts .iter() .enumerate() - .find_map(|(i, &u)| (t == u).then_some(i)); - if let Some(idx) = idx { - ts.remove(idx); - } else { - panic!("{t:?} found but not expected"); - } + .find_map(|(i, &u)| (t == u).then_some(i)) + .unwrap_or_else(|| panic!("{t:?} found but not expected")); + ts.remove(idx); } - for t in ts { + if let Some(t) = ts.first() { panic!("{t:?} expected but not found"); } }