Size parse_many's workers by bytes, and let the caller be one (#232) - #247
Conversation
f98491d to
52ed36b
Compare
|
Gate status, and why I am not patching it here. This PR was green at 15/15 before I rebased it onto This PR is the clearest evidence in the series that the failure is not the PR. It touches
Rock-stable across rounds, so not noise; and the only work separating the flat row from the failing one is the decode. #246 failed the same way on the same rebase (+7.3%), and #248 took three Leaving it failing and explained rather than passing by guesswork. Ready to rebase once there is a measured spread to judge against. |
52ed36b to
75e4ac1
Compare
|
Rebased onto Same source as the previous run (a
The benchmark that fails moved. The one that failed at +19% is now 6.9% faster. The same unchanged scheduler change now scatters ±15% in both directions against a 0.5% control floor. And #246 — rebased in the same batch, also no code change — went from failing at +7.3% to passing 15/15. Two independent flips in one batch. This is not a property of either PR; it is the spread of the instrument. On this revision, on these runners, code placement alone moves the parse benchmarks by at least ±15%, which is more than twice the gate's 7% threshold. A 7% verdict on these paths is currently a coin toss with extra steps. That is exactly the number #249 exists to measure properly — four salted builds of one source, interleaved, reporting the per-benchmark spread — instead of us inferring it from failures. The inference is now strong enough to act on: this PR should be judged against the measured spread, not against 7%, and I am not going to touch its code to make a placement lottery land the right way. Its own benchmark, meanwhile, is doing exactly what it was written to do: |
parse_many capped its workers on message count alone, never on how much
work there was. Sixteen one-kilobyte messages -- an IMAP fetch page, a queue
poll -- got min(cores, 16) threads, each of which parsed about one message
and exited. Creating and joining an OS thread is 15-40 us against a parse of
roughly 1.1 us per KB, so for that shape the scheduler cost one to two
orders of magnitude more than the work it scheduled.
Measured, on an Apple M4 before this change: a 16-message page with the
default thread count took 0.070 ms, and the same page with threads=1 took
0.029 ms. parse_many was 2.2x slower than asking it not to use threads --
for the shape a mail pipeline produces most often, and against a README that
says it "costs nothing at any size".
Three std-only changes, no new dependency, the dynamic cursor kept:
(a) Workers are capped at one per MIN_BYTES_PER_WORKER (64 KiB) of input as
well as one per message, so a batch with less work than one worker's
worth takes the existing inline path. threads= stays an upper bound:
the gate may lower it and never raises it.
(b) The claim loop is a closure both the spawned workers and the calling
thread run. The caller already sits inside py.detach with the GIL
released, so blocking it on joins was one spawn and one join per call
for nothing -- half the thread cost at workers == 2. A panic on the
caller unwinds out of thread::scope, which joins the spawned workers
first, and reaches catch_panics exactly as a resume_unwind did.
(c) available_parallelism is probed once per process, in the binding layer.
std does not memoise it; on Linux it reads /proc/self/cgroup,
/proc/self/mountinfo and the cgroup cpu.max files on every call. The
cache cannot live in mail_parser -- that module's docs rule out statics
and OnceLock so its free-threading audit stays valid -- and the core
keeps its own fallback for the fuzz targets that include it by path.
The trade-off is stated at the function: a cgroup quota changed after
the first call is not observed; pass threads= to override.
Results, input ordering, per-slot Ok/Err, raise_on_error, strict, mode= and
the GIL being released for the whole batch are all unchanged. Worker count
is not observable from Python, so the new tests pin the only thing that is:
default threads and threads=1 agree, subject for subject, in every mode --
for a batch under the gate and for one above it, so the genuinely parallel
path stays covered rather than being quietly serialised by the gate.
Apple M4 (10 vCPU), 5 interleaved rounds, pure-Python controls within 0.5%:
parse_many_small_page 0.070 -> 0.032 ms 2.2x
parse_many_small_page_threads1 0.029 -> 0.028 ms flat (control)
parse_many (16 x 767 KiB) 1.326 -> 1.333 ms flat
The control is the point: same batch, same parsing, scheduler forced off. It
does not move, so the saving is scheduling.
Signed-off-by: kurok <22548029+kurok@users.noreply.github.com>
75e4ac1 to
d9ef69f
Compare
1 participant
Closes #232.
Branched from
master(e7bbb05). Independent of #245 and #246.The bug, measured before the fix
parse_manycapped its workers on message count alone, never on how much work there was. Sixteen one-kilobyte messages — an IMAP fetch page, a queue poll — gotmin(cores, 16)threads, each of which parsed roughly one message and exited. Creating and joining an OS thread is 15–40 µs against a parse of ~1.1 µs/KB, so the scheduler cost one to two orders of magnitude more than the work it scheduled.On this machine, on master:
parse_many(page)— default threadsparse_many(page, threads=1)parse_manywas 2.2x slower than asking it not to use threads — for the shape a mail pipeline produces most often, against a README that says it "costs nothing at any size". No benchmark covered that shape, which is why it went unnoticed.What changed
Three std-only changes. No new dependency, the dynamic cursor kept.
(a) Workers are capped by bytes as well as by count.
MIN_BYTES_PER_WORKER = 64 KiB; a batch with less total work than one worker's worth takes the existing inline path.threads=stays an upper bound — the gate may lower it, never raise it.(b) The calling thread is a worker. The claim loop is now a closure that both the spawned workers and the caller run;
workers - 1get spawned. The caller already sits insidepy.detachwith the GIL released, so blocking it on joins was one spawn and one join per call for nothing — half the thread cost whenworkers == 2. A panic on the caller unwinds out ofthread::scope, which joins the spawned workers first, and reachescatch_panicsexactly as aresume_unwindfrom a joined handle did.(c) The default parallelism is probed once per process, in the binding layer.
stddoes not memoiseavailable_parallelism; on Linux it reads/proc/self/cgroup,/proc/self/mountinfoand the cgroupcpu.maxfiles on every call. The cache cannot live inmail_parser— that module's docs rule out statics andOnceLockso its free-threading audit stays valid — so it lives next to theOnceLocks this layer already has, and the core keeps its own fallback for the fuzz targets that include it by path. The trade-off is stated at the function: a cgroup quota changed after the first call is not observed; passthreads=to override.Numbers
Apple M4 (10 vCPU), rustc 1.98.0, CPython 3.12, 5 interleaved rounds. Pure-Python controls within 0.5%.
parse_many_small_page(new)parse_many_small_page_threads1(new control)parse_many(16 × 767 KiB)The control is the point. Same batch, same parsing, scheduler forced off — it does not move. That is what says the 2.2x is scheduling and not parsing, and it is why the benchmark was added in a pair.
The 16 × 767 KiB batch is unaffected, as intended: 12 MB is far above the gate.
What I am not claiming from this table.
parse_many_smallandparse_many_metadata_smallread ~27% faster,threaded___parse_many~8%, and every metadata/lazy-untouched benchmark +16–24%. Those last ones are the M4 code-placement artifact documented on #244 — master sits at 0.037 ms onparse_metadatawhere it was 0.030 before base64-simd was linked, and x86 measured them flat on #244, #245 and #246. The 2000-message rows are above the byte gate and spawn the same worker count either way, so I do not have a mechanism that accounts for 27%; the plausible one (one fewer spawn out of ten) is worth well under 1%. Treat those rows as unexplained until the gate reports. The claim this PR makes is the first row and its control.Full A/B report
Measured on
Apple M4, 10 vCPU.Median of 5 interleaved rounds per side; each value is a benchmark's minimum. Positive delta =
masteris slower.test__fast_mail_parser___attachment_rereadtest__fast_mail_parser___full_readtest__fast_mail_parser___parse_lazy_all_attachmentstest__fast_mail_parser___parse_lazy_untouchedtest__fast_mail_parser___parse_manytest__fast_mail_parser___parse_many_metadatatest__fast_mail_parser___parse_messagetest__fast_mail_parser___parse_message_stricttest__fast_mail_parser___parse_metadatatest__fast_mail_parser___parse_metadata_strtest__fast_mail_parser___parse_treetest__fast_mail_parser___parse_tree_lazy_untouchedtest__fast_mail_parser___parse_tree_metadatatest__mail_parser___parse_messagetest__mailparser_lib___full_readtest__stdlib_email___full_readtest__threaded___parse_manytest__threaded___parse_many_metadata_smalltest__threaded___parse_many_smalltest__threaded___parse_many_small_pagetest__threaded___parse_many_small_page_threads1test__threaded___threadpool_parse_emailtest__threaded___threadpool_parse_email_smallNoise floor from the pure-Python controls: 0.5% (they cannot be affected by the build, so this is measurement error).
Contract
Unchanged: one result per input in input order, per-slot
Ok/Err,raise_on_error,strict,mode=,threads=0→ValueError, negative →OverflowError, the GIL released for the whole batch, a parser panic surfacing asParseError.Worker count is not observable from Python, so the new tests in
tests/test_parse_many.pypin the only thing that is — that results do not depend on how the scheduler sized itself:test__a_small_batch_with_default_threads_matches_threads_one— 16 messages, asserted under the gate, all three modes.test__a_large_batch_with_default_threads_still_matches_threads_one— 400 × 20 messages, asserted above the gate, so the genuinely parallel path stays covered rather than being quietly serialised by the change under test.test__threads_is_an_upper_bound_not_a_target—threads=64on a tiny batch still returns correct results.test__a_single_message_batch_still_parsesandtest__empty_and_tiny_batches_do_not_divide_by_zero—workersbottoms out through two.min()s and a.max(1), andtotal_bytes / MIN_BYTES_PER_WORKERis integer division, so these are where an off-by-one or a zero would show.Docs
Readme.mdsaidparse_many"costs nothing at any size". That was false for small batches, so the section now says workers are sized by bytes, that a sub-64-KiB batch parses inline, and what it used to cost. Theparse_manydocstring in__init__.pyisaysthreadsis an upper bound and that small batches use fewer.Checks run locally
pytest tests --ignore=tests/benchmark(820 passed, 3 skipped),cargo fmt --all --check,cargo clippy --all-targets -- -D warnings -W clippy::cast_possible_truncation,mypy --strict,ruff check .— all green. Novendor/change, so noPATCH.mdand no vendoredcargo test.src/mail_parser.rsstays PyO3-free (it is compiled by path into two fuzz targets) and gains no shared state — the one cache added is in the binding layer.