Skip to content

Measure x86 code-layout sensitivity with a salted layout A/B, and trial 64-byte block alignment for x86_64 wheels #240

Description

@kurok

Problem

The PR benchmark gate compares two builds of the extension on one x86 runner (.github/workflows/test.yml benchmark job, AB_THRESHOLD_PCT: "7"). Two builds that differ at all differ in code layout, and on the Zen CPUs GitHub's fleet provides, layout alone has been measured to move the decode-path benchmarks by +3.2-4.4% on the gate's own CPU (#205), +3-7% on an Apple M4 at the 0.9.0 cut (CHANGELOG.md:27-30), and +27% / +96% on one runner draw for a binary whose x86-64 instruction stream was byte-identical (#207, CONTRIBUTING.md:328-336). The two byte-at-a-time loops that produced the 96% class are fixed (#213, #214), but what remains of a full parse is 53% data_encoding::decode_base_mut, 27% the inlined whitespace strip and 7.5% memmem (M4 profile) -- loops whose addresses the fat-LTO link decides -- and CONTRIBUTING.md:359-360 records that "nobody has measured how sensitive they are".

Today the gate's only remedy for a suspicious verdict is prose: ab_median.py:193-200 tells the reader to re-run and compare the CPU line, i.e. hope the next runner draw is a different CPU model. A maintainer whose local machine is an M4 cannot reproduce the x86 effect at all, and QEMU is not a proxy (no micro-op-cache model). Every 7%+ verdict on a decode-path change -- and three round-1 issues (Decode base64 with base64-simd, run-copying QP decoder, Evaluate Body once per part) are exactly that -- is therefore "maybe".

The hardware is not the missing piece; the CI runners already are x86 Zen. What is missing is a controlled experiment: the same source built K times differing only in a layout salt, measured interleaved on one runner. The per-benchmark spread across salts is that revision's layout sensitivity on that CPU. The same harness, run once more with x86 code-alignment rustflags, decides whether the tail can be removed from the shipped x86_64 wheels (median unchanged, 0-96% tail gone) or whether that idea should be recorded as tried and rejected.

Evidence

All citations are from master 73f56da.

  • CONTRIBUTING.md:305-309 -- "This crate is unusually sensitive to codegen. A rustc minor version alone moved the parse path 15-96% (Investigate the ~26% slowdown under rustc 1.98.0, then unpin the toolchain #120) ... the lesson stands: do not assume a change is free because it looks free."
  • CONTRIBUTING.md:328-336 -- "Its x86-64 instruction stream was byte-identical under rustc 1.97.1 and 1.98.0 -- 88 instructions, only label hashes differed -- yet the runners measured +96% on the metadata path. ... The crate hash changes with the rustc version, and with the package version (Code layout can swing the hot path 0-96% depending on the runner's CPU #204), which changes symbol names, link order, and so 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%."
  • CONTRIBUTING.md:356-363 -- "The two dominant instances are gone, but the base64 decode proper (data_encoding) and charset conversion are also loops whose placement the linker decides, and nobody has measured how sensitive they are. So: re-run a large failure before acting on it. A real regression reproduces on different hardware; a layout-versus-CPU artifact does not."
  • .github/scripts/ab_median.py:189-200 -- the significant-verdict branch appends: "Re-run before acting on this. Two builds differing only in a version string measured within 0.4% of each other on one runner and 96% apart on another, with per-round spread under 0.3% both times and identical binaries (Code layout can swing the hot path 0-96% depending on the runner's CPU #204). A real regression reproduces on different hardware; a code-layout artifact does not. Compare the CPU line above between the two runs." That is the whole remedy.
  • CHANGELOG.md:27-30 -- "The version bump itself moved the decoding paths +3-7% against the last master commit on the M4 (two interleaved A/Bs, metadata paths within 0.7%) -- the code-layout effect Code layout can swing the hot path 0-96% depending on the runner's CPU #204 describes, now measured at release time rather than discovered later. The two loops that made it 0-96% are fixed; what remains is the base64 decode proper." A version-string change is a known, cheap layout perturbation, and it is visible even on the M4 at the few-percent level.
  • rust-toolchain.toml:19-25 -- with the loops replaced "the same A/B reads 1.98.0 within +/-0.5% of 1.97.1 on the CPU that showed the worst of it". One perturbation (toolchain), one CPU model: consistent with low residual sensitivity, but not a measurement of it.
  • Cargo.toml:25-30 -- [profile.release] opt-level = 3 / debug = false / strip = "debuginfo" / lto = true / codegen-units = 1. Layout is decided once, across the whole graph; nothing constrains block or function alignment.
  • .cargo/config.toml:1-10 -- only [target.x86_64-apple-darwin] and [target.aarch64-apple-darwin] with -C link-arg=-undefined -C link-arg=dynamic_lookup. There is no [target.x86_64-unknown-linux-gnu] section. grep -rn 'llvm-args\|align-all\|RUSTFLAGS' . over the tree matches only these two apple sections: no alignment flag exists anywhere, and no workflow sets RUSTFLAGS (so a [target.*] rustflags entry would actually take effect -- RUSTFLAGS in the environment would override it).
  • pyproject.toml:3 -- dynamic = ["version"]: the wheel version comes from Cargo.toml:6 version = "0.9.0". A +layoutN build-metadata salt on that line would therefore change the wheel's PEP 440 local version; harmless for a measurement, but the -C metadata salt below keeps the version identical and needs no file edit.
  • .github/workflows/toolchain-ab.yml:78-106 -- the pattern to copy: second side under its own CARGO_TARGET_DIR: ${{ runner.temp }}/target-candidate ("A separate target dir is load-bearing"), then unzip -p "$whl" '*.so' | sha256sum on both sides and exit 1 if equal ("the comparison would be meaningless"). toolchain-ab.yml:110-132 installs each wheel with pip install -q --no-cache-dir "${whl}[test]" into its own venv and measures pytest -q tests/benchmark --benchmark-min-rounds=25 --benchmark-json="$side-$round.json" round-robin.
  • .github/workflows/abi3-ab.yml:51-78 -- the sed-edit-then-verify pattern ("Never trust a sed silently") for the version-string fallback salt.
  • .github/workflows/test.yml:441-449, 460-471 -- the gate measures head/base for rounds 1 2 3 with --benchmark-min-rounds=25, then ab_median.py --a-label "this revision" --b-label "base" --informational test__threaded___ under AB_THRESHOLD_PCT: "7". Whatever layout sensitivity this revision has is inside every one of those verdicts.
  • vendor/mailparse/src/body.rs:139-142 -- fn decode_base64(body: &[u8]) ... let cleaned = crate::bytescan::strip_ascii_whitespace(body); Ok(data_encoding::BASE64_MIME_PERMISSIVE.decode(&cleaned)?) -- the remaining hot loops (53% + 27% of a full parse on the M4), placed by the LTO link.
  • Known numbers (do not re-measure): full parse 0.225 ms (M4) / 0.604 ms (EPYC 7763); metadata 0.030 ms; parse_many(8, threads=1) 1.84 ms; PR-gate within-job noise ~0.3%.

Proposal

Three deliverables; the third is conditional on what the first two measure. No new dependency. Nothing in src/ or vendor/ changes; parsing behaviour is untouched by construction.

1. .github/workflows/layout-ab.yml (workflow_dispatch, modelled on toolchain-ab.yml). Inputs: salts (default 4), rounds (default 3), rustflags (default empty; the alignment candidate under test), python-version (default 3.12). It builds the same checkout salts times as the plain group, side k with RUSTFLAGS="-C metadata=layout${k}" (k=0 is the unsalted build, i.e. exactly what the gate builds), each under CARGO_TARGET_DIR=$RUNNER_TEMP/target-plain-$k and into $RUNNER_TEMP/w-plain-$k/. If rustflags is non-empty it builds an aligned group the same way with RUSTFLAGS="${rustflags} -C metadata=layout${k}". Guard: the .so sha256 of every built side must be pairwise distinct (a salt that produced the same bytes means the perturbation did nothing and the run would report a reassuring 0%). Then one venv per side, and the measurement loop for round; for side in <all sides> -- interleaving across all sides, not per group -- writing <group>-<k>-<round>.json.

Why -C metadata=<salt> as the salt: rustc hashes every -C metadata value into the crate's StableCrateId, which is what a version bump changes too (CONTRIBUTING.md:331-333: "The crate hash changes with the rustc version, and with the package version (#204), which changes symbol names, link order, and so the loop's address"). Unlike the version sed it keeps the wheel version and filename identical, needs no git checkout Cargo.toml, and composes with the alignment flags in one variable. Fallback, if the hash guard ever reports two identical .sos for two salts: the proven version-string edit sed -i 's/^version = "\(.*\)"/version = "\1+layout'$k'"/' Cargo.toml with the abi3-ab.yml:63-73 style verification.

2. .github/scripts/layout_spread.py (sibling of ab_median.py; do not overload its two-sided CLI). Input: one or two groups, --group plain plain-0-*.json ... --group aligned aligned-0-*.json ... (or --group LABEL --sides k... --rounds N reading LABEL-k-round.json; either is fine, pick one and test it). It reuses ab_median.py's read_mins, read_machine, CONTROL_PREFIXES (import them; do not copy them). Per group and benchmark: m(b, k) = median over rounds of the benchmark's stats.min; spread(b) = (max_k m / min_k m - 1) * 100. Control floor F = max spread over control benchmarks. Output (stdout + GITHUB_STEP_SUMMARY): the CPU line, one table per group with the per-salt medians (ms), the spread, and a per-benchmark classification; with two groups a third table with cost(b) = (median_k m_aligned / median_k m_plain - 1) * 100 and the two .so sizes. Exit code 0 always except on input errors -- this is a dispatch-only measurement, the decision is a human's (same policy as ab_median.py:229-230).

Classification rule, pinned in tests: a treatment benchmark is layout-sensitive iff spread(b) > F and spread(b) > 3%. 3% is the floor because #205 measured the version-bump effect at 3.2-4.4% on the gate CPU; below that a 4-salt sweep cannot tell layout from the residual and should not pretend to. test__threaded___* benchmarks are informational, exactly as in the gate (test.yml:470), and excluded from the classification.

3. Decision on x86 alignment flags, made from the numbers. Candidates, each an LLVM cl::opt that rustc forwards with -C llvm-args= (an unknown name fails the build loudly, which is the desired failure mode -- these names are LLVM-internal and a toolchain bump can rename them):

  • -C llvm-args=-align-all-nofallthru-blocks=6 -- 64-byte-align every basic block that does not fall through from its predecessor, i.e. every loop header and branch target. The finding's primary candidate.
  • -C llvm-args=-align-loops=64 -- align loop headers only (narrower, less i-cache padding).
  • -C llvm-args=-align-all-functions=6 -- function starts only (cheapest; may be insufficient because the hot loops sit mid-function).

Decision rule (two dispatches per verdict, because #207 showed one runner draw can differ from the next; print and compare the CPU lines):

  • If the plain group's spread is <= max(F, 1%) on every treatment benchmark in both dispatches: this revision is not measurably layout-sensitive on these CPUs. Adopt no flag. Record the bound in CONTRIBUTING.md and soften ab_median.py:193-200 to quote it.
  • If the plain group is sensitive and, with a candidate flag, every treatment benchmark has spread_aligned <= max(F_aligned, 1%) and cost(b) <= +2% in both dispatches: adopt that flag for x86_64 only:
    # .cargo/config.toml -- x86_64 only; never the aarch64/apple sections.
    [target.x86_64-unknown-linux-gnu]
    rustflags = ["-C", "llvm-args=-align-all-nofallthru-blocks=6"]
    [target.x86_64-unknown-linux-musl]
    rustflags = ["-C", "llvm-args=-align-all-nofallthru-blocks=6"]
    [target.x86_64-pc-windows-msvc]
    rustflags = ["-C", "llvm-args=-align-all-nofallthru-blocks=6"]
    (The flag is a codegen option, linker-agnostic, so musl and MSVC get it on the linux-gnu evidence; state in the config comment that only linux-gnu was measured. i686 targets stay untouched.) Then a normal PR carries the config change through the ordinary gate, which measures base-without-flag vs head-with-flag on that runner.
  • If the plain group is sensitive and no candidate satisfies the rule: adopt nothing; record the per-candidate numbers in the issue and in CONTRIBUTING.md as a tried-and-rejected fix. That is still a result: the gate's prose can then quote a measured sensitivity instead of an anecdote.

Implementation notes for the agent

  1. Workflow. Create .github/workflows/layout-ab.yml by copying toolchain-ab.yml and replacing the two-toolchain build with the salted loop. Keep: permissions: contents: read, actions/checkout / actions/setup-python / actions/upload-artifact at the SHAs already pinned in toolchain-ab.yml:52-53,142-147, the --no-cache-dir wheel install, --benchmark-min-rounds=25. Do not add Swatinem/rust-cache. Build with python -m pip wheel . --no-deps -w "$RUNNER_TEMP/w-$group-$k" under RUSTFLAGS and CARGO_TARGET_DIR as above. Note RUSTFLAGS in the environment replaces .cargo/config.toml [target.*] rustflags; on ubuntu-latest there are none, so this is safe, but write that in a comment. Print each side's rustc --version, RUSTFLAGS, .so sha256 and .so byte size into GITHUB_STEP_SUMMARY. Fail if any two sha256s are equal. Upload *-[0-9]-[0-9].json.
  2. Runtime budget. 8 sides x 3 rounds of the full tests/benchmark suite is roughly 30-50 runner-minutes; acceptable for a dispatch. Add an optional pytest-args input (default empty) so a re-run can pass -k 'fast_mail_parser or mail_parser___parse_message'; the control benchmarks (ab_median.py:40-44 prefixes) must always be included or there is no floor -- assert at least one control benchmark is present in layout_spread.py and sys.exit with ::error:: otherwise.
  3. Script. .github/scripts/layout_spread.py importing read_mins, read_machine, is_control from ab_median (add sys.path.insert(0, os.path.dirname(__file__)) or make the import relative to the script dir; ab_median.py has no side effects at import because of its if __name__ == "__main__" guard at ab_median.py:240-241). It must pass ruff check . (project config in ruff.toml). mypy --strict runs only on fast_mail_parser/, so the script needs no annotations beyond ruff's rules.
  4. Tests. tests/test_layout_spread.py, mirroring tests/test_ab_median.py:1-60 (SCRIPT = ... / ".github" / "scripts" / "layout_spread.py", pytestmark = pytest.mark.skipif(not SCRIPT.exists(), ...), synthetic pytest-benchmark JSON via a _report_named helper, subprocess.run([sys.executable, SCRIPT, ...])). Cases: (a) four identical sides -> spread 0.0%, nothing classified sensitive; (b) one side +10% on test__fast_mail_parser___parse_message with controls flat -> classified sensitive; (c) same +10% but a control side at +12% -> not sensitive (floor rule); (d) +2.5% treatment spread with 0.1% floor -> not sensitive (3% rule); (e) two groups -> the cost table is emitted and the value is right to 0.1%; (f) the CPU line from machine_info.cpu.brand_raw is reported (as test_ab_median.py:213); (g) no control benchmark -> exits nonzero with ::error::.
  5. Dispatch and record. Run gh workflow run layout-ab.yml -f salts=4 -f rounds=3 twice (plain only), then twice per candidate with -f rustflags='-C llvm-args=-align-all-nofallthru-blocks=6' (and the other candidates only if the first fails the rule). Paste every step summary (CPU line, tables, .so sizes) into the PR body. If a candidate flag fails to compile ("Unknown command line argument"), record the rustc/LLVM version from the summary and move to the next candidate.
  6. Apply the decision. Either (a) edit .cargo/config.toml with the x86_64 sections above and a comment block (why, which workflow measured it, which CPUs, that only linux-gnu was measured, that RUSTFLAGS in the environment would silently drop it, and that a toolchain bump must re-dispatch layout-ab.yml because the option names are LLVM-internal); add one sentence to rust-toolchain.toml's "How to bump" comment pointing at that; or (b) make no build change. In both cases rewrite CONTRIBUTING.md:356-363 to replace "nobody has measured how sensitive they are" with the measured spread, the CPUs, the dispatch command, and the decision; update the constant prose in ab_median.py:193-200 to quote the measured bound for the current revision (its test tests/test_ab_median.py:199 test__a_significant_verdict_tells_the_reader_to_re_run asserts on that text -- adjust the assertion with it).
  7. Traps. .cargo/config.toml is not listed in pyproject.toml:46-49 [tool.maturin] include; maturin normally ships it in the sdist on its own, but verify with tar tf dist/*.tar.gz | grep '.cargo/config.toml' after python -m build --sdist (or maturin sdist) and add it to include if absent -- otherwise a source install would silently build without the flag. If the flag is adopted, the CI sdist install check (test.yml:503-515) exercises the source path. Never run cargo inside vendor/mailparse without --target-dir elsewhere (test.yml:60-64); this issue does not touch vendor/, so vendor/mailparse/PATCH.md needs no update. abi3: unaffected (alignment is codegen-only). __init__.pyi / tests/test_contract.py: untouched -- no API moves.
  8. CHANGELOG. One entry under ## [Unreleased] / ### Changed (or ### Added for the workflow) giving the measured spread per benchmark, the CPUs, and the flag decision with its cost(b) numbers.

Harness -- how to prove it

The x86 effect is not reproducible on an Apple M4 (CONTRIBUTING.md:334-336), so the measurement that decides this issue is the dispatched workflow on the CI runner; local work proves the harness mechanics and the lint/test surface.

Local (harness mechanics, in a venv with uv pip install maturin pytest pytest-benchmark mail-parser==4.6.4):

# two salted builds into two venvs, then a 2-round interleave -- checks the RUSTFLAGS salt,
# the differing .so hashes and the script end-to-end; the M4 spread itself is not the deliverable
for k in 0 1; do
  RUSTFLAGS="-C metadata=layout$k" CARGO_TARGET_DIR=/tmp/fmp-target-$k \
    uv pip wheel . --no-deps -w /tmp/w-plain-$k
  unzip -p /tmp/w-plain-$k/*.whl '*.so' | shasum -a 256
  python -m venv /tmp/venv-plain-$k && /tmp/venv-plain-$k/bin/pip install -q "$(ls /tmp/w-plain-$k/*.whl)[test]"
done
for r in 1 2; do for k in 0 1; do
  /tmp/venv-plain-$k/bin/pytest -q tests/benchmark --benchmark-min-rounds=25 --benchmark-json=plain-$k-$r.json
done; done
python .github/scripts/layout_spread.py --group plain plain-0-*.json plain-1-*.json

Expected on the M4 (from CHANGELOG.md:27-30): decode-path spread in the 3-7% range, test__fast_mail_parser___parse_metadata flat; the hashes must differ. Local M4 numbers go in the PR body labelled as M4 and are explicitly not the basis for the flag decision.

CI (the decision):

gh workflow run layout-ab.yml -f salts=4 -f rounds=3                # x2, plain sensitivity
gh workflow run layout-ab.yml -f salts=4 -f rounds=3 \
  -f rustflags='-C llvm-args=-align-all-nofallthru-blocks=6'         # x2 per candidate

Apply the decision rule in Proposal 3 to the step summaries. If a flag is adopted, the follow-up PR that edits .cargo/config.toml goes through the normal gate (test.yml benchmark job: base built without the flag, head with it, 3 interleaved rounds, AB_THRESHOLD_PCT=7): no treatment benchmark may regress > 7%, expected cost(b) within +/-2% on test__fast_mail_parser___parse_message, _parse_metadata, _parse_many, _parse_tree, _parse_lazy_*, _full_read; test__threaded___* informational. The M4 is not a proxy for the gate here -- this is the layout-sensitive case by definition, so the CI numbers are the ones that count, and two dispatches must agree.

Full local lint/test surface before the PR:

pytest tests --ignore=tests/benchmark          # includes the new tests/test_layout_spread.py and tests/test_ab_median.py
cargo fmt --all -- --check
cargo clippy --all-targets -- -D warnings -W clippy::cast_possible_truncation
mypy --strict fast_mail_parser/
ruff check .
cargo test --manifest-path vendor/mailparse/Cargo.toml --target-dir /tmp/vendor-target

Acceptance criteria

  • .github/workflows/layout-ab.yml exists, is workflow_dispatch-only, builds salts sides with RUSTFLAGS="-C metadata=layout<k>" (plus an aligned group when rustflags is given), each with its own CARGO_TARGET_DIR, and fails if any two .so sha256s are equal.
  • The measurement loop interleaves across all sides per round, and the step summary prints the CPU line, per-side RUSTFLAGS, .so sha256 and byte size.
  • .github/scripts/layout_spread.py reports per-benchmark per-salt medians, spread, control floor, the sensitive/not classification (> F and > 3%), and with two groups the cost(b) table; it imports the shared helpers from ab_median.py rather than copying them.
  • tests/test_layout_spread.py pins cases (a)-(g) from note 4 and passes; ruff check . is clean.
  • Two plain dispatches and two dispatches per tried candidate flag have run on x86 runners; their summaries (CPU, tables, sizes) are pasted in the PR body.
  • The decision rule was applied and its outcome is one of: (i) .cargo/config.toml gains x86_64-only rustflags with the explanatory comment, the sdist is verified to contain .cargo/config.toml, and the adopting PR passed the gate with every treatment benchmark within 7% (numbers in the PR body); or (ii) no build change, with the measured spreads recorded.
  • CONTRIBUTING.md Performance section no longer says "nobody has measured how sensitive they are"; it states the measured spread per benchmark, the CPUs, the dispatch command and the decision.
  • ab_median.py's significant-verdict prose quotes the measured bound for the current revision and tests/test_ab_median.py still passes.
  • CHANGELOG.md [Unreleased] has an entry with the numbers and the decision.
  • No change under src/, vendor/, fast_mail_parser/, tests/test_contract.py, or __init__.pyi; aarch64/apple build flags unchanged.

Out of scope

  • Wiring a stored per-benchmark sensitivity table into the PR gate's verdict automatically (ab_median.py --layout-sensitivity); follow-up once a measured table exists.
  • PGO, target-cpu, custom allocators, or any change to the release profile beyond alignment flags.
  • Alignment flags for i686 targets or for aarch64/apple.
  • Changing the vendored mailparse or data_encoding decode loop itself (round-1 issues Decode base64 with base64-simd and Evaluate Body once per part own that); this issue makes their verdicts interpretable.
  • Adding small/QP/RFC 2047 gated benchmarks (round-1 Benchmark gate only sees one 767 KiB base64 message); when they land, re-dispatch layout-ab.yml to cover them.

Related issues

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    ci/cdCI pipelines, build, releaseperformancePerformance / efficiencypriority: mediumMedium priorityrustPull requests that update rust codetestingTest coverage & quality

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions