Skip to content

fix: support 64K alignment#8599

Merged
a10y merged 1 commit into
developfrom
aduffy/allow-64k
Jun 25, 2026
Merged

fix: support 64K alignment#8599
a10y merged 1 commit into
developfrom
aduffy/allow-64k

Conversation

@a10y

@a10y a10y commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

Our Alignment type currently has a restriction that it fits in a u16. This is easy enough to satisfy most of the time.

However, while doing some testing with GH200 machines, which are ARM and which NVIDIA recommends you setup with 64K page alignment, I have seen some production code fail because it uses a custom allocator that passes back page-aligned memory, and attempting to construct 64K alignment is one past the maximum value so it panics.

What changes are included in this PR?

  • Remove panic if Alignment > u16::MAX, now you can represent 64K alignment on buffers

What APIs are changed? Are there any user-facing changes?

Alignment now has no restriction that it must fit into a u16, so some code which may have previously panicked will no longer panic.

The infallable From<Alignment> for u16 impl has been removed entirely, there were zero callers within Vortex. This is technically a breaking change and anyone who was relying on this behavior should switch to using the exponent instead which will fit in u8.

Signed-off-by: Andrew Duffy <andrew@a10y.dev>
@a10y a10y requested a review from a team June 25, 2026 21:26
@a10y a10y added the changelog/fix A bug fix label Jun 25, 2026
@a10y a10y enabled auto-merge (squash) June 25, 2026 21:29

@robert3005 robert3005 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.

This is one chonky alignment

@codspeed-hq

codspeed-hq Bot commented Jun 25, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚡ 7 improved benchmarks
❌ 6 regressed benchmarks
✅ 1576 untouched benchmarks
⏩ 4 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation copy_nullable[65536] 1 ms 1.4 ms -24.26%
Simulation copy_non_nullable[65536] 908.6 µs 1,089.3 µs -16.58%
Simulation slice_empty_vortex 310 ns 368.6 ns -15.9%
Simulation chunked_varbinview_canonical_into[(100, 100)] 224.4 µs 259.5 µs -13.55%
Simulation map_each[BufferMut<i32>, 128] 407.8 ns 466.1 ns -12.51%
Simulation chunked_varbinview_into_canonical[(100, 100)] 271.5 µs 306.4 µs -11.38%
Simulation chunked_bool_canonical_into[(1000, 10)] 26.8 µs 16.1 µs +66.54%
Simulation bitwise_not_vortex_buffer_mut[128] 244.4 ns 186.1 ns +31.34%
Simulation bitwise_not_vortex_buffer_mut[1024] 304.7 ns 246.4 ns +23.68%
Simulation chunked_varbinview_into_canonical[(1000, 10)] 205.8 µs 168.9 µs +21.85%
Simulation bitwise_not_vortex_buffer_mut[2048] 398.6 ns 340.3 ns +17.14%
Simulation true_count_arrow_buffer[128] 784.7 ns 697.2 ns +12.55%
Simulation rebuild_naive 109.5 µs 98.8 µs +10.79%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing aduffy/allow-64k (5bec64a) with develop (3c53111)

Open in CodSpeed

Footnotes

  1. 4 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.

@a10y a10y merged commit 97f49ed into develop Jun 25, 2026
90 of 94 checks passed
@a10y a10y deleted the aduffy/allow-64k branch June 25, 2026 21:44

a10y commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

take it up with Jensen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants