Improve performance when writing lp and reading sol files

We improve the performance of the solution file parser in the case of
cplex, by making use of the more performant quick-xml crate and
buffering the reads through `BufReader`. quick-xml indeed requires the
reader to be buffered, probably to avoid making too many read syscalls.

Buffering is also applied to writing the lp file. Manual benchmarks are
showing significant improvements over non-buffered read/writes for
problem sizes of several thousands of variables

Cargo.toml

@@ -27,3 +27,4 @@ cplex = ["xml-rs"]
 [dependencies]
 tempfile = "3"
 xml-rs = { version = "0.8.3", optional = true }
+quick-xml = "0.31"

src/lp_format.rs

@@ -4,6 +4,7 @@ use std::cmp::Ordering;
 use std::fmt;
 use std::fmt::Formatter;
 use std::io::prelude::*;
+use std::io::BufWriter;
 use std::io::Result;
 
 use tempfile::NamedTempFile;
@@ -136,8 +137,16 @@ pub trait LpProblem<'a>: Sized {
             .prefix(self.name())
             .suffix(".lp")
             .tempfile()?;
-        write!(f, "{}", self.display_lp())?;
-        f.flush()?;
+
+        // Use a buffered writer to limit the number of syscalls
+        let mut buf_f = BufWriter::new(&mut f);
+        write!(buf_f, "{}", self.display_lp())?;
+        buf_f.flush()?;
+
+        // need to explicitly drop the buffered writer here,
+        // since it holds a reference to the actual file
+        drop(buf_f);
+
         Ok(f)
     }
 }

src/solvers/cplex.rs

@@ -4,10 +4,11 @@
 use std::collections::HashMap;
 use std::ffi::OsString;
 use std::fs::File;
+use std::io::BufReader;
 use std::path::Path;
 
-use xml::reader::XmlEvent;
-use xml::EventReader;
+use quick_xml::events::{BytesStart, Event};
+use quick_xml::Reader;
 
 use crate::lp_format::LpProblem;
 use crate::solvers::{Solution, SolverProgram, SolverWithSolutionParsing, Status, WithMipGap};
@@ -95,57 +96,176 @@ impl SolverProgram for Cplex {
     }
 }
 
+fn extract_variable_name_and_value_from_event(
+    variable_event: BytesStart,
+) -> Result<(String, f32), String> {
+    let mut name = None;
+    let mut value = None;
+    for attribute in variable_event.attributes() {
+        let attribute = attribute.map_err(|e| format!("attribute error: {}", e))?;
+        match attribute.key.as_ref() {
+            b"name" => name = Some(String::from_utf8_lossy(attribute.value.as_ref()).to_string()),
+            b"value" => {
+                value = Some(
+                    String::from_utf8_lossy(attribute.value.as_ref())
+                        .parse()
+                        .map_err(|e| format!("invalid variable value for {:?}: {}", name, e))?,
+                );
+            }
+            _ => {}
+        }
+    }
+
+    name.and_then(|name| value.map(|value| (name, value)))
+        .ok_or_else(|| "name and value not found for variable".to_string())
+}
+
+fn read_specific_solution(f: &File, variables_len: Option<usize>) -> Result<Solution, String> {
+    let results = variables_len
+        .map(HashMap::with_capacity)
+        .unwrap_or_default();
+
+    let mut solution = Solution {
+        status: Status::Optimal,
+        results,
+    };
+
+    let f = BufReader::new(f);
+    let mut reader = Reader::from_reader(f);
+    let mut buf = Vec::new();
+
+    loop {
+        match reader.read_event_into(&mut buf) {
+            Err(e) => {
+                return Err(format!(
+                    "Error at position {}: {:?}",
+                    reader.buffer_position(),
+                    e
+                ))
+            }
+            // exits the loop when reaching end of file
+            Ok(Event::Eof) => {
+                break;
+            }
+            // we reached the "variables" section, where the variables to parse are
+            Ok(Event::Start(e)) if e.local_name().as_ref() == b"variables" => loop {
+                match reader.read_event_into(&mut buf) {
+                    // we matched either the start of a "variable" tag, or a "variable" tag without body
+                    Ok(Event::Empty(e)) | Ok(Event::Start(e))
+                        if e.local_name().as_ref() == b"variable" =>
+                    {
+                        // let's try to parse the variable name and value
+                        let (name, value) = extract_variable_name_and_value_from_event(e)?;
+                        solution.results.insert(name, value);
+                    }
+                    // we reached the end of the "variables" section, at this point all the variables should have been parsed.
+                    // we can safely return
+                    Ok(Event::End(e)) if e.local_name().as_ref() == b"variables" => {
+                        return Ok(solution);
+                    }
+                    Err(e) => {
+                        return Err(format!(
+                            "Error at position {}: {:?}",
+                            reader.buffer_position(),
+                            e
+                        ))
+                    }
+                    // an end-of-file here would be an error, since the 'variables' section would not be terminated
+                    Ok(Event::Eof) => {
+                        return Err(format!(
+                            "Error at position {}: Unterminated variables section",
+                            reader.buffer_position(),
+                        ))
+                    }
+                    _ => {}
+                }
+            },
+            // There are several other `Event`s we do not consider here
+            _ => {}
+        }
+    }
+
+    Ok(solution)
+}
+
 impl SolverWithSolutionParsing for Cplex {
     fn read_specific_solution<'a, P: LpProblem<'a>>(
         &self,
         f: &File,
         problem: Option<&'a P>,
     ) -> Result<Solution, String> {
-        let len = problem.map(|p| p.variables().size_hint().0).unwrap_or(0);
-        let parser = EventReader::new(f);
-        let mut solution = Solution {
-            status: Status::Optimal,
-            results: HashMap::with_capacity(len),
-        };
-        for e in parser {
-            match e {
-                Ok(XmlEvent::StartElement {
-                    name, attributes, ..
-                }) => {
-                    if name.local_name == "variable" {
-                        let mut name = None;
-                        let mut value = None;
-                        for attr in attributes {
-                            match attr.name.local_name.as_str() {
-                                "name" => name = Some(attr.value),
-                                "value" => {
-                                    let parsed = attr.value.parse().map_err(|e| {
-                                        format!("invalid variable value for {:?}: {}", name, e)
-                                    })?;
-                                    value = Some(parsed)
-                                }
-                                _ => {}
-                            };
-                        }
-                        if let (Some(name), Some(value)) = (name, value) {
-                            solution.results.insert(name, value);
-                        }
-                    }
-                }
-                Err(e) => return Err(format!("xml error: {}", e)),
-                _ => {}
-            }
-        }
-        Ok(solution)
+        let len = problem.map(|p| p.variables().size_hint().0);
+        read_specific_solution(f, len)
     }
 }
 
 #[cfg(test)]
 mod tests {
+    use super::read_specific_solution;
     use crate::solvers::{Cplex, SolverProgram, WithMipGap};
+    use std::collections::HashMap;
     use std::ffi::OsString;
+    use std::io::{Seek, Write};
     use std::path::Path;
 
+    const SAMPLE_SOL_FILE: &str = r##"<?xml version = "1.0" standalone="yes"?>
+<?xml-stylesheet href="https://www.ilog.com/products/cplex/xmlv1.0/solution.xsl" type="text/xsl"?>
+<CPLEXSolution version="1.2">
+ <header
+   problemName="../../../examples/data/mexample.mps"
+   solutionName="incumbent"
+   solutionIndex="-1"
+   objectiveValue="-122.5"
+   solutionTypeValue="3"
+   solutionTypeString="primal"
+   solutionStatusValue="101"
+   solutionStatusString="integer optimal solution"
+   solutionMethodString="mip"
+   primalFeasible="1"
+   dualFeasible="1"
+   MIPNodes="0"
+   MIPIterations="3"/>
+ <quality
+   epInt="1e-05"
+   epRHS="1e-06"
+   maxIntInfeas="0"
+   maxPrimalInfeas="0"
+   maxX="40"
+   maxSlack="2"/>
+ <linearConstraints>
+  <constraint name="c1" index="0" slack="0"/>
+  <constraint name="c2" index="1" slack="2"/>
+  <constraint name="c3" index="2" slack="0"/>
+ </linearConstraints>
+ <variables>
+  <variable name="x1" index="0" value="40"/>
+  <variable name="x2" index="1" value="10.5"/>
+  <variable name="x3" index="2" value="19.5"/>
+  <variable name="x4" index="3" value="3"/>
+ </variables>
+</CPLEXSolution>"##;
+
+    #[test]
+    fn sol_file_parsing() {
+        let mut tmpfile = tempfile::tempfile().expect("unable to create tempfile");
+        tmpfile
+            .write_all(SAMPLE_SOL_FILE.as_bytes())
+            .expect("unable to write sol file to tempfile");
+        tmpfile.rewind().expect("unable to rewind sol file");
+
+        let solution = read_specific_solution(&tmpfile, None).expect("failed to read sol file");
+
+        assert_eq!(
+            solution.results,
+            HashMap::from([
+                ("x1".to_owned(), 40.0),
+                ("x2".to_owned(), 10.5),
+                ("x3".to_owned(), 19.5),
+                ("x4".to_owned(), 3.0)
+            ])
+        );
+    }
+
     #[test]
     fn cli_args_default() {
         let solver = Cplex::default();