Use memchr for the two byte-at-a-time loops in the parse path - #142
Closed
kurok wants to merge 2 commits into
Closed
Use memchr for the two byte-at-a-time loops in the parse path#142kurok wants to merge 2 commits into
kurok wants to merge 2 commits into
Conversation
find_from_u8 scanned byte by byte, and on a multipart message it is where parse_mail spends its time: sampling a parse of a 767 KiB message with four base64 attachments put 96% of the cycles in find_from_u8_line_prefix. The loop's speed also depended on where the linker happened to place it. The same instructions ran at half speed on x86-64 when the loop straddled a 64-byte boundary, so unrelated changes in a dependent crate -- a version bump, a different rustc -- moved parse time by 2x with no change to this code. memmem is a vectorised search with runtime CPU dispatch: faster at every input size, and laid out independently of whoever calls it. Measured from a dependent crate on the message above: full parse 1.10 ms -> 0.76 ms; a structure-only parse that decodes no bodies 0.37 ms -> 0.03 ms. Semantics are unchanged: the first occurrence of `key` at or after `ix_start`, None when there is none. The two asserts stay. memchr's MSRV is 1.61. Signed-off-by: kurok <22548029+kurok@users.noreply.github.com>
kurok
added a commit
to namecheap/fast_mail_parser
that referenced
this pull request
Aug 27, 2026
* Search for MIME boundaries with memchr::memmem via a patched mailparse The codegen cliff #120 and #204 were circling has one cause, and it is a loop in a dependency. Sampling a metadata-mode parse of the 767 KiB fixture put 96.5% of the time in mailparse's find_from_u8, the byte-by-byte scan parse_mail runs for every MIME boundary. Its x86-64 instruction stream is byte-identical under rustc 1.97.1 and 1.98.0: 88 instructions, only label hashes differ. Yet two runners measured the metadata path at +96% under 1.98.0. The compiler had not made the loop slower; it had moved it. The crate hash changes with the rustc version -- and with the package version, which is #204 -- which changes symbol names, link order, and the loop's address. On the runners' Zen CPUs a scalar loop straddling the wrong 64-byte boundary falls out of the micro-op cache and runs at half speed. An Apple M4 measured the same two builds at +/-0.2%, which is why the effect never showed locally. The fix is a memchr::memmem search: vectorised, and laid out independently of this crate. It lives in vendor/mailparse -- upstream 0.16.1 with that one function changed and memchr added -- applied through [patch.crates-io] so the crate name and version stay what every other dependency expects. The same change is upstream as staktrace/mailparse#142; PATCH.md records the removal steps for when a release carries it. Interleaved A/B against master on the M4, controls within 1.2%: parse_email(mode="metadata") 0.365 ms -> 0.030 ms parse_email (full) 1.094 ms -> 0.743 ms parse_email_tree 1.124 ms -> 0.736 ms parse_many, 8 x 767 KiB 9.082 ms -> 6.189 ms Every mode faster, no output change: 803 tests pass and the vendored crate's own suite passes. memchr 2.8.3 is the only new crate in the lockfile. Two things the vendoring needed that a [patch] alone does not give. maturin follows [dependencies] path entries into the sdist but not [patch] ones, so without an explicit include the sdist would have built from the unpatched crates.io copy -- silently, and passing every test. The include is now explicit and the sdist job's install-from-source is what checks it. And the fuzz crate resolves mailparse on its own, so it carries the same patch, or it would be fuzzing a different parser than the one shipped. The lint job runs the vendored crate's tests, with a separate target dir: a target/ inside vendor/mailparse would be swept into the sdist by the include glob, which is also why .gitignore now names it. Signed-off-by: kurok <22548029+kurok@users.noreply.github.com> * Quote the gate's numbers in the README and changelog Every figure in these passages says "on the CI runner", so they change with the gate that measured this branch: an AMD EPYC 9V45, median of three interleaved rounds. The M4 numbers stay in the changelog as the local confirmation, with the reason they are smaller -- that machine never had the slow layout to lose. The hardware paragraph under the headline table used to cite the old ratios as its example of CPU dependence. It now cites them as the before, since they are no longer what any runner will measure. Signed-off-by: kurok <22548029+kurok@users.noreply.github.com> --------- Signed-off-by: kurok <22548029+kurok@users.noreply.github.com>
decode_base64 removed whitespace with iter().filter().cloned().collect(): a test and a bounds-checked push for every byte of the body. On a message with large base64 attachments that filter is where the parse spends its time -- sampling a full parse of a 767 KiB message put 78% of the cycles in it, more than in the base64 decode itself. Whitespace is now located with memchr and the runs between are copied whole. A base64 body is almost entirely 76-byte lines ending in CRLF, so the common case is one search and one memcpy per line. Tabs and form feeds are rare enough that they get a second search over each run rather than a place in the first. The set of bytes removed is unchanged -- exactly what u8::is_ascii_whitespace names -- and a test checks the new function against the filter it replaces over every byte value, alone and next to whitespace, including the vertical tab that is_ascii_whitespace does not include. Signed-off-by: kurok <22548029+kurok@users.noreply.github.com>
kurok
changed the title
Owner
|
Sorry, I'm not accepting PRs which add external dependencies, particularly ones that rely heavily on unsafe code. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two loops in the parse path test one byte at a time over the whole message body, and on a message with large attachments they are where
parse_mailspends its time. Both are replaced withmemchrsearches; both return exactly what they returned before.1.
find_from_u8— the searchparse_mailruns for every MIME boundary. Sampling a structure-only parse of a 767 KiB message (four base64 attachments) put 96% of the cycles here. Nowmemchr::memmem::find; same semantics (first occurrence ofkeyat or afterix_start,Nonewhen there is none), asserts kept.2.
decode_base64's whitespace strip —iter().filter(|c| !c.is_ascii_whitespace()).cloned().collect(), a test and a bounds-checked push per byte. With the first loop fixed, a full parse of the same message spent 78% of its time here, more than in the base64 decode itself. Now whitespace is found withmemchrand the runs between are copied whole — one search and one memcpy per 76-byte line in the common case. The set of bytes removed is unchanged (exactlyu8::is_ascii_whitespace); a new test checks the function against the original filter over every byte value, alone and next to whitespace, including the vertical tabis_ascii_whitespaceexcludes.Beyond speed, both loops' throughput depended on where the linker placed them: the same x86-64 instructions ran at half speed when a loop straddled a 64-byte boundary, so unrelated changes in a dependent crate — a version bump, a different rustc — moved parse time 2x with no change to this code.
memchr's routines are laid out independently of the caller.Measured from a dependent crate (fast_mail_parser) on the message above, interleaved A/B, Apple M4:
cargo testpasses on both commits;cargo fmt --checkclean. The first commit also removes one existing clippy warning (cused as a loop counter).memchr2.7+ has no dependencies of its own; its MSRV is 1.61.