Skip to content

Commit 97f49ed

Browse files
authored
fix: support 64K alignment (#8599)
## 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](https://docs.nvidia.com/dccpu/grace-perf-tuning-guide/os-settings.html#page-size), 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`. 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>
1 parent 3c53111 commit 97f49ed

2 files changed

Lines changed: 16 additions & 22 deletions

File tree

vortex-buffer/src/alignment.rs

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ use vortex_error::vortex_err;
1010

1111
/// The alignment of a buffer.
1212
///
13-
/// This type is a wrapper around `usize` that ensures the alignment is a power of 2 and fits into
14-
/// a `u16`.
13+
/// This type is a wrapper around `usize` that ensures the alignment is a non-zero power of 2.
1514
#[derive(Clone, Debug, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
1615
pub struct Alignment(usize);
1716

@@ -32,11 +31,10 @@ impl Alignment {
3231
///
3332
/// ## Panics
3433
///
35-
/// Panics if `align` is not a power of 2, or is greater than `u16::MAX`.
34+
/// Panics if `align` is zero or is not a power of 2.
3635
#[inline]
3736
pub const fn new(align: usize) -> Self {
3837
assert!(align > 0, "Alignment must be greater than 0");
39-
assert!(align <= u16::MAX as usize, "Alignment must fit into u16");
4038
assert!(align.is_power_of_two(), "Alignment must be a power of 2");
4139
Self(align)
4240
}
@@ -63,8 +61,8 @@ impl Alignment {
6361
Self::new(align_of::<T>())
6462
}
6563

66-
/// The largest valid alignment: the greatest power of 2 that fits into a `u16`.
67-
pub const MAX: Alignment = Alignment::new(1 << 15);
64+
/// The largest valid alignment: the greatest power of 2 representable in a `usize`.
65+
pub const MAX: Alignment = Alignment::new(1 << (usize::BITS - 1));
6866

6967
/// Check if `self` alignment is a "larger" than `other` alignment.
7068
///
@@ -110,14 +108,14 @@ impl Alignment {
110108
/// Returns the log2 of the alignment.
111109
pub fn exponent(&self) -> u8 {
112110
u8::try_from(self.0.trailing_zeros())
113-
.vortex_expect("alignment fits into u16, so exponent fits in u7")
111+
.vortex_expect("alignment is a power of 2 within usize, so its exponent fits in u8")
114112
}
115113

116114
/// Create from the log2 exponent of the alignment.
117115
///
118116
/// ## Panics
119117
///
120-
/// Panics if `alignment` is not a power of 2, or is greater than `u16::MAX`.
118+
/// Panics if `1 << exponent` overflows `usize`.
121119
#[inline]
122120
pub const fn from_exponent(exponent: u8) -> Self {
123121
Self::new(1 << exponent)
@@ -160,13 +158,6 @@ impl From<Alignment> for usize {
160158
}
161159
}
162160

163-
impl From<Alignment> for u16 {
164-
#[inline]
165-
fn from(value: Alignment) -> Self {
166-
u16::try_from(value.0).vortex_expect("Alignment must fit into u16")
167-
}
168-
}
169-
170161
impl From<Alignment> for u32 {
171162
#[inline]
172163
fn from(value: Alignment) -> Self {
@@ -184,9 +175,6 @@ impl TryFrom<u32> for Alignment {
184175
if value == 0 {
185176
return Err(vortex_err!("Alignment must be greater than 0"));
186177
}
187-
if value > u16::MAX as usize {
188-
return Err(vortex_err!("Alignment must fit into u16, got {value}"));
189-
}
190178
if !value.is_power_of_two() {
191179
return Err(vortex_err!("Alignment must be a power of 2, got {value}"));
192180
}
@@ -206,9 +194,11 @@ mod test {
206194
}
207195

208196
#[test]
209-
#[should_panic]
210-
fn alignment_overflow() {
211-
Alignment::new(u16::MAX as usize + 1);
197+
fn alignment_above_u16() {
198+
// 64KiB alignment (one past `u16::MAX`) is valid — common on ARM with 64K pages.
199+
let alignment = Alignment::new(u16::MAX as usize + 1);
200+
assert_eq!(*alignment, 1 << 16);
201+
assert_eq!(alignment, Alignment::from_exponent(16));
212202
}
213203

214204
#[test]
@@ -238,6 +228,10 @@ mod test {
238228
Ok(alignment) => assert_eq!(alignment, Alignment::new(8)),
239229
Err(err) => panic!("unexpected error for valid alignment: {err}"),
240230
}
231+
match Alignment::try_from(1u32 << 16) {
232+
Ok(alignment) => assert_eq!(alignment, Alignment::new(1 << 16)),
233+
Err(err) => panic!("64KiB alignment should be valid: {err}"),
234+
}
241235
assert!(Alignment::try_from(0u32).is_err());
242236
assert!(Alignment::try_from(3u32).is_err());
243237
}

vortex-buffer/src/buffer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ pub struct Buffer<T> {
3636
/// valid alignment without allocating. A zero-length slice never reads memory, so it may use a
3737
/// dangling pointer as long as it is non-null and aligned.
3838
const EMPTY_BACKING: &[u8] = {
39-
let addr = 1usize << 20;
39+
let addr = 1usize << (usize::BITS - 1);
4040
assert!(Alignment::MAX.is_offset_aligned(addr));
4141
// SAFETY: the pointer is non-null and aligned, and the slice is zero-length.
4242
unsafe { std::slice::from_raw_parts(std::ptr::without_provenance(addr), 0) }

0 commit comments

Comments
 (0)