0.42.0 is about 30% slower than 0.41.0 on documents with many small events

Open
  • Is something wrong?
#1,039 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
55/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
rust
Domain
backend, performance

Research direction

Start by examining the UTF-8 validation added in #963 and the reader paths used by read_event_into and read_event; the issue provides a standalone benchmark repro in Cargo.toml and src/main.rs. Run the benchmark against 0.41.0 and 0.42.0, then compare event-level validation with the reported whole-document check. Done means preserving correct UTF-8 validation while reducing the measured regression on many-small-event documents.

Written by the indexing model from the issue text.

Description

encoding optimization

After upgrading from 0.41.0 to 0.42.0, reading a document made of many small events takes about 30% longer, with both read_event_into and read_event. The document in the repro below is shaped like a spreadsheet sheet: 200,000 rows, 2.4 million events, 19.9 MB.

We noticed it through calamine, which moved to 0.42 in tafia/calamine#711: reading a 200,000-row .xlsx sheet with worksheet_range takes about 12% longer with calamine 0.37.0 than with 0.36.1 (174 ms to 196 ms), and the added 21 ms matches what the parser alone adds on that sheet.

The per-event UTF-8 validation added in #963 looks like the cause. Adding a std::str::from_utf8 call on each event's bytes to 0.41.0 costs a little more than the upgrade does. Checking the whole document at once takes about 1 ms, so the cost seems to come from making one call per event, about 11 ns each, rather than from the checking itself.

19.9 MB, 2,400,005 events 0.41.0 0.42.0 Change
read_event_into, BufReader<&[u8]> 84.5 ms 110.8 ms +31%
read_event, &[u8] 57.5 ms 75.5 ms +31%
0.41.0 read_event_into plus from_utf8 per event 116.7 ms +38%
from_utf8 of the whole document 1.1 ms

Medians of 15 interleaved runs; a second run agreed within about 1%. With the encoding feature, the changes are +25% and +33%. Windows 11, AMD EPYC 7763, rustc 1.99.0, release build.

Two ideas, in case they help:

  • Reader::from_str takes input that is already valid UTF-8, but its events are still checked, because it reads through the &[u8] reader.
  • Checking larger spans at once, such as each chunk that fill_buf returns, might cost much less than one call per event.
Repro: Cargo.toml and src/main.rs 
[package]
name = "qx-utf8-bench"
version = "0.1.0"
edition = "2021"

[dependencies]
qx41 = { package = "quick-xml", version = "=0.41.0" }
qx42 = { package = "quick-xml", version = "=0.42.0" }

[features]
encoding = ["qx41/encoding", "qx42/encoding"]

use std::fmt::Write;
use std::hint::black_box;
use std::io::BufReader;
use std::time::Instant;

/// A document shaped like a spreadsheet sheet: one row per record, two cells each.
fn document(rows: usize) -> String {
    let mut xml = String::from(concat!(
        r#"<?xml version="1.0" encoding="UTF-8" standalone="yes"?>"#,
        r#"<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"><sheetData>"#
    ));
    for r in 1..=rows {
        write!(
            xml,
            r#"<row r="{r}" spans="1:2"><c r="A{r}" t="s"><v>{}</v></c><c r="B{r}"><v>{}</v></c></row>"#,
            r % 1000,
            r * 7
        )
        .unwrap();
    }
    xml.push_str("</sheetData></worksheet>");
    xml
}

fn buffered_041(xml: &[u8]) -> usize {
    let mut reader = qx41::Reader::from_reader(BufReader::new(xml));
    let mut buf = Vec::new();
    let mut events = 0;
    loop {
        match reader.read_event_into(&mut buf).unwrap() {
            qx41::events::Event::Eof => return events,
            _ => events += 1,
        }
        buf.clear();
    }
}

fn buffered_042(xml: &[u8]) -> usize {
    let mut reader = qx42::Reader::from_reader(BufReader::new(xml));
    let mut buf = Vec::new();
    let mut events = 0;
    loop {
        match reader.read_event_into(&mut buf).unwrap() {
            qx42::events::Event::Eof => return events,
            _ => events += 1,
        }
        buf.clear();
    }
}

fn borrowed_041(xml: &[u8]) -> usize {
    let mut reader = qx41::Reader::from_reader(xml);
    let mut events = 0;
    loop {
        match reader.read_event().unwrap() {
            qx41::events::Event::Eof => return events,
            _ => events += 1,
        }
    }
}

fn borrowed_042(xml: &[u8]) -> usize {
    let mut reader = qx42::Reader::from_reader(xml);
    let mut events = 0;
    loop {
        match reader.read_event().unwrap() {
            qx42::events::Event::Eof => return events,
            _ => events += 1,
        }
    }
}

/// 0.41.0 plus a `from_utf8` check of each event's bytes.
fn buffered_041_checked(xml: &[u8]) -> usize {
    use qx41::events::Event;
    let mut reader = qx41::Reader::from_reader(BufReader::new(xml));
    let mut buf = Vec::new();
    let mut events = 0;
    loop {
        let event = reader.read_event_into(&mut buf).unwrap();
        let bytes: &[u8] = match &event {
            Event::Eof => return events,
            Event::Start(e) | Event::Empty(e) => &**e,
            Event::End(e) => &**e,
            Event::Text(e) => &**e,
            _ => &[],
        };
        events += black_box(std::str::from_utf8(bytes)).is_ok() as usize;
        buf.clear();
    }
}

fn whole_document(xml: &[u8]) -> usize {
    std::str::from_utf8(xml).unwrap().len()
}

fn main() {
    let xml = document(200_000);
    let xml = xml.as_bytes();
    let cases: [(&str, fn(&[u8]) -> usize); 6] = [
        ("0.41.0 read_event_into, BufReader<&[u8]>", buffered_041),
        ("0.42.0 read_event_into, BufReader<&[u8]>", buffered_042),
        ("0.41.0 read_event, &[u8]", borrowed_041),
        ("0.42.0 read_event, &[u8]", borrowed_042),
        ("0.41.0 read_event_into + from_utf8 per event", buffered_041_checked),
        ("from_utf8 of the whole document", whole_document),
    ];
    let events = buffered_041(xml);
    for (name, case) in &cases[1..5] {
        assert_eq!(case(xml), events, "{name}");
    }
    println!("{:.1} MB, {events} events", xml.len() as f64 / 1e6);
    let rounds = 15;
    let mut times = vec![Vec::new(); cases.len()];
    for round in 0..rounds {
        for k in 0..cases.len() {
            let i = (k + round) % cases.len();
            let start = Instant::now();
            black_box((cases[i].1)(black_box(xml)));
            times[i].push(start.elapsed().as_secs_f64() * 1e3);
        }
    }
    for (i, (name, _)) in cases.iter().enumerate() {
        times[i].sort_by(f64::total_cmp);
        println!("{name:46} {:7.1} ms", times[i][rounds / 2]);
    }
}
Dominant language
Rust
Stars
1.6k
Forks
299
Avg merge
2d 4h
Merged PRs (30d)
9

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from tafia/quick-xml

All issues in tafia/quick-xml

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.