Rebuild the Python runtime after fork - #9376
Open
robert3005 wants to merge 6 commits into
Open
Conversation
Merging this PR will degrade performance by 0.3%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | cold_misaligned[(64, 256)] |
4.4 ms | 5 ms | -12.69% |
| ⚡ | WallTime | words_gather_scalar[65536] |
9.4 µs | 8.3 µs | +13.86% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing rk/python-fork-runtime (cf98f9c) with develop (f8d4db9)2
Footnotes
-
89 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
develop(a272f51) during the generation of this report, so f8d4db9 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
AdamGS
reviewed
Aug 12, 2026
onursatici
reviewed
Aug 12, 2026
onursatici
left a comment
onursatici
left a comment
Contributor
There was a problem hiding this comment.
I like the idea but I think it is still possible to be blocked.
In general for this to work we should:
- get the current_pid == state_pid check done without holding any locks, because that very lock could be stuck locked in fork
- when re-initialising the runtime state, we should make sure we are creating everything new, currently the internal Executor from the current thread runtime is copied and has an inner lock
Contributor
Author
|
I'm reworking it, the key problem is that blocking has a process global pool that's impossible to reset in any meaningful way |
robert3005
force-pushed
the
rk/python-fork-runtime
branch
from
August 12, 2026 15:12
c5ecfa2 to
16b4224
Compare
onursatici
reviewed
Aug 12, 2026
|
|
||
| // NOTE(ngates): we implement this for a Weak reference to adhere to the constraint that this | ||
| // trait should not hold strong references to the underlying runtime. | ||
| impl Executor for smol::Executor<'static> { |
Contributor
There was a problem hiding this comment.
is removing this a break? Should we add this back so users can still use the raw smol::Executor directly? our SmolExecutor is not public so if users were using this there is no clear way for them to migrate
Contributor
Author
There was a problem hiding this comment.
it is technically a break, we provide an alternative but you're right
onursatici
reviewed
Aug 12, 2026
| # Suppressing the Kanal frames does not make TSan propagate the handoff's happens-before edge. | ||
| # Catch the resulting reports at the pool's worker boundary instead of suppressing each operation | ||
| # that a blocking job may perform. | ||
| race:vortex_io*blocking_pool*worker_loop |
Contributor
There was a problem hiding this comment.
I know tsan doesn't like kanal and we keep hitting this, but is this too generic of an ignore? Should be fine but wish we had a channel impl that is good enough and tsan friendly
I wouldn't block on this though I think this is fine and a future thing to change
Contributor
Author
There was a problem hiding this comment.
I can try out other channel impls, basically otherwise any test now complains about tsan.
onursatici
approved these changes
Aug 12, 2026
onursatici
left a comment
onursatici
left a comment
Contributor
There was a problem hiding this comment.
Looks good, up to you on how you want to handle the public api change and we can merge
robert3005
force-pushed
the
rk/python-fork-runtime
branch
from
August 13, 2026 10:14
16b4224 to
680f766
Compare
`fork(2)` copies only the calling thread, so a forked child inherits a `CurrentThreadRuntime` whose worker threads no longer exist: the executor's sleeper list and the pool's handle list describe phantom threads, so `set_workers` believes it already has enough workers and spawns none, and every Vortex operation in the child blocks forever. The same applies to the process-global `blocking` pool behind `spawn_blocking`, whose inherited `idle_count` keeps it from growing. Tag the runtime with its owning pid and build a fresh one on first use from a different process, rather than trying to repair the inherited state; the stale state is leaked deliberately, since running its destructors would take locks that may not have survived the fork. An `os.register_at_fork(after_in_child=)` handler does the rebuild on the child's single-threaded startup path and warms the blocking pool. The shared session is repointed at the new executor, and the requested worker count is held outside the runtime so a child inherits it. `VortexFile` gains `path` and `__reduce__`, reopening by path in the receiving process, and the `datasets` filter path no longer needs its in-memory fallback now that a filter can be pickled into `num_proc` workers. Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
robert3005
force-pushed
the
rk/python-fork-runtime
branch
from
August 13, 2026 10:23
680f766 to
53d89ad
Compare
robert3005
added
changelog/break
github-actions
Bot
removed
the
action/bench-all
Contributor
Polar Signals Profiling ResultsLatest Run
Powered by Polar Signals Cloud |
Contributor
Benchmarks: String Encoding 📖vortex / vortex-file-compressed / ms (0.992x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / % (1.000x ➖, 0↑ 0↓)
|
Contributor
Benchmarks: TPC-H SF=1 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.998x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.000x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.003x ➖, 1↑ 1↓)
duckdb / vortex-file-compressed / ns (0.990x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.027x ➖, 0↑ 2↓)
duckdb / parquet / ns (1.001x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: Random Access 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
random-access / vortex-file-compressed / ns (0.993x ➖, 0↑ 0↓)
random-access / parquet / ns (0.999x ➖, 0↑ 0↓)
random-access / lance / ns (0.992x ➖, 0↑ 0↓)
|
Contributor
Benchmarks: FineWeb NVMe 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.005x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.011x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.006x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.062x ➖, 2↑ 3↓)
duckdb / vortex-compact / ns (1.025x ➖, 1↑ 3↓)
duckdb / parquet / ns (0.992x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-DS SF=1 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.000x ➖, 1↑ 0↓)
datafusion / vortex-compact / ns (1.001x ➖, 1↑ 3↓)
datafusion / parquet / ns (1.001x ➖, 2↑ 1↓)
duckdb / vortex-file-compressed / ns (0.986x ➖, 5↑ 6↓)
duckdb / vortex-compact / ns (0.996x ➖, 1↑ 4↓)
duckdb / parquet / ns (0.998x ➖, 3↑ 4↓)
No file size changes detected. |
Contributor
Benchmarks: PolarSignals Profiling 📖Vortex (geomean): 1.008x ➖ datafusion / vortex-file-compressed / ns (1.008x ➖, 1↑ 1↓)
No file size changes detected. |
Contributor
Benchmarks: Appian on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-compact / ns (0.993x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.993x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (0.990x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.995x ➖, 0↑ 0↓)
File Size Changes (10 files changed, -63.8% overall, 0↑ 10↓)
Totals:
|
Contributor
Benchmarks: FineWeb S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.839x ➖, 2↑ 0↓)
datafusion / vortex-compact / ns (0.933x ➖, 1↑ 0↓)
datafusion / parquet / ns (1.003x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.004x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.769x ➖, 1↑ 0↓)
|
Contributor
Benchmarks: TPC-H SF=1 on S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.019x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.953x ➖, 1↑ 0↓)
datafusion / parquet / ns (1.022x ➖, 1↑ 2↓)
duckdb / vortex-file-compressed / ns (1.023x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.064x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.027x ➖, 0↑ 0↓)
|
Contributor
Benchmarks: Clickbench Sorted on NVME 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.117x ❌, 0↑ 2↓)
datafusion / vortex-compact / ns (0.990x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.003x ➖, 1↑ 1↓)
duckdb / vortex-file-compressed / ns (1.212x ❌, 0↑ 10↓)
duckdb / vortex-compact / ns (1.180x ❌, 0↑ 8↓)
duckdb / parquet / ns (1.017x ➖, 0↑ 0↓)
File Size Changes (200 files changed, +0.0% overall, 114↑ 86↓)
Totals:
|
Contributor
Benchmarks: Clickbench on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.019x ➖, 1↑ 2↓)
datafusion / vortex-compact / ns (0.999x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.988x ➖, 1↑ 0↓)
duckdb / vortex-file-compressed / ns (1.084x ➖, 1↑ 16↓)
duckdb / vortex-compact / ns (1.060x ➖, 0↑ 11↓)
duckdb / parquet / ns (1.020x ➖, 0↑ 2↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=10 on NVME 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.992x ➖, 1↑ 0↓)
datafusion / vortex-compact / ns (1.000x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.983x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.024x ➖, 0↑ 1↓)
duckdb / vortex-compact / ns (1.064x ➖, 0↑ 5↓)
duckdb / parquet / ns (1.008x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-H SF=10 on S3 📖Verdict: No clear signal (environment too noisy confidence) How to read Verdict and Engines
datafusion / vortex-compact / ns (0.867x ➖, 5↑ 0↓)
datafusion / parquet / ns (0.924x ➖, 1↑ 0↓)
duckdb / vortex-compact / ns (1.027x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.088x ➖, 0↑ 1↓)
|
Contributor
Benchmarks: Statistical and Population Genetics 📖Verdict: No clear signal (low confidence) How to read Verdict and Engines
duckdb / vortex-file-compressed / ns (1.025x ➖, 1↑ 2↓)
duckdb / vortex-compact / ns (0.999x ➖, 2↑ 4↓)
duckdb / parquet / ns (1.000x ➖, 0↑ 0↓)
No file size changes detected. |
AdamGS
reviewed
Aug 13, 2026
| TIMEOUT_SECONDS = 120 | ||
| ROWS = 10_000 | ||
|
|
||
| PREAMBLE = textwrap.dedent( |
| import vortex.expr as ve | ||
|
|
||
| PATH = sys.argv[1] | ||
| CTX = multiprocessing.get_context("fork") |
Contributor
There was a problem hiding this comment.
we should also test spawn and forkserver, they have some different behaviors around this sort of stuff.
AdamGS
approved these changes
Aug 13, 2026
Contributor
Benchmarks: Compression 📖vortex / vortex-file-compressed / ns (0.983x ➖, 2↑ 0↓)
vortex / vortex-file-compressed / bytes (1.000x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / ratio (0.989x ➖, 2↑ 0↓)
vortex / parquet / ns (0.998x ➖, 0↑ 0↓)
vortex / parquet / bytes (1.000x ➖, 0↑ 0↓)
|
Signed-off-by: Robert Kruszewski <github@robertk.io>
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
3 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.
Make python runtime pool fork friendly. While it's preferable to use threads
instead of processes we should gracefully recover from being forked instead of
deadlocking
Stack created with GitHub Stacks CLI • Give Feedback 💬