Conversation
…234) Mail::from_payload, lazy_from_payload and metadata_from_payload each carried their own copy of two blocks: the eleven-statement envelope extraction (header map, Subject, Date, From/To/Cc/Bcc/Reply-To) and the per-part classification (skip multipart/*, apply the RFC 2183 body-vs-attachment rule, derive filename, Content-ID and disposition token). The MIME-tree traversal carried a fourth copy of the identity derivation. The copies had already drifted in the way duplication drifts: header_addresses documented ten call sites when there were fifteen. And the drift that matters is not cosmetic -- the envelope is what strict=True and the whole warning channel are about, and the classification rule decides what an attachment IS. Three derivations of those were three chances for two views of one message to disagree, which is the failure parse_agreement's invariants 3 and 7 exist to catch. Now: envelope(), classify_part() and part_identity(). Straight-line and #[inline], so each caller emits the instructions it emitted when it owned a copy. What #100 measured at +47% was threading a runtime MODE through the parse -- a branch in the hot path taken for the benefit of the cold one. There is no mode and no branch here, which is the property that makes sharing free. Counted in the core: DispositionType::Attachment goes 3 -> 1 (the remaining one is disposition_token's own match), the Content-ID derivation 4 -> 1, and the multipart/* skip 3 -> 1 (the other is extract_mail_parts'). The tree traversal reads a part's identity from the same helper the flat modes do, so a part cannot answer to a different name depending on which API asked. One deviation from the issue's sketch, on purpose. It proposes a single PartInfo { filename, content_id, disposition, is_body }. Bundling the rule with the identity would derive a Content-ID and a disposition token for every text/plain body in every message -- work no mode does today. So the rule (classify_part) is separate from the identity (part_identity), and the identity is read only where it is used. Body parts consequently stop deriving a filename they never used, which full and lazy mode did do. Step 2 of the issue -- FlatMail<A> + a PartSink trait to share the loop itself -- is not here. The issue marks it optional and splittable once step 1's A/B is clean; it rewrites Mail::from_payload, the hottest function in the crate, and it deserves its own measurement rather than riding along with this one. Measured flat: local interleaved A/B on an M4, 3 rounds, worst +1.8% (parse_qp_message) against a 1.4% control noise floor, verdict "no significant difference". 864 Python tests, 13 core tests (two new, giving the RFC 2183 rule and the identity derivation direct tests instead of only three end-to-end ones), 179K fuzz executions with debug assertions live, clippy, fmt, mypy --strict, ruff and both repo invariant scripts.
Contributor
Author
|
Green on the first run: 11/11, benchmark gate The four checks that did not register are CodeQL's, for the same reason as on #259 — this repo uses GitHub's default setup for code scanning, which only runs on pull requests targeting the default branch. They will run once this retargets to master as the stack merges. |
This was referenced Sep 17, 2026
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
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.
Closes #234 (step 1; step 2 deliberately deferred — see below).
The duplication
Mail::from_payload,lazy_from_payloadandmetadata_from_payloadeach carried their own copy of the eleven-statement envelope extraction and of the per-part classification. The MIME-tree traversal carried a fourth copy of the identity derivation.The copies had drifted in the way duplication drifts —
header_addressesdocumented ten call sites when there were fifteen — and the drift that matters here is not cosmetic. The envelope is whatstrict=Trueand the whole warning channel are about; the classification rule decides what an attachment is. Three derivations of those are three chances for two views of one message to disagree, which is whatparse_agreement's invariants 3 and 7 exist to catch.The shape
Straight-line and
#[inline], so each caller emits the instructions it emitted when it owned a copy. What #100 measured at +47% was threading a runtime mode through the parse — a branch in the hot path taken for the benefit of the cold one. There is no mode and no branch here, which is the property that makes sharing free.Counted in the core:
DispositionType::Attachment(the RFC 2183 rule)get_first_value("Content-ID")derivationsstarts_with("multipart/")skips¹ the remaining one is
disposition_token's own match. ² the other isextract_mail_parts's, which is structural recursion, not classification.The
warn_datecheck stays at the two callers that want it: metadata mode deliberately does not make it, and a parameter to say so would be exactly the branch this avoids.One deviation from the issue's sketch
It proposes a single
PartInfo { filename, content_id, disposition, is_body }. Bundling the rule with the identity would derive aContent-IDand a disposition token for everytext/plainbody in every message — work no mode does today. So the rule is separate from the identity, and the identity is read only where it is used. A side effect: body parts stop deriving a filename they never used, which full and lazy mode both did.Step 2 is not here
The issue's step 2 —
FlatMail<A>plus aPartSinktrait, to share the loop body itself — is marked optional and splittable "if step 1's A/B is clean". It is clean, and step 2 rewritesMail::from_payload, the hottest function in the crate. It deserves its own measurement rather than riding along with this one. Happy to do it as a follow-up.Measurements
Local interleaved A/B on an M4 (
Apple M4, 10 vCPU), 3 rounds, against this PR's base:parse_messageparse_metadataparse_lazy_untouchedparse_lazy_all_attachmentsparse_manyparse_many_metadataparse_treeparse_tree_metadataparse_tree_lazy_untouchedNoise floor from the pure-Python controls 1.4%; worst treatment delta +1.8% (
parse_qp_message); verdict no significant difference. The issue predicted "every one within about ±1%, nothing expected to improve" — that is what happened.Verification: 864 Python tests; 13 core Rust tests, two of them new — the RFC 2183 rule and the identity derivation now have direct tests instead of only three end-to-end ones;
cargo clippy --workspace --all-targets -D warnings -W clippy::cast_possible_truncation;cargo fmt;mypy --strict;ruff;check_vendored_mailparse.sh;check_bench_lockfile.py. 179Kcargo fuzz run -a parse_agreementexecutions, zero crashes.__init__.pyi,tests/,docs/,vendor/andsrc/are untouched — the diff is the core and the changelog.