Repository: tafia/quick-xml
Pull request #166 (2019-08-23 03:48:36)

Allow less bloat plus some speedup
Following [Raph Levien's post](https://raphlinus.github.io/rust/2019/08/21/rust-bloat.html) regarding bloat, allow quick-xml user to opt in for bloats..
* allow NOT using encoding_rs when we know input xml to be UTF8
* disable both failure and encoding_rs per default, use features `use-failure` and `encoding` to revert them back (**BREAKING CHANGE**)
* speed up reader even further leveraging on memchr3 iterator (about 18% better on quick-xml benches)


Patch:
diff --git a/Cargo.toml b/Cargo.toml
index 17374a0..9209072 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "quick-xml"
-version = "0.15.0"
+version = "0.16.0"
 authors = ["Johann Tuffe <tafia973@gmail.com>"]
 description = "High performance xml reader and writer"
 
@@ -16,11 +16,10 @@ license = "MIT"
 travis-ci = { repository = "tafia/quick-xml" }
 
 [dependencies]
-encoding_rs = "0.8.14"
+encoding_rs = { version = "0.8.17", optional = true }
 failure = { version = "0.1.5", optional = true }
-derive_more = "0.14"
-memchr = "2.1.2"
-log = "0.4.6"
+derive_more = "0.15"
+memchr = "2.2.1"
 
 [dev-dependencies]
 xml-rs = "0.8.0"
@@ -29,4 +28,6 @@ xml-rs = "0.8.0"
 bench = false
 
 [features]
-default = ["failure"]
+default = []
+encoding = ["encoding_rs"]
+use-failure = ["failure"]
diff --git a/Changelog.md b/Changelog.md
index a766c86..5b532f8 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -8,6 +8,10 @@
   - test: Adding missing tests
   - chore: Changes to the build process or auxiliary tools/libraries/documentation
 
+## 0.16.0
+- feat: set failure and encoding_rs crates as optional. You should now use respectively 
+`use-failure` and `encoding` features to get the old behavior
+
 ## 0.15.0
 - feat: remove Seek bound
 - style: rustfmt
diff --git a/README.md b/README.md
index f55b77f..c5f9998 100644
--- a/README.md
+++ b/README.md
@@ -8,7 +8,7 @@ High performance xml pull reader/writer.
 The reader:
 - is almost zero-copy (use of `Cow` whenever possible)
 - is easy on memory allocation (the API provides a way to reuse buffers)
-- support various encoding, namespaces resolution, special characters.
+- support various encoding (with `encoding` feature), namespaces resolution, special characters.
 
 [docs.rs](https://docs.rs/quick-xml)
 
@@ -105,6 +105,12 @@ let expected = r#"<my_elem k1="v1" k2="v2" my-key="some value"><child>text</chil
 assert_eq!(result, expected.as_bytes());
 ```
 
+# Features
+
+quick-xml supports 2 additional features, non activated by default:
+- `encoding`: support non utf8 xmls
+- `use-failure`: support for failure chainable error
+
 ## Performance
 
 Benchmarking is hard and the results depend on your input file and your machine.
@@ -113,12 +119,12 @@ Here on my particular file, quick-xml is around **50 times faster** than [xml-rs
 
 ```
 // quick-xml benches
-test bench_quick_xml            ... bench:     256,921 ns/iter (+/- 18,306)
-test bench_quick_xml_escaped    ... bench:     324,320 ns/iter (+/- 19,968)
-test bench_quick_xml_namespaced ... bench:     396,318 ns/iter (+/- 23,663)
+test bench_quick_xml            ... bench:     198,866 ns/iter (+/- 9,663)
+test bench_quick_xml_escaped    ... bench:     282,740 ns/iter (+/- 61,625)
+test bench_quick_xml_namespaced ... bench:     389,977 ns/iter (+/- 32,045)
 
 // same bench with xml-rs
-test bench_xml_rs               ... bench:  14,839,533 ns/iter (+/- 2,377,647)
+test bench_xml_rs               ... bench:  14,468,930 ns/iter (+/- 321,171)
 ```
 
 For a feature and performance comparison, you can also have a look at RazrFalcon's [choose-your-xml-rs](https://github.com/RazrFalcon/choose-your-xml-rs).
diff --git a/src/escape.rs b/src/escape.rs
index 0ecbc4d..1111212 100644
--- a/src/escape.rs
+++ b/src/escape.rs
@@ -185,9 +185,9 @@ fn parse_hexadecimal(bytes: &[u8]) -> Result<u32, EscapeError> {
     for &b in bytes {
         code <<= 4;
         code += match b {
-            b'0'...b'9' => b - b'0',
-            b'a'...b'f' => b - b'a' + 10,
-            b'A'...b'F' => b - b'A' + 10,
+            b'0'..=b'9' => b - b'0',
+            b'a'..=b'f' => b - b'a' + 10,
+            b'A'..=b'F' => b - b'A' + 10,
             b => return Err(EscapeError::InvalidHexadecimal(b as char)),
         } as u32;
     }
@@ -203,7 +203,7 @@ fn parse_decimal(bytes: &[u8]) -> Result<u32, EscapeError> {
     for &b in bytes {
         code *= 10;
         code += match b {
-            b'0'...b'9' => b - b'0',
+            b'0'..=b'9' => b - b'0',
             b => return Err(EscapeError::InvalidDecimal(b as char)),
         } as u32;
     }
diff --git a/src/events/attributes.rs b/src/events/attributes.rs
index a6329a4..54af99d 100644
--- a/src/events/attributes.rs
+++ b/src/events/attributes.rs
@@ -106,10 +106,27 @@ impl<'a> Attribute<'a> {
     ///
     /// [`unescaped_value()`]: #method.unescaped_value
     /// [`Reader::decode()`]: ../../reader/struct.Reader.html#method.decode
+    #[cfg(feature = "encoding_rs")]
     pub fn unescape_and_decode_value<B: BufRead>(&self, reader: &Reader<B>) -> Result<String> {
         self.unescaped_value()
             .map(|e| reader.decode(&*e).into_owned())
     }
+
+    /// Returns the unescaped and decoded string value.
+    ///
+    /// This allocates a `String` in all cases. For performance reasons it might be a better idea to
+    /// instead use one of:
+    ///
+    /// * [`unescaped_value()`], as it doesn't allocate when no escape sequences are used.
+    /// * [`Reader::decode()`], as it only allocates when the decoding can't be performed otherwise.
+    ///
+    /// [`unescaped_value()`]: #method.unescaped_value
+    /// [`Reader::decode()`]: ../../reader/struct.Reader.html#method.decode
+    #[cfg(not(feature = "encoding_rs"))]
+    pub fn unescape_and_decode_value<B: BufRead>(&self, reader: &Reader<B>) -> Result<String> {
+        self.unescaped_value()
+            .and_then(|e| reader.decode(&*e).map(|s| s.to_owned()))
+    }
 }
 
 impl<'a> From<(&'a [u8], &'a [u8])> for Attribute<'a> {
diff --git a/src/events/mod.rs b/src/events/mod.rs
index 07bb869..d388710 100644
--- a/src/events/mod.rs
+++ b/src/events/mod.rs
@@ -2,6 +2,7 @@
 
 pub mod attributes;
 
+#[cfg(feature = "encoding_rs")]
 use encoding_rs::Encoding;
 use std::borrow::Cow;
 use std::io::BufRead;
@@ -156,10 +157,27 @@ impl<'a> BytesStart<'a> {
     ///
     /// [`unescaped()`]: #method.unescaped
     /// [`Reader::decode()`]: ../reader/struct.Reader.html#method.decode
+    #[cfg(feature = "encoding_rs")]
     pub fn unescape_and_decode<B: BufRead>(&self, reader: &Reader<B>) -> Result<String> {
         self.unescaped().map(|e| reader.decode(&*e).into_owned())
     }
 
+    /// Returns the unescaped and decoded string value.
+    ///
+    /// This allocates a `String` in all cases. For performance reasons it might be a better idea to
+    /// instead use one of:
+    ///
+    /// * [`unescaped()`], as it doesn't allocate when no escape sequences are used.
+    /// * [`Reader::decode()`], as it only allocates when the decoding can't be performed otherwise.
+    ///
+    /// [`unescaped()`]: #method.unescaped
+    /// [`Reader::decode()`]: ../reader/struct.Reader.html#method.decode
+    #[cfg(not(feature = "encoding_rs"))]
+    pub fn unescape_and_decode<B: BufRead>(&self, reader: &Reader<B>) -> Result<String> {
+        self.unescaped()
+            .and_then(|e| reader.decode(&*e).map(|s| s.to_owned()))
+    }
+
     /// Adds an attribute to this element.
     pub fn push_attribute<'b, A: Into<Attribute<'b>>>(&mut self, attr: A) {
         let a = attr.into();
@@ -297,6 +315,7 @@ impl<'a> BytesDecl<'a> {
     }
 
     /// Gets the decoder struct
+    #[cfg(feature = "encoding_rs")]
     pub fn encoder(&self) -> Option<&'static Encoding> {
         self.encoding()
             .and_then(|e| e.ok())
@@ -424,10 +443,23 @@ impl<'a> BytesText<'a> {
     /// it might be wiser to manually use
     /// 1. BytesText::unescaped()
     /// 2. Reader::decode(...)
+    #[cfg(feature = "encoding_rs")]
     pub fn unescape_and_decode<B: BufRead>(&self, reader: &Reader<B>) -> Result<String> {
         self.unescaped().map(|e| reader.decode(&*e).into_owned())
     }
 
+    /// helper method to unescape then decode self using the reader encoding
+    ///
+    /// for performance reasons (could avoid allocating a `String`),
+    /// it might be wiser to manually use
+    /// 1. BytesText::unescaped()
+    /// 2. Reader::decode(...)
+    #[cfg(not(feature = "encoding_rs"))]
+    pub fn unescape_and_decode<B: BufRead>(&self, reader: &Reader<B>) -> Result<String> {
+        self.unescaped()
+            .and_then(|e| reader.decode(&*e).map(|s| s.to_owned()))
+    }
+
     /// Gets escaped content.
     pub fn escaped(&self) -> &[u8] {
         self.content.as_ref()
diff --git a/src/lib.rs b/src/lib.rs
index 00dc44a..b04ae82 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -102,9 +102,16 @@
 //! let expected = r#"<my_elem k1="v1" k2="v2" my-key="some value"><child>text</child></my_elem>"#;
 //! assert_eq!(result, expected.as_bytes());
 //! ```
+//!
+//! # Features
+//!
+//! quick-xml supports 2 additional features, non activated by default:
+//! - `encoding`: support non utf8 xmls
+//! - `use-failure`: support for failure chainable error
 #![deny(missing_docs)]
 #![recursion_limit = "1024"]
 
+#[cfg(feature = "encoding_rs")]
 extern crate encoding_rs;
 #[cfg(feature = "failure")]
 #[macro_use]
diff --git a/src/reader.rs b/src/reader.rs
index 2a66859..7f2b91b 100644
--- a/src/reader.rs
+++ b/src/reader.rs
@@ -1,16 +1,17 @@
 //! A module to handle `Reader`
 
+#[cfg(feature = "encoding_rs")]
 use std::borrow::Cow;
 use std::fs::File;
 use std::io::{self, BufRead, BufReader};
 use std::path::Path;
 use std::str::from_utf8;
 
+#[cfg(feature = "encoding_rs")]
 use encoding_rs::Encoding;
 
 use errors::{Error, Result};
-use events::attributes::Attribute;
-use events::{BytesDecl, BytesEnd, BytesStart, BytesText, Event};
+use events::{attributes::Attribute, BytesDecl, BytesEnd, BytesStart, BytesText, Event};
 
 use memchr;
 
@@ -84,6 +85,7 @@ pub struct Reader<B: BufRead> {
     opened_starts: Vec<usize>,
     /// a buffer to manage namespaces
     ns_buffer: NamespaceBufferIndex,
+    #[cfg(feature = "encoding_rs")]
     /// the encoding specified in the xml, defaults to utf8
     encoding: &'static Encoding,
 }
@@ -103,6 +105,7 @@ impl<B: BufRead> Reader<B> {
             buf_position: 0,
             check_comments: false,
             ns_buffer: NamespaceBufferIndex::default(),
+            #[cfg(feature = "encoding_rs")]
             encoding: ::encoding_rs::UTF_8,
         }
     }
@@ -420,6 +423,7 @@ impl<B: BufRead> Reader<B> {
 
     /// reads `BytesElement` starting with a `?`,
     /// return `Decl` or `PI` event
+    #[cfg(feature = "encoding_rs")]
     fn read_question_mark<'a, 'b>(&'a mut self, buf: &'b [u8]) -> Result<Event<'b>> {
         let len = buf.len();
         if len > 2 && buf[len - 1] == b'?' {
@@ -439,6 +443,25 @@ impl<B: BufRead> Reader<B> {
         }
     }
 
+    /// reads `BytesElement` starting with a `?`,
+    /// return `Decl` or `PI` event
+    #[cfg(not(feature = "encoding_rs"))]
+    fn read_question_mark<'a, 'b>(&'a mut self, buf: &'b [u8]) -> Result<Event<'b>> {
+        let len = buf.len();
+        if len > 2 && buf[len - 1] == b'?' {
+            if len > 5 && &buf[1..4] == b"xml" && is_whitespace(buf[4]) {
+                let event = BytesDecl::from_start(BytesStart::borrowed(&buf[1..len - 1], 3));
+                // Try getting encoding from the declaration event
+                Ok(Event::Decl(event))
+            } else {
+                Ok(Event::PI(BytesText::from_escaped(&buf[1..len - 1])))
+            }
+        } else {
+            self.buf_position -= len;
+            Err(Error::UnexpectedEof("XmlDecl".to_string()))
+        }
+    }
+
     #[inline]
     fn close_expanded_empty(&mut self) -> Result<Event<'static>> {
         self.tag_state = TagState::Closed;
@@ -660,6 +683,7 @@ impl<B: BufRead> Reader<B> {
     /// This encoding will be used by [`decode`].
     ///
     /// [`decode`]: #method.decode
+    #[cfg(feature = "encoding_rs")]
     pub fn encoding(&self) -> &'static Encoding {
         self.encoding
     }
@@ -671,10 +695,25 @@ impl<B: BufRead> Reader<B> {
     ///
     /// If no encoding is specified, defaults to UTF-8.
     #[inline]
+    #[cfg(feature = "encoding_rs")]
     pub fn decode<'b, 'c>(&'b self, bytes: &'c [u8]) -> Cow<'c, str> {
         self.encoding.decode(bytes).0
     }
 
+    /// Decodes a UTF8 slice regarless of XML declaration.
+    ///
+    /// Decode `bytes` with BOM sniffing and with malformed sequences replaced with the
+    /// `U+FFFD REPLACEMENT CHARACTER`.
+    ///
+    /// # Note
+    ///
+    /// If you instead want to use XML declared encoding, use the `encoding_rs` feature
+    #[inline]
+    #[cfg(not(feature = "encoding_rs"))]
+    pub fn decode<'c>(&self, bytes: &'c [u8]) -> Result<&'c str> {
+        from_utf8(bytes).map_err(Error::Utf8)
+    }
+
     /// Reads until end element is found
     ///
     /// Manages nested cases where parent and child elements have the same name
