Vortex Session

[gatesn]

VortexSession

We have had a VortexSession object for a while, but never really made use of it, in a large part because none of the Vortex components could take a dependency without creating a cycle.

Since Vortex is incredibly extensible, the idea of the session is to hold the plugin registries for arrays, layouts, expressions and so on. While our existing solution does this, it created very fragile APIs where it was possible to construct file readers/writers without using the main registry of plugins. Similarly, metrics registries were getting lost and some metrics ended up in isolated VortexMetrics::default() instances - never to see the light of day.

This PR introduces a top-level VortexSession create that essentially acts as a type map. Components of Vortex can register session state, and then read it back again later. This allows components to accept and hold a VortexSession internally, while our root vortex crate performs a default configuration.

Runtime Handles

One of the most fragile bits of mis-configured API was the Vortex runtime handle. This is our abstraction over runtimes (in practice, only over Tokio or a custom smol-based CurrentThreadRuntime) and provides an API for components to spawn CPU, I/O, blocking and other types of work.

This PR demonstrates the session API by holding a runtime handle and plumbing this through APIs that formerly accepted a with_handle argument.

Language Bindings

This PR doesn't not completely migrate language bindings onto a session-based API. Many bindings still use a global static session where a session instance would be preferred.

[a10y]

any reason to make pub?

if we really don't want people calling it we could slap a #[doc(hidden)] on it

[gatesn]

I haven't yet decided between:

• User creates an empty session, in which case we can just impl Default and the functions on the session are get_or_create.
• Downstream crates must explicitly initialize their state into the session. Possibly using inventory?? This seems weirder to me though. But it does allow for non-default state on the session, for example I don't think the async runtime should be defaulted on first access?

[codecov]

Codecov Report

❌ Patch coverage is 80.34321% with 126 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.72%. Comparing base (c4de94e) to head (ab4ac88).
⚠️ Report is 18 commits behind head on develop.

Files with missing lines	Patch %	Lines
vortex-ffi/src/file.rs	25.00%	27 Missing ⚠️
vortex-session/src/lib.rs	76.92%	18 Missing ⚠️
vortex-file/src/writer.rs	22.22%	14 Missing ⚠️
vortex-session/src/registry.rs	53.84%	12 Missing ⚠️
vortex-expr/src/session.rs	72.72%	9 Missing ⚠️
vortex-scan/src/gpu/gpubuilder.rs	0.00%	8 Missing ⚠️
vortex-layout/src/session.rs	73.91%	6 Missing ⚠️
vortex-io/src/runtime/single.rs	54.54%	5 Missing ⚠️
vortex-io/src/runtime/tokio.rs	37.50%	5 Missing ⚠️
vortex-file/src/file.rs	33.33%	4 Missing ⚠️
... and 8 more

☔ View full report in Codecov by Sentry.
📢 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.

[AdamGS]

another possible solution is to split the implementation and the trait - we implement some DefaultVortexSession in the top-level Vortex crate, and a VortexSession crate in vortex-session which can be a dependency of all the other crates, and they can take a &dyn VortexSession argument that gets populated or just passed around.

[gatesn]

@AdamGS the trait definition wouldn't be able to reference types from any of the other vortex crates. If you remove those types, you end up with general get/set on the session, which is basically the design we end up with here.

[cloudflare-workers-and-pages]

Deploying vortex-bench with    Cloudflare Pages

Latest commit:	1bb1aa3
Status:	✅  Deploy successful!
Preview URL:	https://80c9ae0b.vortex-93b.pages.dev
Branch Preview URL:	https://ngates-vortex-session.vortex-93b.pages.dev

View logs

[AdamGS]

@gatesn Depending on how its used, the trait can potentially only know about the traits, right? in which case it could be placed really close to the root of the dependency tree. It might require some restructuring there, which might not be desirable.
Either way - seems like you're already pretty deep into this, and I don't think I fully grok the implications here.

[codspeed-hq]

CodSpeed Performance Report

Merging #5111 will not alter performance

_{Comparing ngates/vortex-session (ab4ac88) with develop (69ef61d)¹}

Summary

✅ 1318 untouched
🆕 7 new
⏩ 127 skipped²

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
🆕	decompress[("alp_for_bp_f64", 0x764ac0)]	N/A	24.2 ms	N/A
🆕	decompress[("datetime_for_bp", 0x767da0)]	N/A	34.9 ms	N/A
🆕	decompress[("dict_fsst_varbin_bp_string", 0x766eb0)]	N/A	14.5 ms	N/A
🆕	decompress[("dict_fsst_varbin_string", 0x766820)]	N/A	14.5 ms	N/A
🆕	decompress[("dict_varbinview_string", 0x7654e0)]	N/A	14.7 ms	N/A
🆕	decompress[("for_bp_u64", 0x764370)]	N/A	2.5 ms	N/A
🆕	decompress[("runend_for_bp_u32", 0x765970)]	N/A	2 ms	N/A

Footnotes

1. No successful run was found on develop (c0598a6) during the generation of this report, so 69ef61d was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

2. 127 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. ↩

[lwwmanning]

it's weird that this is not a mut ref?