Skip to content

Expose FFI runtime worker pool control#8789

Open
onursatici wants to merge 4 commits into
developfrom
os/ffi-workers
Open

Expose FFI runtime worker pool control#8789
onursatici wants to merge 4 commits into
developfrom
os/ffi-workers

Conversation

@onursatici

@onursatici onursatici commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

The FFI runtime is caller-driven by default: it owns no threads, and host threads drive the shared executor while inside FFI calls. Concurrent calls from multiple host threads can provide parallelism, but a single host thread cannot execute internally spawned work in parallel.

What changes are included in this PR?

Exposes process-global controls for configuring optional Vortex-owned worker threads for the FFI runtime. The default remains zero workers, preserving the existing host-thread-driven execution model. Configuring a non-zero count opts into a Vortex-owned background thread pool; setting it back to zero restores the default.

Signed-off-by: Onur Satici <onur@spiraldb.com>
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Polar Signals Profiling Results

Latest Run

Status Commit Job Attempt Link
🟢 Done 34ad733 1 Explore Profiling Data
Previous Runs (3)
Status Commit Job Attempt Link
🟢 Done 288f4a1 1 Explore Profiling Data
🟢 Done 49260f6 1 Explore Profiling Data
🟢 Done 81d8001 1 Explore Profiling Data

Powered by Polar Signals Cloud

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Benchmarks: Vortex queries

Verdict: No clear signal (low confidence)
Attributed Vortex impact: +0.7%
Engines: DataFusion No clear signal (+6.3%, low confidence) · DuckDB No clear signal (-4.5%, low confidence)
Vortex (geomean): 0.968x ➖
Parquet (geomean): 0.965x ➖
Shifts: Parquet (control) -3.5% · Median polish -3.5%

How to read Verdict and Engines
  • Verdict: Overall PR-level signal after subtracting baseline drift estimated from Parquet control rows. It can be Likely improvement, Likely regression, or No clear signal.
  • Engines: Per-engine attribution. DataFusion is compared against DataFusion/Parquet controls; DuckDB is compared against DuckDB/Parquet controls. This answers whether each engine improved or regressed independently.
  • Confidence: Based on directional consistency, share of rows above the noise floor, and control-run noise.

datafusion / vortex-file-compressed (1.004x ➖, 0↑ 0↓)
name PR 34ad733 (ns) base 01e5eb0 (ns) ratio (PR/base)
vortex_q00/datafusion:vortex-file-compressed 10142449 9772504 1.04
vortex_q01/datafusion:vortex-file-compressed 6290377 6477516 0.97
datafusion / parquet (0.945x ➖, 0↑ 0↓)
name PR 34ad733 (ns) base 01e5eb0 (ns) ratio (PR/base)
vortex_q00/datafusion:parquet 20158929 21257508 0.95
vortex_q01/datafusion:parquet 4634958 4923363 0.94
duckdb / vortex-file-compressed (0.940x ➖, 0↑ 0↓)
name PR 34ad733 (ns) base 01e5eb0 (ns) ratio (PR/base)
vortex_q00/duckdb:vortex-file-compressed 10163017 10663593 0.95
vortex_q01/duckdb:vortex-file-compressed 5957450 6422596 0.93
duckdb / parquet (0.985x ➖, 0↑ 0↓)
name PR 34ad733 (ns) base 01e5eb0 (ns) ratio (PR/base)
vortex_q00/duckdb:parquet 23348722 23624422 0.99
vortex_q01/duckdb:parquet 9365021 9545825 0.98

No file size changes detected.

@onursatici onursatici added the changelog/feature A new feature label Jul 16, 2026
Signed-off-by: Onur Satici <onur@spiraldb.com>
@0ax1
0ax1 requested a review from myrrc July 16, 2026 13:46

@myrrc myrrc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless that's absolutely critical I wouldn't like to land this even temporarily. We don't want own thread pool in the FFI, everything should run on host threads.

Same rationale applies to duckdb integration (we also use smol:: there).
Removing these functions would be a breaking change.
If we want proper parallel writes, let's land #8785 and #8773

@onursatici onursatici closed this Jul 16, 2026
@myrrc myrrc reopened this Jul 16, 2026
@0ax1

0ax1 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor
image

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.99%. Comparing base (01e5eb0) to head (49260f6).

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@myrrc

myrrc commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

We've chatted in private.

Current issue with FFI is that we don't have an explicit thread pool (which is good, library shouldn't create its own threads) but we need parallelism so we have an implicit pool that's on the users side.

The need to call i.e. push() from multiple threads to have parallelism (and yet it would run on host threads) is very non-trivial and makes it hard to reason about user code calling Vortex code.

As not using any of two work pool function is (allegedly) same as current behavior, it's not a breaking change.

And once I wrap up my work on "bring your own scheduler" and coroutine-like FFI API, we can remove this implicit pool stuff and leave user with two choices:

  1. We (vortex) manage the thread pool, and you (user) use the current nice API, or
  2. You manage your thread pool and do all IO yourself with coroutine-like API.

Signed-off-by: Onur Satici <onur@spiraldb.com>
@myrrc

myrrc commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

As a bonus, explicit thread pool would solve #7324

@myrrc myrrc added the lang/c Relates to the Vortex C (FFI) API label Jul 16, 2026
Signed-off-by: Onur Satici <onur@spiraldb.com>
@onursatici
onursatici enabled auto-merge (squash) July 16, 2026 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature lang/c Relates to the Vortex C (FFI) API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants