Release the GIL while parsing expressions of 32 bytes or more - #59
Merged
Merged
Conversation
compile(), and the parse inside evaluate(), now run the CEL parser detached
from the interpreter. Parsing is pure Rust (nothing in Program::compile can
call back into Python) and it is the expensive half of evaluate(): about 8 us
for a one-token expression, 70 us for a policy-sized one and milliseconds for
large literals, against a detach/attach round trip of well under 100 ns.
Threads that parse concurrently now scale with cores.
Measured on 4 cores (min of repeats, fixed total work):
evaluate(policy) 1 thread 12.5k/s 4 threads: 0.96x before, 3.02x after
compile(policy) 1 thread 13.2k/s 4 threads: 0.96x before, 3.19x after
single-thread latency unchanged within noise (69.6 vs 69.6 us for the policy)
Expressions shorter than 32 bytes keep the GIL. Their parse (8-13 us) sits at
the point where the cost of re-acquiring a contended GIL, about 14 us with four
threads waiting on this machine, cancels the released work: releasing it for
evaluate("x + y") gained 1.6x on two threads but lost 25% on four. Length is a
faithful proxy here because parse time grows with the input, so the bound can
only cost a missed speedup, never a regression; it also leaves margin for
machines whose re-acquire is slower than the one this was tuned on.
Execution still holds the GIL; the work-aware gate for that is tracked in #45.
catch_unwind sits inside the detached region so a parser panic is caught before
control crosses PyO3's re-attach guard.
Tests detect the release by overlap rather than speed: a spinning thread counts
iterations during a ~400 ms parse, which stays near zero with the GIL held and
climbs into the millions with it released, independent of core count.
Claude-Session: https://claude.ai/code/session_019WbvXZFm8Nb2LXF2kiWoWW
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The doc comments on PARSE_DETACH_MIN_LEN and compile_program, and the CHANGELOG entry, narrated the measurements behind the 32-byte bound. A reader needs the constraint (a short parse costs about as much as re-acquiring a contended GIL) and where the numbers live (issue #45), not the numbers themselves. Claude-Session: https://claude.ai/code/session_019WbvXZFm8Nb2LXF2kiWoWW
…75fkzx-parse-detach
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.
Why
First half of PR 2 in the #45 plan: the part of GIL release that is unambiguously a win. Parsing is pure Rust (nothing in
Program::compilecan call back into Python) and it is the expensive half ofevaluate(): about 8 µs for a one-token expression, 70 µs for a policy-sized one, milliseconds for large literals, against a detach/attach round trip of well under 100 ns. Today every thread parsing an expression serialises on the interpreter.What
compile(), and the parse step insideevaluate(), runProgram::compileinsidepy.detach(...)when the expression is 32 bytes or more.catch_unwindsits inside the detached region so a parser panic is caught before control crosses PyO3's re-attach guard.evaluate("x + y")gained 1.6× on two threads but lost 25% on four. Length is a faithful proxy here because parse time grows with the input, so the gate can only cost a missed speedup for short expressions, never a regression, and the constant has margin for machines whose GIL handoff is slower than this one. (This is a different situation fromexecute(), where neither callback presence nor context size predicts the work, which is why execution stays attached for now.)Numbers
4 cores, release build, min of repeats, fixed total work per thread count.
evaluate(policy)compile(policy)evaluate("x + y")(below the gate)Single-thread latency is unchanged within noise:
compile(policy)69.6 → 69.6 µs,evaluate(policy)73.6 → 73.2 µs,compile("x + y")12.3 → 12.3 µs.Tests
tests/test_gil_release.pydetects the release by overlap, not speed: a spinning thread counts iterations while the main thread parses a 20,000-element list literal (~250 ms). With the GIL held the count stays near zero; released, it climbs into the millions. That is a binary signal, so it does not depend on the runner's core count or load. Skipped on free-threaded builds, where the spinner runs regardless. Also: parse results and errors identical on both sides of the length gate; 400 concurrent compiles across 8 threads each produce the right program. Full suite: 564 passed, 1 skipped, 5 xfailed; clippy, fmt, ruff clean.Independent of #58 (touches different regions of
src/lib.rs); the CHANGELOG Unreleased section will need a one-line merge with whichever lands second.https://claude.ai/code/session_019WbvXZFm8Nb2LXF2kiWoWW
Generated by Claude Code