Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
Windows x64. Importing `cel` leaves the GIL disabled
([#45](https://github.com/hardbyte/python-common-expression-language/issues/45)).

### Performance

- **Parsing no longer holds the GIL.** `compile()`, and the parse inside
`evaluate()`, release the GIL for expressions of 32 bytes or more, so threads
that parse concurrently run in parallel: about 3× the throughput from 4 threads
on a 4-core machine, with single-threaded cost unchanged. Executing a compiled
program still holds the GIL
([#45](https://github.com/hardbyte/python-common-expression-language/issues/45)).

### Changed

- `Program` and `OptionalValue` are immutable; attributes can no longer be
Expand Down
6 changes: 6 additions & 0 deletions docs/reference/python-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ assert result2 == 30 # → 30
- Use `evaluate()` for one-time evaluation or interactive/REPL usage
- Use `compile()` + `execute()` when evaluating the same expression with many different contexts or in performance-critical loops

Parsing (`compile()`, and the parse inside `evaluate()`) runs with the GIL
released for expressions of 32 bytes or more, so threads that parse concurrently
run in parallel. Shorter expressions parse in a few microseconds, which is less
than the cost of re-acquiring a contended GIL, so they keep it. Executing a
compiled program also holds the GIL: evaluations are usually sub-microsecond.

## Classes

### Program
Expand Down
35 changes: 29 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,18 +267,37 @@ impl PyOptionalValue {
/// >>> program.execute({"x": 10, "y": 20})
/// 30
#[pyfunction]
fn compile(expression: String) -> PyResult<PyProgram> {
let program = compile_program(&expression)?;
fn compile(py: Python<'_>, expression: String) -> PyResult<PyProgram> {
let program = compile_program(py, &expression)?;
Ok(PyProgram {
program,
source: expression,
})
}

/// Expressions shorter than this (in bytes) are parsed with the GIL held.
///
/// A short parse costs about as much as re-acquiring a contended GIL, so
/// releasing it for one is a net loss under contention. Parse time grows with
/// expression length, which makes length a safe gate: erring long only forgoes a
/// speedup. Benchmarks behind the bound are in issue #45.
const PARSE_DETACH_MIN_LEN: usize = 32;

/// Parses `expression`, turning both parse errors and parser panics into
/// `ValueError` so callers can rely on one exception type for a bad expression.
fn compile_program(expression: &str) -> PyResult<Program> {
panic::catch_unwind(|| Program::compile(expression))
///
/// The parse is pure Rust and cannot call back into Python, so it runs with the
/// GIL released unless the expression is too short for that to pay off.
/// `catch_unwind` sits inside the detached region so a parser panic is caught
/// before control crosses back through PyO3's re-attach guard.
fn compile_program(py: Python<'_>, expression: &str) -> PyResult<Program> {
let parse = || panic::catch_unwind(|| Program::compile(expression));
let parsed = if expression.len() >= PARSE_DETACH_MIN_LEN {
py.detach(parse)
} else {
parse()
};
parsed
.map_err(|_| {
warn!("CEL parser panic for expression: '{}'", expression);
PyValueError::new_err(format!(
Expand Down Expand Up @@ -1058,11 +1077,15 @@ impl TryIntoValue for RustyPyType<'_> {
/// - CEL Language Guide: For comprehensive language documentation
/// - Python API Reference: For detailed API documentation
#[pyfunction(signature = (src, evaluation_context=None))]
fn evaluate(src: String, evaluation_context: Option<&Bound<'_, PyAny>>) -> PyResult<RustyCelType> {
fn evaluate(
py: Python<'_>,
src: String,
evaluation_context: Option<&Bound<'_, PyAny>>,
) -> PyResult<RustyCelType> {
// Validate the context before parsing so a bad context and a bad expression
// report in the same order they always have.
let environment = prepare_environment(evaluation_context)?;
let program = compile_program(&src)?;
let program = compile_program(py, &src)?;
run_program(&program, &src, &environment).map(RustyCelType)
}

Expand Down
104 changes: 104 additions & 0 deletions tests/test_gil_release.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
"""Parsing releases the GIL.

``cel.compile()`` and the parse inside ``cel.evaluate()`` run with the GIL
released for expressions of 32 bytes or more, so other Python threads make
progress while an expression is parsed. Shorter expressions parse in a few
microseconds, comparable to the cost of re-acquiring a contended GIL, so they keep
it. Execution of a compiled program still holds the GIL (see issue #45 for why: a
sub-microsecond evaluation loses badly to the cost of re-acquiring a contended
GIL, and a work-aware gate for execution is a separate change).

The tests below detect release by *overlap* rather than by speed: a spinning
thread counts iterations while the main thread parses. With the GIL held for the
whole parse the spinner cannot run and the count stays at essentially zero; with
it released the spinner gets a core and the count climbs into the millions. That
is a binary signal, so it does not depend on the runner's core count or load.
"""

import sys
import sysconfig
import threading

import cel
import pytest

# A list literal parses in time linear in its length; 20,000 elements is a few
# hundred milliseconds of pure parsing, long enough for the spinner to show up.
BIG_EXPRESSION = "[" + ", ".join(str(i) for i in range(20_000)) + "]"

# On a free-threaded build the spinner runs regardless, so overlap proves nothing;
# the contract there is simply "still works", covered by the whole suite.
requires_gil_build = pytest.mark.skipif(
sys.implementation.name != "cpython" or bool(sysconfig.get_config_var("Py_GIL_DISABLED")),
reason="overlap is only observable on a CPython build with a GIL",
)


def ticks_during(action):
"""Run ``action`` on this thread while another thread spins; return the spinner's count."""
ticks = 0
started = threading.Event()
stop = threading.Event()

def spin():
nonlocal ticks
started.set()
while not stop.is_set():
ticks += 1

spinner = threading.Thread(target=spin)
spinner.start()
started.wait()
# Let the spinner settle so a switch-interval handoff right at the start does
# not count as overlap.
threading.Event().wait(0.02)
before = ticks
action()
after = ticks
stop.set()
spinner.join()
return after - before


@requires_gil_build
def test_compile_releases_the_gil():
assert ticks_during(lambda: cel.compile(BIG_EXPRESSION)) > 10_000


@requires_gil_build
def test_evaluate_releases_the_gil_while_parsing():
assert ticks_during(lambda: cel.evaluate(BIG_EXPRESSION)) > 10_000


def test_parse_results_are_unchanged():
"""Detaching changes where the parse runs, not what it produces or raises.

Covers both sides of the length threshold: short expressions parse attached,
long ones detached, and errors surface identically from either path.
"""
assert cel.compile("1 + 2").execute() == 3
assert cel.evaluate(BIG_EXPRESSION)[-1] == 19_999
long_policy = 'user.role == "admin" || (resource.owner == user.id && size(user.groups) > 0)'
assert len(long_policy) >= 32
assert cel.evaluate(long_policy, {"user": {"role": "admin"}, "resource": {}}) is True
with pytest.raises(ValueError, match="Failed to parse"):
cel.compile("1 +")
with pytest.raises(ValueError, match="Failed to parse"):
cel.evaluate("'unterminated")
with pytest.raises(ValueError, match="Failed to parse"):
cel.compile(
"this is a long enough expression to be parsed detached but it is not valid CEL !!"
)


def test_concurrent_compiles_are_independent():
"""Many threads parsing at once each get their own correct program."""
from concurrent.futures import ThreadPoolExecutor

def work(i):
return cel.compile(f"{i} * 2 + 1").execute()

with ThreadPoolExecutor(max_workers=8) as pool:
results = list(pool.map(work, range(400)))

assert results == [i * 2 + 1 for i in range(400)]