From 876be9c55ad5060ede431d01ec2e7315f48a7c35 Mon Sep 17 00:00:00 2001 From: RunasSudo Date: Sun, 21 Aug 2022 05:24:54 +1000 Subject: [PATCH] Write more efficient implementation for CandidateMap which does not rely on hashing --- src/candmap.rs | 216 +++++++++++++++++++++++++++++++---- src/constraints.rs | 2 +- src/election.rs | 3 +- src/stv/gregory/transfers.rs | 4 +- src/stv/mod.rs | 24 ++-- 5 files changed, 211 insertions(+), 38 deletions(-) diff --git a/src/candmap.rs b/src/candmap.rs index 3bbfc0c..a6bcfe9 100644 --- a/src/candmap.rs +++ b/src/candmap.rs @@ -17,72 +17,92 @@ use crate::election::Candidate; -use nohash_hasher::BuildNoHashHasher; - -use std::{collections::{HashMap, hash_map}, ops::Index}; +use std::ops::Index; /// Newtype for [HashMap] on [Candidate]s #[derive(Clone)] pub struct CandidateMap<'e, V> { - // TODO: Can we implement this more efficiently as a Vec? - map: HashMap<&'e Candidate, V, BuildNoHashHasher> + keys: Vec>, + values: Vec> } impl<'e, V> CandidateMap<'e, V> { /// See [HashMap::new] pub fn new() -> Self { Self { - map: HashMap::with_hasher(BuildNoHashHasher::default()) + keys: Vec::new(), + values: Vec::new() } } /// See [HashMap::with_capacity] pub fn with_capacity(capacity: usize) -> Self { - Self { - map: HashMap::with_capacity_and_hasher(capacity, BuildNoHashHasher::default()) + let mut ret = Self { + keys: Vec::with_capacity(capacity), + values: Vec::with_capacity(capacity) + }; + ret.maybe_resize(capacity); + return ret; + } + + fn maybe_resize(&mut self, len: usize) { + if len < self.keys.len() { + return; } + + self.keys.resize_with(len, || None); + self.values.resize_with(len, || None); } /// See [HashMap::len] #[inline] pub fn len(&self) -> usize { - return self.map.len(); + return self.keys.iter().filter(|k| k.is_some()).count(); } /// See [HashMap::insert] #[inline] pub fn insert(&mut self, candidate: &'e Candidate, value: V) { - self.map.insert(candidate, value); + self.maybe_resize(candidate.index + 1); + self.keys[candidate.index] = Some(candidate); + self.values[candidate.index] = Some(value); } /// See [HashMap::get] #[inline] pub fn get(&self, candidate: &'e Candidate) -> Option<&V> { - return self.map.get(candidate); + return self.values.get(candidate.index).unwrap_or(&None).as_ref(); } /// See [HashMap::get_mut] #[inline] pub fn get_mut(&mut self, candidate: &'e Candidate) -> Option<&mut V> { - return self.map.get_mut(candidate); + match self.values.get_mut(candidate.index) { + Some(v) => { + return v.as_mut(); + } + None => { + return None; + } + } } /// See [HashMap::iter] #[inline] - pub fn iter(&self) -> hash_map::Iter<&'e Candidate, V> { - return self.map.iter(); + pub fn iter(&self) -> Iter<'_, 'e, V> { + return Iter { map: &self, index: 0 }; } /// See [HashMap::iter_mut] #[inline] - pub fn iter_mut(&mut self) -> hash_map::IterMut<&'e Candidate, V> { - return self.map.iter_mut(); + pub fn iter_mut(&mut self) -> IterMut<'_, 'e, V> { + return IterMut { map: self, index: 0 }; } /// See [HashMap::values] #[inline] - pub fn values(&self) -> hash_map::Values<&'e Candidate, V> { - return self.map.values(); + pub fn values(&self) -> Values<'_, 'e, V> { + return Values { map: &self, index: 0 }; } } @@ -90,16 +110,170 @@ impl<'e, V> Index<&Candidate> for CandidateMap<'e, V> { type Output = V; fn index(&self, candidate: &Candidate) -> &Self::Output { - return &self.map[candidate]; + return self.values.get(candidate.index).unwrap_or(&None).as_ref().unwrap(); + } +} + +pub struct Iter<'m, 'e, V> { + map: &'m CandidateMap<'e, V>, + index: usize +} + +impl<'m, 'e, V> Iterator for Iter<'m, 'e, V> { + type Item = (&'e Candidate, &'m V); + + fn next(&mut self) -> Option { + loop { + match self.map.keys.get(self.index) { + Some(k) => { + // Key within range + match k { + Some(kk) => { + // Key is set + + // SAFETY: Guaranteed to be set, as we update key and value at the same time + let v = unsafe { self.map.values.get_unchecked(self.index).as_ref().unwrap_unchecked() }; + self.index += 1; + return Some((kk, v)); + } + None => { + // Key is unset + self.index += 1; + continue; + } + } + } + None => { + // Key outside range + return None; + } + } + } + } +} + +pub struct IterMut<'m, 'e, V> { + map: &'m mut CandidateMap<'e, V>, + index: usize +} + +impl<'m, 'e, V> Iterator for IterMut<'m, 'e, V> { + type Item = (&'e Candidate, &'m mut V); + + fn next(&mut self) -> Option { + loop { + match self.map.keys.get(self.index) { + Some(k) => { + // Key within range + match k { + Some(kk) => { + // Key is set + + // SAFETY: Guaranteed to be set, as we update key and value at the same time + let v = unsafe { self.map.values.get_unchecked_mut(self.index).as_mut().unwrap_unchecked() }; + let v_ptr = v as *mut V; + + // SAFETY: Need unsafe pointer magic for IterMut + let vv = unsafe { &mut *v_ptr }; + + self.index += 1; + return Some((kk, vv)); + } + None => { + // Key is unset + self.index += 1; + continue; + } + } + } + None => { + // Key outside range + return None; + } + } + } + } +} + +pub struct Values<'m, 'e, V> { + map: &'m CandidateMap<'e, V>, + index: usize +} + +impl<'m, 'e, V> Iterator for Values<'m, 'e, V> { + type Item = &'m V; + + fn next(&mut self) -> Option { + loop { + match self.map.values.get(self.index) { + Some(v) => { + // Key within range + match v { + Some(vv) => { + // Key is set + self.index += 1; + return Some(vv); + } + None => { + // Key is unset + self.index += 1; + continue; + } + } + } + None => { + // Key outside range + return None; + } + } + } + } +} + +pub struct IntoIter<'e, V> { + map: CandidateMap<'e, V>, + index: usize +} + +impl<'e, V> Iterator for IntoIter<'e, V> { + type Item = (&'e Candidate, V); + + fn next(&mut self) -> Option { + loop { + match self.map.keys.get(self.index) { + Some(k) => { + // Key within range + match k { + Some(kk) => { + // Key is set + + // SAFETY: Guaranteed to be set, as we update key and value at the same time + let v = unsafe { self.map.values.get_unchecked_mut(self.index).take().unwrap_unchecked() }; + self.index += 1; + return Some((kk, v)); + } + None => { + // Key is unset + self.index += 1; + continue; + } + } + } + None => { + // Key outside range + return None; + } + } + } } } impl<'e, V> IntoIterator for CandidateMap<'e, V> { type Item = (&'e Candidate, V); - type IntoIter = hash_map::IntoIter<&'e Candidate, V>; + type IntoIter = IntoIter<'e, V>; fn into_iter(self) -> Self::IntoIter { - return self.map.into_iter(); + return IntoIter { map: self, index: 0 }; } } diff --git a/src/constraints.rs b/src/constraints.rs index 57ac608..2e458a1 100644 --- a/src/constraints.rs +++ b/src/constraints.rs @@ -788,7 +788,7 @@ pub fn init_repeat_count_rollback<'a, N: Number>(state: &mut CountState<'a, N>, // Copy ballot papers to rollback state for (candidate, count_card) in state.candidates.iter_mut() { - rollback_candidates.insert(*candidate, count_card.clone()); + rollback_candidates.insert(candidate, count_card.clone()); } state.rollback_state = RollbackState::NeedsRollback { candidates: Some(rollback_candidates), exhausted: Some(rollback_exhausted), constraint, group }; diff --git a/src/election.rs b/src/election.rs index 40d51da..ddc3603 100644 --- a/src/election.rs +++ b/src/election.rs @@ -261,8 +261,7 @@ impl<'a, N: Number> CountState<'a, N> { if opts.sort_votes { // Sort by votes if requested - candidates = self.candidates.iter() - .map(|(c, cc)| (*c, cc)).collect(); + candidates = self.candidates.iter().collect(); // First sort by order of election (as a tie-breaker, if votes are equal) candidates.sort_unstable_by(|a, b| b.1.order_elected.cmp(&a.1.order_elected)); // Then sort by votes diff --git a/src/stv/gregory/transfers.rs b/src/stv/gregory/transfers.rs index 255bfd7..00d7354 100644 --- a/src/stv/gregory/transfers.rs +++ b/src/stv/gregory/transfers.rs @@ -146,13 +146,13 @@ impl<'e, N: Number> TransferTable<'e, N> { // Candidate votes for (candidate, cell) in column.cells.iter_mut() { column.total.ballots += &cell.ballots; - self.total.add_transfers(*candidate, &cell.ballots); + self.total.add_transfers(candidate, &cell.ballots); self.total.total.ballots += &cell.ballots; let votes_in = cell.ballots.clone() * &column.value_fraction; cell.votes_in += &votes_in; column.total.votes_in += &votes_in; - self.total.cells.get_mut(*candidate).unwrap().votes_in += &votes_in; + self.total.cells.get_mut(candidate).unwrap().votes_in += &votes_in; self.total.total.votes_in += votes_in; } diff --git a/src/stv/mod.rs b/src/stv/mod.rs index ee41ed0..e2bc39e 100644 --- a/src/stv/mod.rs +++ b/src/stv/mod.rs @@ -1312,7 +1312,7 @@ where for<'r> &'r N: ops::Sub<&'r N, Output=N> { // Do not defer if this could change the last 2 candidates - let mut hopefuls: Vec<(&&Candidate, &CountCard)> = state.candidates.iter() + let mut hopefuls: Vec<(&Candidate, &CountCard)> = state.candidates.iter() .filter(|(_, cc)| cc.state == CandidateState::Hopeful || cc.state == CandidateState::Guarded) .collect(); @@ -1501,7 +1501,7 @@ fn hopefuls_below_threshold<'a, N: Number>(state: &CountState<'a, N>, opts: &STV let excluded_candidates: Vec<&Candidate> = state.candidates.iter() .filter_map(|(c, cc)| if cc.state == CandidateState::Hopeful && cc.votes <= min_threshold { - Some(*c) + Some(c) } else { None }) @@ -1520,7 +1520,7 @@ fn hopefuls_below_threshold<'a, N: Number>(state: &CountState<'a, N>, opts: &STV fn hopefuls_to_bulk_exclude<'a, N: Number>(state: &CountState<'a, N>, _opts: &STVOptions) -> Vec<&'a Candidate> { let mut excluded_candidates = Vec::new(); - let mut hopefuls: Vec<(&&Candidate, &CountCard)> = state.candidates.iter() + let mut hopefuls: Vec<(&Candidate, &CountCard)> = state.candidates.iter() .filter(|(_, cc)| cc.state == CandidateState::Hopeful) .collect(); @@ -1547,7 +1547,7 @@ fn hopefuls_to_bulk_exclude<'a, N: Number>(state: &CountState<'a, N>, _opts: &ST continue; } - let try_exclude: Vec<&Candidate> = try_exclude.iter().map(|(c, _)| **c).collect(); + let try_exclude: Vec<&Candidate> = try_exclude.iter().map(|(c, _)| *c).collect(); // Do not exclude if this violates constraints match constraints::test_constraints_any_time(state, &try_exclude, CandidateState::Excluded) { @@ -1639,7 +1639,7 @@ where for<'r> &'r N: ops::Div<&'r N, Output=N>, { // Cannot filter by raw vote count, as candidates may have 0.00 votes but still have recorded ballot papers - let mut excluded_with_votes: Vec<(&&Candidate, &CountCard)> = state.candidates.iter() + let mut excluded_with_votes: Vec<(&Candidate, &CountCard)> = state.candidates.iter() .filter(|(_, cc)| cc.state == CandidateState::Excluded && !cc.finalised) .collect(); @@ -1649,7 +1649,7 @@ where let order_excluded = excluded_with_votes[0].1.order_elected; let excluded_candidates: Vec<&Candidate> = excluded_with_votes.into_iter() .filter(|(_, cc)| cc.order_elected == order_excluded) - .map(|(c, _)| *c) + .map(|(c, _)| c) .collect(); let names: Vec<&str> = excluded_candidates.iter().map(|c| c.name.as_str()).sorted().collect(); @@ -1757,9 +1757,9 @@ fn init_tiebreaks(state: &mut CountState, opts: &STVOptions) { } // Sort candidates in this stage by votes, grouping by ties - let mut sorted_candidates: Vec<(&&Candidate, &CountCard)> = state.candidates.iter().collect(); + let mut sorted_candidates: Vec<(&Candidate, &CountCard)> = state.candidates.iter().collect(); sorted_candidates.sort_unstable_by(|a, b| a.1.votes.cmp(&b.1.votes)); - let sorted_candidates: Vec)>> = sorted_candidates.into_iter() + let sorted_candidates: Vec)>> = sorted_candidates.into_iter() .group_by(|(_, cc)| &cc.votes) .into_iter() .map(|(_, candidates)| candidates.collect()) @@ -1795,23 +1795,23 @@ fn update_tiebreaks(state: &mut CountState, _opts: &STVOptions) { } // Sort candidates in this stage by votes, grouping by ties - let mut sorted_candidates: Vec<(&&Candidate, &CountCard)> = state.candidates.iter().collect(); + let mut sorted_candidates: Vec<(&Candidate, &CountCard)> = state.candidates.iter().collect(); sorted_candidates.sort_unstable_by(|a, b| a.1.votes.cmp(&b.1.votes)); let sorted_candidates: Vec> = sorted_candidates.into_iter() .group_by(|(_, cc)| &cc.votes) .into_iter() - .map(|(_, candidates)| candidates.map(|(c, _)| *c).collect()) + .map(|(_, candidates)| candidates.map(|(c, _)| c).collect()) .collect(); // Update forwards tie-breaking order if let Some(hm) = state.forwards_tiebreak.as_mut() { // TODO: Check if already completely sorted - let mut sorted_last_round: Vec<(&&Candidate, &usize)> = hm.iter().collect(); + let mut sorted_last_round: Vec<(&Candidate, &usize)> = hm.iter().collect(); sorted_last_round.sort_unstable_by(|a, b| a.1.cmp(b.1)); let sorted_last_round: Vec> = sorted_last_round.into_iter() .group_by(|(_, v)| **v) .into_iter() - .map(|(_, group)| group.map(|(c, _)| *c).collect()) + .map(|(_, group)| group.map(|(c, _)| c).collect()) .collect(); let mut i: usize = 0;