@@ -865,7 +904,7 @@ fn read_until<R: BufRead>(r: &mut R, byte: u8, buf: &mut Vec<u8>) -> Result<usiz
 #[inline]
 fn read_elem_until<R: BufRead>(r: &mut R, end_byte: u8, buf: &mut Vec<u8>) -> Result<usize> {
     #[derive(Clone, Copy)]
-    enum ElemReadState {
+    enum State {
         /// The initial state (inside element, but outside of attribute value)
         Elem,
         /// Inside a single-quoted attribute value
@@ -873,7 +912,7 @@ fn read_elem_until<R: BufRead>(r: &mut R, end_byte: u8, buf: &mut Vec<u8>) -> Re
         /// Inside a double-quoted attribute value
         DoubleQ,
     }
-    let mut state = ElemReadState::Elem;
+    let mut state = State::Elem;
     let mut read = 0;
     let mut done = false;
     while !done {
@@ -885,27 +924,24 @@ fn read_elem_until<R: BufRead>(r: &mut R, end_byte: u8, buf: &mut Vec<u8>) -> Re
                 Err(e) => return Err(Error::Io(e)),
             };
 
-            let mut bytes = available.iter().enumerate();
-
+            let mut memiter = memchr::memchr3_iter(end_byte, b'\'', b'"', available);
             let used: usize;
             loop {
-                match bytes.next() {
-                    Some((i, &b)) => {
-                        state = match (state, b) {
-                            (ElemReadState::Elem, b) if b == end_byte => {
+                match memiter.next() {
+                    Some(i) => {
+                        state = match (state, available[i]) {
+                            (State::Elem, b) if b == end_byte => {
                                 // only allowed to match `end_byte` while we are in state `Elem`
                                 buf.extend_from_slice(&available[..i]);
                                 done = true;
                                 used = i + 1;
                                 break;
                             }
-                            (ElemReadState::Elem, b'\'') => ElemReadState::SingleQ,
-                            (ElemReadState::Elem, b'\"') => ElemReadState::DoubleQ,
+                            (State::Elem, b'\'') => State::SingleQ,
+                            (State::Elem, b'\"') => State::DoubleQ,
 
                             // the only end_byte that gets us out if the same character
-                            (ElemReadState::SingleQ, b'\'') | (ElemReadState::DoubleQ, b'\"') => {
-                                ElemReadState::Elem
-                            }
+                            (State::SingleQ, b'\'') | (State::DoubleQ, b'\"') => State::Elem,
 
                             // all other bytes: no state change
                             _ => state,
@@ -959,12 +995,6 @@ struct Namespace {
 }
 
 impl Namespace {
-    /// Gets the prefix slice out of namespace buffer
-    #[inline]
-    fn prefix<'a, 'b>(&'a self, ns_buffer: &'b [u8]) -> &'b [u8] {
-        &ns_buffer[self.start..self.start + self.prefix_len]
-    }
-
     /// Gets the value slice out of namespace buffer
     ///
     /// Returns `None` if `value_len == 0`
@@ -973,10 +1003,19 @@ impl Namespace {
         if self.value_len == 0 {
             None
         } else {
-            Some(
-                &ns_buffer
-                    [self.start + self.prefix_len..self.start + self.prefix_len + self.value_len],
-            )
+            let start = self.start + self.prefix_len;
+            Some(&ns_buffer[start..start + self.value_len])
+        }
+    }
+
+    /// Check if the namespace matches the potentially qualified name
+    #[inline]
+    fn is_match(&self, ns_buffer: &[u8], qname: &[u8]) -> bool {
+        if self.prefix_len == 0 {
+            !qname.contains(&b':')
+        } else {
+            qname.get(self.prefix_len).map_or(false, |n| *n == b':')
+                && qname.starts_with(&ns_buffer[self.start..self.start + self.prefix_len])
         }
     }
 }
@@ -1004,15 +1043,10 @@ impl NamespaceBufferIndex {
         element_name: &'b [u8],
         buffer: &'c [u8],
     ) -> Option<&'c [u8]> {
-        let ns = match memchr::memchr(b':', element_name) {
-            None => self.slices.iter().rev().find(|n| n.prefix_len == 0),
-            Some(len) => self
-                .slices
-                .iter()
-                .rev()
-                .find(|n| n.prefix(buffer) == &element_name[..len]),
-        };
-        ns.and_then(|n| n.opt_value(buffer))
+        self.slices
+            .iter()
+            .rfind(|n| n.is_match(buffer, element_name))
+            .and_then(|n| n.opt_value(buffer))
     }
 
     fn pop_empty_namespaces(&mut self, buffer: &mut Vec<u8>) {
@@ -1092,29 +1126,18 @@ impl NamespaceBufferIndex {
         buffer: &'c [u8],
         use_default: bool,
     ) -> (Option<&'c [u8]>, &'b [u8]) {
-        match memchr::memchr(b':', qname) {
-            Some(len) => {
-                let (prefix, value) = qname.split_at(len);
-                let ns = self
-                    .slices
-                    .iter()
-                    .rev()
-                    .find(|n| n.prefix(buffer) == prefix)
-                    .and_then(|ns| ns.opt_value(buffer));
-                (ns, &value[1..])
-            }
-            None => {
-                let ns = if use_default {
-                    self.slices
-                        .iter()
-                        .rev()
-                        .find(|n| n.prefix_len == 0)
-                        .and_then(|ns| ns.opt_value(buffer))
+        self.slices
+            .iter()
+            .rfind(|n| n.is_match(buffer, qname))
+            .map_or((None, qname), |n| {
+                let len = n.prefix_len;
+                if len > 0 {
+                    (n.opt_value(buffer), &qname[len + 1..])
+                } else if use_default {
+                    (n.opt_value(buffer), qname)
                 } else {
-                    None
-                };
-                (ns, qname)
-            }
-        }
+                    (None, qname)
+                }
+            })
     }
 }


Test patch:
diff --git a/tests/test.rs b/tests/test.rs
index 845382c..2ea012f 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -325,6 +325,7 @@ fn test_default_ns_shadowing_expanded() {
 }
 
 #[test]
+#[cfg(feature = "encoding_rs")]
 fn test_koi8_r_encoding() {
     let src: &[u8] = include_bytes!("documents/opennews_all.rss");
     let mut r = Reader::from_reader(src as &[u8]);