ordered binary search working - but need to be sped up

README.md

@@ -4,7 +4,7 @@ Copyright - Yehowshua Immanuel
 
 ## Current Features
  - pretty fast, parses 3.04 GB VCD file in ~54s on M1 Macbook Air with 
-   respect to 50s with GTKWave on the same device. FastWave currently
+   respect to 30s with GTKWave on the same device. FastWave currently
    offers highly robust error handling which GTKWave doesn't have.
    
    I noticed that when running FastWave in the VsCode terminal as opposed
@@ -40,6 +40,7 @@ Here's a command to test on a malformed VCD:
 
 ## Features
  - [ ] macro for getting line number when propagating errors
+ - [ ] search for any unwraps or any direct vectors indexing
  - [ ] re-order all signal timelines as binary balanced trees with respect to timestamps
        - support multithreaded re-ordering
  - [ ] looks into making a macro for filename and linenumber later

src/vcd/parse.rs

@@ -1,6 +1,6 @@
-use std::{fs::File};
-use std::collections::HashMap;
 use num::BigInt;
+use std::collections::HashMap;
+use std::fs::File;
 
 use super::*;
 
@@ -19,7 +19,44 @@ use scopes::*;
 mod events;
 use events::*;
 
-pub fn parse_vcd(file : File) -> Result<VCD, String> {
+use std::cmp::Ordering;
+
+fn compare_strs(a: &str, b: &str) -> Ordering {
+    let last_idx = if a.len() > b.len() { a.len() } else { b.len() };
+    // let last_idx += -1;
+    Ordering::Less
+}
+
+fn ordered_binary_lookup(map: &Vec<(String, SignalIdx)>, key: &str) -> Result<SignalIdx, String> {
+    let mut upper_idx = map.len() - 1;
+    let mut lower_idx = 0usize;
+
+    while lower_idx <= upper_idx {
+        let mid_idx = lower_idx + ((upper_idx - lower_idx) / 2);
+        let (str_val, signal_idx) = map.get(mid_idx).unwrap();
+        let ordering = key.partial_cmp(str_val.as_str()).unwrap();
+
+        match ordering {
+            Ordering::Less => {
+                upper_idx = mid_idx - 1;
+            }
+            Ordering::Equal => {
+                return Ok(*signal_idx);
+            }
+            Ordering::Greater => {
+                lower_idx = mid_idx + 1;
+            }
+        }
+    }
+
+    return Err(format!(
+        "Error near {}:{}. Unable to find key: `{key}` in the map.",
+        file!(),
+        line!()
+    ));
+}
+
+pub fn parse_vcd(file: File) -> Result<VCD, String> {
     let mut word_gen = WordReader::new(file);
 
     let header = parse_metadata(&mut word_gen)?;
@@ -29,17 +66,48 @@ pub fn parse_vcd(file : File) -> Result<VCD, String> {
     let mut signal_map = std::collections::HashMap::new();
 
     // after we parse metadata, we form the VCD object
-    let mut vcd = VCD{
+    let mut vcd = VCD {
-        metadata           : header,
+        metadata: header,
-        timeline           : vec![],
+        timeline: vec![],
-        timeline_markers   : vec![],
+        timeline_markers: vec![],
-        all_signals        : vec![],
+        all_signals: vec![],
-        all_scopes         : vec![],
+        all_scopes: vec![],
-        scope_roots        : vec![],
+        scope_roots: vec![],
     };
 
     parse_scopes(&mut word_gen, &mut vcd, &mut signal_map)?;
-    parse_events(&mut word_gen, &mut vcd, &mut signal_map)?;
+
+    // the signal map should not contain any empty string
+    for (k, v) in &signal_map {
+        if k.len() == 0 {
+            return Err(format!("Critical error near {}:{}. There should be no empty strings in vcd string -> SignalIdx hashmap.", file!(), line!()));
+        }
+    }
+
+    // now that we've parsed all scopes and filled the hashmap
+    // with signals, we convert hashmap to an ordered vector
+    let mut signal_map1: Vec<(String, SignalIdx)> = signal_map
+        .iter()
+        .map(|(string, idx)| (string.clone(), idx.clone()))
+        .collect();
+    signal_map1.sort_by(|a: &(String, SignalIdx), b: &(String, SignalIdx)| {
+        let a = &a.0;
+        let b = &b.0;
+        a.partial_cmp(&b).unwrap()
+    });
+
+    let now = std::time::Instant::now();
+    for (k, v) in &signal_map1 {
+        let signal_idx = ordered_binary_lookup(&signal_map1, k.as_str())?;
+        assert!(*v == signal_idx);
+    }
+    let ordered_binary_search_elapsed = now.elapsed();
+    println!(
+        "ordered_binary_search_elapsed: {:.2?}",
+        ordered_binary_search_elapsed
+    );
+
+    // parse_events(&mut wosrd_gen, &mut vcd, &mut signal_map)?;
 
     Ok(vcd)
 }
@@ -55,29 +123,18 @@ mod tests {
         // two loops
         // testing dates
         for file in test::GOOD_DATE_FILES {
-            let metadata = parse_metadata(
+            let metadata = parse_metadata(&mut WordReader::new(File::open(file).unwrap()));
-                &mut WordReader::new(
-                    File::open(file)
-                    .unwrap()
-                )
-            );
             assert!(metadata.is_ok());
             assert!(metadata.unwrap().date.is_some());
         }
 
         for file in test::FILES {
-            let metadata = parse_metadata(
+            let metadata = parse_metadata(&mut WordReader::new(File::open(file).unwrap()));
-                &mut WordReader::new(
-                    File::open(file)
-                    .unwrap()
-                )
-            );
             assert!(metadata.is_ok());
 
             let (scalar, _timescale) = metadata.unwrap().timescale;
             assert!(scalar.is_some());
         }
-
     }
 
     #[test]
@@ -94,6 +151,5 @@ mod tests {
 
             // assert!(vcd.is_ok());
         }
-
     }
 }

src/vcd/types.rs

@@ -4,18 +4,27 @@ use chrono::prelude::*;
 pub(super) struct Version(pub String);
 
 #[derive(Debug)]
-pub(super) enum Timescale {Fs, Ps, Ns, Us, Ms, S, Unit}
+pub(super) enum Timescale {
+    Fs,
+    Ps,
+    Ns,
+    Us,
+    Ms,
+    S,
+    Unit,
+}
 
 #[derive(Debug)]
 pub(super) struct Metadata {
-    pub(super) date      : Option<DateTime<Utc>>,
+    pub(super) date: Option<DateTime<Utc>>,
-    pub(super) version   : Option<Version>,
+    pub(super) version: Option<Version>,
-    pub(super) timescale : (Option<u32>, Timescale)}
+    pub(super) timescale: (Option<u32>, Timescale),
+}
 
 #[derive(Debug, Copy, Clone)]
 pub(super) struct ScopeIdx(pub(super) usize);
 
-#[derive(Debug, Copy, Clone)]
+#[derive(Debug, Copy, Clone, PartialEq)]
 pub(super) struct SignalIdx(pub(super) usize);
 
 #[derive(Debug, Copy, Clone)]
@@ -25,62 +34,70 @@ pub(super) struct TimelineIdx(pub(super) u32);
 pub struct StartIdx(pub(super) u32);
 
 #[derive(Debug)]
-pub(super) enum SigType {Integer, Parameter, Real, Reg, Str, Wire, Tri1, Time}
+pub(super) enum SigType {
+    Integer,
+    Parameter,
+    Real,
+    Reg,
+    Str,
+    Wire,
+    Tri1,
+    Time,
+}
 
 #[derive(Debug)]
-pub(super) enum Signal{
+pub(super) enum Signal {
-    Data{
+    Data {
-         name             : String,
+        name: String,
-         sig_type         : SigType,
+        sig_type: SigType,
         // I've seen a 0 bit signal parameter in a xilinx
         // simulation before that gets assigned 1 bit values.
         // I consider this to be bad behavior. We capture such
         // errors in the following type.
-         signal_error     : Option<String>,
+        signal_error: Option<String>,
-         num_bits         : Option<usize>,
+        num_bits: Option<usize>,
         // TODO : may be able to remove self_idx
-         self_idx         : SignalIdx,
+        self_idx: SignalIdx,
         // we could encounter a mix of pure values and strings
         // for the same signal timeline
-         u8_timeline             : Vec<u8>,
+        u8_timeline: Vec<u8>,
-         u8_timeline_markers     : Vec<TimelineIdx>,
+        u8_timeline_markers: Vec<TimelineIdx>,
-         string_timeline         : Vec<String>,
+        string_timeline: Vec<String>,
-         string_timeline_markers : Vec<TimelineIdx>,
+        string_timeline_markers: Vec<TimelineIdx>,
-         scope_parent : ScopeIdx},
+        scope_parent: ScopeIdx,
-    Alias{
+    },
-         name         : String,
+    Alias {
-         signal_alias : SignalIdx}
+        name: String,
+        signal_alias: SignalIdx,
+    },
 }
 
 #[derive(Debug)]
 pub(super) struct Scope {
-    pub(super) name          : String,
+    pub(super) name: String,
 
-    pub(super) parent_idx    : Option<ScopeIdx>,
+    pub(super) parent_idx: Option<ScopeIdx>,
-    pub(super) self_idx      : ScopeIdx,
+    pub(super) self_idx: ScopeIdx,
-
-    pub(super) child_signals : Vec<SignalIdx>,
-    pub(super) child_scopes  : Vec<ScopeIdx>}
 
+    pub(super) child_signals: Vec<SignalIdx>,
+    pub(super) child_scopes: Vec<ScopeIdx>,
+}
 
 // TODO: document how timeline is represented
 #[derive(Debug)]
 pub struct VCD {
-    pub(super) metadata    : Metadata,
+    pub(super) metadata: Metadata,
-    pub timeline           : Vec<u8>,
+    pub timeline: Vec<u8>,
-    pub timeline_markers   : Vec<StartIdx>,
+    pub timeline_markers: Vec<StartIdx>,
-    pub(super) all_signals : Vec<Signal>,
+    pub(super) all_signals: Vec<Signal>,
-    pub(super) all_scopes  : Vec<Scope>,
+    pub(super) all_scopes: Vec<Scope>,
-    pub(super) scope_roots : Vec<ScopeIdx>}
+    pub(super) scope_roots: Vec<ScopeIdx>,
+}
 
 impl VCD {
     // TODO : make this a generic traversal function that applies specified
     // functions upon encountering scopes and signals
-    fn print_scope_tree(
+    fn print_scope_tree(&self, root_scope_idx: ScopeIdx, depth: usize) {
-        &self,
-        root_scope_idx : ScopeIdx,
-        depth : usize)
-    {
         let all_scopes = &self.all_scopes;
         let all_signals = &self.all_signals;
 
@@ -94,17 +111,16 @@ impl VCD {
         for SignalIdx(ref signal_idx) in &root_scope.child_signals {
             let child_signal = &all_signals[*signal_idx];
             let name = match child_signal {
-                Signal::Data{name, ..} => {name}
+                Signal::Data { name, .. } => name,
-                Signal::Alias{name, ..} => {name}
+                Signal::Alias { name, .. } => name,
             };
             println!("{indent} - sig: {name}")
         }
         println!();
 
         for scope_idx in &root_scope.child_scopes {
-            self.print_scope_tree(*scope_idx, depth+1);
+            self.print_scope_tree(*scope_idx, depth + 1);
         }
-
     }
 
     pub fn print_scopes(&self) {
@@ -120,19 +136,19 @@ impl VCD {
 
         for signal in &self.all_signals {
             match signal {
-                Signal::Alias {..} => {}
+                Signal::Alias { .. } => {}
                 Signal::Data {
                     name,
                     self_idx,
                     u8_timeline,
-                    .. } => {
+                    ..
+                } => {
                     if u8_timeline.len() > max_len {
                         max_len = u8_timeline.len();
                         let SignalIdx(idx_usize) = self_idx;
                         idx = *idx_usize;
                         signal_name = name.clone();
                     }
-
                 }
             }
         }