Skip to content

Commit

Permalink
perf: Avoid storing compiled dfa states twice
Browse files Browse the repository at this point in the history
  • Loading branch information
Markus Westerlind committed Sep 7, 2018
1 parent 320a145 commit 563dafb
Showing 1 changed file with 66 additions and 19 deletions.
85 changes: 66 additions & 19 deletions src/dfa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ articles on regexes is strongly recommended: https://swtch.com/~rsc/regexp/
implementation.)
*/

use std::collections::HashMap;
use std::fmt;
use std::iter::repeat;
use std::mem;
Expand All @@ -55,6 +54,8 @@ use exec::ProgramCache;
use prog::{Inst, Program};
use sparse::SparseSet;

use self::state_map::StateMap;

/// Return true if and only if the given program can be executed by a DFA.
///
/// Generally, a DFA is always possible. A pathological case where it is not
Expand Down Expand Up @@ -117,7 +118,7 @@ struct CacheInner {
/// things, we just pass indexes around manually. The performance impact of
/// this is probably an instruction or two in the inner loop. However, on
/// 64 bit, each StatePtr is half the size of a *State.
compiled: HashMap<Box<[u8]>, StatePtr>,
compiled: StateMap,
/// The transition table.
///
/// The transition table is laid out in row-major order, where states are
Expand All @@ -134,9 +135,6 @@ struct CacheInner {
/// bytes that never discriminate a distinct path through the DFA from each
/// other.
trans: Transitions,
/// Our set of states. Note that `StatePtr / num_byte_classes` indexes
/// this Vec rather than just a `StatePtr`.
states: Vec<State>,
/// A set of cached start states, which are limited to the number of
/// permutations of flags set just before the initial byte of input. (The
/// index into this vec is a `EmptyFlags`.)
Expand Down Expand Up @@ -269,7 +267,7 @@ impl<T> Result<T> {
/// it is packed into a single byte; Otherwise the byte 128 (-128 as an i8)
/// is coded as a flag, followed by 4 bytes encoding the delta.
#[derive(Clone, Eq, Hash, PartialEq)]
struct State{
pub struct State{
data: Box<[u8]>,
}

Expand Down Expand Up @@ -430,9 +428,8 @@ impl Cache {
let starts = vec![STATE_UNKNOWN; 256];
let mut cache = Cache {
inner: CacheInner {
compiled: HashMap::new(),
compiled: StateMap::new(),
trans: Transitions::new(num_byte_classes),
states: vec![],
start_states: starts,
stack: vec![],
flush_count: 0,
Expand Down Expand Up @@ -1182,7 +1179,7 @@ impl<'a> Fsm<'a> {
Some(v) => v,
}
// In the cache? Cool. Done.
if let Some(&si) = self.cache.compiled.get(&self.cache.insts_scratch_space[..]) {
if let Some(si) = self.cache.compiled.get_ptr(&self.cache.insts_scratch_space[..]) {
return Some(si);
}

Expand Down Expand Up @@ -1278,7 +1275,7 @@ impl<'a> Fsm<'a> {
&mut self,
current_state: Option<&mut StatePtr>,
) -> bool {
if self.cache.states.is_empty() {
if self.cache.compiled.is_empty() {
// Nothing to clear...
return true;
}
Expand Down Expand Up @@ -1308,7 +1305,7 @@ impl<'a> Fsm<'a> {
// 10 or fewer bytes per state.
// Additionally, we permit the cache to be flushed a few times before
// caling it quits.
let nstates = self.cache.states.len();
let nstates = self.cache.compiled.len();
if self.cache.flush_count >= 3
&& self.at >= self.last_cache_flush
&& (self.at - self.last_cache_flush) <= 10 * nstates {
Expand All @@ -1327,7 +1324,6 @@ impl<'a> Fsm<'a> {
};
self.cache.reset_size();
self.cache.trans.clear();
self.cache.states.clear();
self.cache.compiled.clear();
for s in &mut self.cache.start_states {
*s = STATE_UNKNOWN;
Expand All @@ -1347,7 +1343,7 @@ impl<'a> Fsm<'a> {
fn restore_state(&mut self, state: State) -> Option<StatePtr> {
// If we've already stored this state, just return a pointer to it.
// None will be the wiser.
if let Some(&si) = self.cache.compiled.get(&state.data) {
if let Some(si) = self.cache.compiled.get_ptr(&state.data) {
return Some(si);
}
self.add_state(state)
Expand Down Expand Up @@ -1488,7 +1484,7 @@ impl<'a> Fsm<'a> {

/// Returns a reference to a State given a pointer to it.
fn state(&self, si: StatePtr) -> &State {
&self.cache.states[si as usize / self.num_byte_classes()]
self.cache.compiled.get_state(si as usize / self.num_byte_classes()).unwrap()
}

/// Adds the given state to the DFA.
Expand Down Expand Up @@ -1524,13 +1520,10 @@ impl<'a> Fsm<'a> {
+ (2 * state.data.len())
+ (2 * mem::size_of::<State>())
+ mem::size_of::<StatePtr>();
self.cache.states.push(state.clone());
self.cache.compiled.insert(state.data, si);
self.cache.compiled.insert(state, si);
// Transition table and set of states and map should all be in sync.
debug_assert!(self.cache.states.len()
debug_assert!(self.cache.compiled.len()
== self.cache.trans.num_states());
debug_assert!(self.cache.states.len()
== self.cache.compiled.len());
Some(si)
}

Expand Down Expand Up @@ -1853,6 +1846,60 @@ fn read_varu32(data: &[u8]) -> (u32, usize) {
(0, 0)
}

mod state_map {
use std::collections::HashMap;
use std::mem;

use super::{State, StatePtr};

#[derive(Debug)]
pub struct StateMap {
/// The keys are not actually static but rely on always pointing to a buffer in `states`
/// which will never be moved except when clearing the map or on drop, in which case the
/// keys of this map will be removed before
map: HashMap<&'static [u8], StatePtr>,
/// Our set of states. Note that `StatePtr / num_byte_classes` indexes
/// this Vec rather than just a `StatePtr`.
states: Vec<State>,
}

impl StateMap {
pub fn new() -> StateMap {
StateMap {
map: HashMap::new(),
states: Vec::new(),
}
}

pub fn len(&self) -> usize {
self.states.len()
}

pub fn is_empty(&self) -> bool {
self.states.is_empty()
}

pub fn get_ptr(&self, index: &[u8]) -> Option<StatePtr> {
self.map.get(index).cloned()
}

pub fn get_state(&self, index: usize) -> Option<&State> {
self.states.get(index)
}

pub fn insert(&mut self, state: State, si: StatePtr) {
self.map
.insert(unsafe { mem::transmute::<&[u8], &[u8]>(&state.data) }, si);
self.states.push(state);
}

pub fn clear(&mut self) {
self.map.clear();
self.states.clear();
}
}
}

#[cfg(test)]
mod tests {
extern crate rand;
Expand Down

0 comments on commit 563dafb

Please sign in to comment.