perf: Skip table lookup per entry (toml-rs#203)

Before, for every key-value pair, we'd find the table and insert.  Now,
we store off the table as we come across each header and insert directly
into it (ignoring the lookups for dotted keys).

I was hoping this would drop us from N lookups to 1 lookup per table but
we need 2 lookups per table to ensure proper error reporting.

src/parser/document.rs

@@ -82,7 +82,7 @@ impl TomlParser {
     //                ( ws table ws [ comment ] ) /
     //                  ws )
     pub(crate) fn parse(s: &str) -> Result<Document, TomlError> {
-        let parser = RefCell::new(Self::default());
+        let mut parser = RefCell::new(Self::default());
         let input = Stream::new(s);
 
         let parsed = parse_ws(&parser)
@@ -103,7 +103,13 @@ impl TomlParser {
             Ok((_, ref rest)) if !rest.input.is_empty() => {
                 Err(TomlError::from_unparsed(rest.positioner, s))
             }
-            Ok(..) => Ok(*parser.into_inner().document),
+            Ok(..) => {
+                parser
+                    .get_mut()
+                    .finalize_table()
+                    .map_err(|e| TomlError::custom(e.to_string()))?;
+                Ok(*parser.into_inner().document)
+            }
             Err(e) => Err(TomlError::new(e, s)),
         }
     }
@@ -131,9 +137,7 @@ impl TomlParser {
             );
         }
 
-        let root = self.document.as_table_mut();
-        let table = Self::descend_path(root, self.current_table_path.as_slice(), 0, false)
-            .expect("the table path is valid; qed");
+        let table = &mut self.current_table;
         let table = Self::descend_path(table, &path, 0, true)?;
 
         // "Since tables cannot be defined more than once, redefining such tables using a [table] header is not allowed"

src/parser/errors.rs

@@ -24,7 +24,6 @@ impl TomlError {
         )
     }
 
-    #[cfg(feature = "easy")]
     pub(crate) fn custom(message: String) -> Self {
         Self { message }
     }

src/parser/mod.rs

@@ -20,21 +20,130 @@ pub(crate) use self::key::is_unquoted_char;
 pub(crate) use self::key::simple_key as key_parser;
 pub(crate) use self::value::value as value_parser;
 
-use crate::document::Document;
+use self::table::duplicate_key;
 use crate::key::Key;
+use crate::parser::errors::CustomError;
+use crate::repr::Decor;
+use crate::{ArrayOfTables, Document, Entry, Item, Table};
+use vec1::Vec1;
 
 pub(crate) struct TomlParser {
     document: Box<Document>,
-    current_table_path: Vec<Key>,
     current_table_position: usize,
+    current_table: Table,
+    current_is_array: bool,
+    current_table_path: Vec<Key>,
+}
+
+impl TomlParser {
+    pub(crate) fn start_aray_table(
+        &mut self,
+        path: Vec1<Key>,
+        decor: Decor,
+    ) -> Result<(), CustomError> {
+        debug_assert!(self.current_table.is_empty());
+        debug_assert!(self.current_table_path.is_empty());
+
+        // Look up the table on start to ensure the duplicate_key error points to the right line
+        let root = self.document.as_table_mut();
+        let parent_table = Self::descend_path(root, &path[..path.len() - 1], 0, false)?;
+        let key = &path[path.len() - 1];
+        let entry = parent_table
+            .entry_format(key)
+            .or_insert(Item::ArrayOfTables(ArrayOfTables::new()));
+        entry
+            .as_array_of_tables()
+            .ok_or_else(|| duplicate_key(&path, path.len() - 1))?;
+
+        self.current_table_position += 1;
+        self.current_table.decor = decor;
+        self.current_table.set_position(self.current_table_position);
+        self.current_is_array = true;
+        self.current_table_path = path.into_vec();
+
+        Ok(())
+    }
+
+    pub(crate) fn start_table(&mut self, path: Vec1<Key>, decor: Decor) -> Result<(), CustomError> {
+        debug_assert!(self.current_table.is_empty());
+        debug_assert!(self.current_table_path.is_empty());
+
+        // 1. Look up the table on start to ensure the duplicate_key error points to the right line
+        // 2. Ensure any child tables from an implicit table are preserved
+        let root = self.document.as_table_mut();
+        let parent_table = Self::descend_path(root, &path[..path.len() - 1], 0, false)?;
+        let key = &path[path.len() - 1];
+        if let Some(entry) = parent_table.remove(key.get()) {
+            match entry {
+                Item::Table(t) if t.implicit => {
+                    self.current_table = t;
+                }
+                _ => return Err(duplicate_key(&path, path.len() - 1)),
+            }
+        }
+
+        self.current_table_position += 1;
+        self.current_table.decor = decor;
+        self.current_table.set_position(self.current_table_position);
+        self.current_is_array = false;
+        self.current_table_path = path.into_vec();
+
+        Ok(())
+    }
+
+    pub(crate) fn finalize_table(&mut self) -> Result<(), CustomError> {
+        let mut table = std::mem::take(&mut self.current_table);
+        let path = std::mem::take(&mut self.current_table_path);
+
+        let root = self.document.as_table_mut();
+        if path.is_empty() {
+            assert!(root.is_empty());
+            std::mem::swap(&mut table, root);
+        } else if self.current_is_array {
+            let parent_table = Self::descend_path(root, &path[..path.len() - 1], 0, false)?;
+            let key = &path[path.len() - 1];
+
+            let entry = parent_table
+                .entry_format(key)
+                .or_insert(Item::ArrayOfTables(ArrayOfTables::new()));
+            let array = entry
+                .as_array_of_tables_mut()
+                .ok_or_else(|| duplicate_key(&path, path.len() - 1))?;
+            array.push(table);
+        } else {
+            let parent_table = Self::descend_path(root, &path[..path.len() - 1], 0, false)?;
+            let key = &path[path.len() - 1];
+
+            let entry = parent_table.entry_format(key);
+            match entry {
+                Entry::Occupied(entry) => {
+                    match entry.into_mut() {
+                        // if [a.b.c] header preceded [a.b]
+                        Item::Table(ref mut t) if t.implicit => {
+                            std::mem::swap(t, &mut table);
+                        }
+                        _ => return Err(duplicate_key(&path, path.len() - 1)),
+                    }
+                }
+                Entry::Vacant(entry) => {
+                    let item = Item::Table(table);
+                    entry.insert(item);
+                }
+            }
+        }
+
+        Ok(())
+    }
 }
 
 impl Default for TomlParser {
     fn default() -> Self {
         Self {
             document: Box::new(Document::new()),
-            current_table_path: Vec::new(),
             current_table_position: 0,
+            current_table: Table::new(),
+            current_is_array: false,
+            current_table_path: Vec::new(),
         }
     }
 }

src/parser/table.rs

@@ -1,11 +1,10 @@
-use crate::array_of_tables::ArrayOfTables;
 use crate::key::Key;
 use crate::parser::errors::CustomError;
 use crate::parser::key::key;
 use crate::parser::trivia::line_trailing;
 use crate::parser::TomlParser;
 use crate::repr::Decor;
-use crate::{Entry, Item, Table};
+use crate::{Item, Table};
 use combine::parser::char::char;
 use combine::parser::range::range;
 use combine::stream::RangeStream;
@@ -120,75 +119,20 @@ impl TomlParser {
     fn on_std_header(&mut self, path: Vec1<Key>, trailing: &str) -> Result<(), CustomError> {
         debug_assert!(!path.is_empty());
 
+        self.finalize_table()?;
         let leading = mem::take(&mut self.document.trailing);
-        let table = self.document.as_table_mut();
-        self.current_table_position += 1;
-
-        let table = Self::descend_path(table, &path[..path.len() - 1], 0, false)?;
-        let key = &path[path.len() - 1];
-
-        let decor = Decor::new(leading, trailing);
-
-        let entry = table.entry_format(key);
-        match entry {
-            Entry::Occupied(entry) => {
-                match entry.into_mut() {
-                    // if [a.b.c] header preceded [a.b]
-                    Item::Table(ref mut t) if t.implicit => {
-                        t.decor = decor;
-                        t.position = Some(self.current_table_position);
-                        t.set_implicit(false);
-
-                        self.current_table_path = path.into_vec();
-                        Ok(())
-                    }
-                    _ => Err(duplicate_key(&path, path.len() - 1)),
-                }
-            }
-            Entry::Vacant(entry) => {
-                let item = Item::Table(Table::with_decor_and_pos(
-                    decor,
-                    Some(self.current_table_position),
-                ));
-                entry.insert(item);
-                self.current_table_path = path.to_vec();
-                Ok(())
-            }
-        }
+        self.start_table(path, Decor::new(leading, trailing))?;
+
+        Ok(())
     }
 
     fn on_array_header(&mut self, path: Vec1<Key>, trailing: &str) -> Result<(), CustomError> {
         debug_assert!(!path.is_empty());
 
+        self.finalize_table()?;
         let leading = mem::take(&mut self.document.trailing);
-        let table = self.document.as_table_mut();
-
-        let key = &path[path.len() - 1];
-        let table = Self::descend_path(table, &path[..path.len() - 1], 0, false);
-
-        match table {
-            Ok(table) => {
-                if !table.contains_table(key.get()) && !table.contains_value(key.get()) {
-                    let decor = Decor::new(leading, trailing);
-
-                    let entry = table
-                        .entry_format(key)
-                        .or_insert(Item::ArrayOfTables(ArrayOfTables::new()));
-                    let array = entry.as_array_of_tables_mut().unwrap();
-
-                    self.current_table_position += 1;
-                    array.push(Table::with_decor_and_pos(
-                        decor,
-                        Some(self.current_table_position),
-                    ));
-                    self.current_table_path = path.into_vec();
-
-                    Ok(())
-                } else {
-                    Err(duplicate_key(&path, path.len() - 1))
-                }
-            }
-            Err(e) => Err(e),
-        }
+        self.start_aray_table(path, Decor::new(leading, trailing))?;
+
+        Ok(())
     }
 }

src/table.rs

@@ -38,14 +38,6 @@ impl Table {
         }
     }
 
-    pub(crate) fn with_decor_and_pos(decor: Decor, position: Option<usize>) -> Self {
-        Self {
-            decor,
-            position,
-            ..Default::default()
-        }
-    }
-
     /// Convert to an inline array
     pub fn into_inline_table(mut self) -> InlineTable {
         for (_, kv) in self.items.iter_mut() {