Skip to content

Conversation

@onursatici
Copy link
Contributor

@onursatici onursatici commented Oct 20, 2025

Implements __reduce__ and __reduce_ex__ methods to enable pickling of Vortex arrays in Python. Arrays are serialized using the Vortex IPC format.
For pickle protocol 5+ (Python 3.8+, PEP 574), uses PickleBuffer to keep array buffers separate from the main pickle stream rather than copying them inline. This enables us to use shared memory in the future to potentially zero-copy large arrays even across process boundaries. Protocol 4 and below serialise buffers inline as bytes.
Both protocols share the same deserialization path via decode_ipc_array_buffers, which reconstructs arrays from IPC-encoded buffer lists (or memoryviews).

@onursatici
Copy link
Contributor Author

image

@onursatici onursatici added the feature Release label indicating a new feature or request label Oct 20, 2025
@codecov
Copy link

codecov bot commented Oct 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.87%. Comparing base (28f5b3d) to head (62f5e86).
⚠️ Report is 23 commits behind head on develop.

☔ 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.

@onursatici
Copy link
Contributor Author

better
image

tokio = { workspace = true, features = ["fs", "rt-multi-thread"] }
url = { workspace = true }
vortex = { workspace = true, features = ["object_store", "python", "tokio"] }
vortex-ipc = { workspace = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should expose this via the main vortex crate

@gatesn
Copy link
Contributor

gatesn commented Oct 20, 2025

You're gonna need a PR description for this one 😅

@gatesn
Copy link
Contributor

gatesn commented Oct 21, 2025

I don't understand the two screenshots you sent? Also, is there any point implementing P4 at all? We're way beyond 3.8 as a min version requirement.

@onursatici
Copy link
Contributor Author

🤦‍♂️ right. I did initially add 4 to be a baseline to see if 5 would make a difference. Screenshots show the benchmarks ran comparing 4 vs 5. You are right we have min 3.11 required for vortex python so v4 would not be used, I will remove

@onursatici
Copy link
Contributor Author

onursatici commented Oct 22, 2025

@gatesn protocol 5 is introduced on py3.8, but made default just recently at py3.14, so we still need both

Signed-off-by: Onur Satici <[email protected]>
Signed-off-by: Onur Satici <[email protected]>
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 5, 2025

CodSpeed Performance Report

Merging #5011 will not alter performance

Comparing os/pickle (62f5e86) with develop (ee4b921)1

Summary

✅ 1318 untouched
🆕 7 new
⏩ 162 skipped2

Benchmarks breakdown

Benchmark BASE HEAD Change
🆕 decompress[("alp_for_bp_f64", 0x75e2f0)] N/A 24.2 ms N/A
🆕 decompress[("datetime_for_bp", 0x7615f0)] N/A 34.9 ms N/A
🆕 decompress[("dict_fsst_varbin_bp_string", 0x760700)] N/A 14.5 ms N/A
🆕 decompress[("dict_fsst_varbin_string", 0x760070)] N/A 14.5 ms N/A
🆕 decompress[("dict_varbinview_string", 0x75ed10)] N/A 14.7 ms N/A
🆕 decompress[("for_bp_u64", 0x75dba0)] N/A 2.5 ms N/A
🆕 decompress[("runend_for_bp_u32", 0x75f1c0)] N/A 2 ms N/A

Footnotes

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

  2. 162 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.

Signed-off-by: Onur Satici <[email protected]>
Signed-off-by: Onur Satici <[email protected]>
@onursatici onursatici enabled auto-merge (squash) November 6, 2025 14:14
Signed-off-by: Onur Satici <[email protected]>
@onursatici onursatici merged commit 701bd6c into develop Nov 6, 2025
39 of 40 checks passed
@onursatici onursatici deleted the os/pickle branch November 6, 2025 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Release label indicating a new feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants