Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Array-walking is aligned #1191

Merged
merged 3 commits into from
Jul 6, 2023
Merged

Array-walking is aligned #1191

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ea287a8
Precompute fixed-size array-walking
workingjubilee Jul 4, 2023
91122a7
Remove now-unnecessary assert
workingjubilee Jul 4, 2023
543e8b1
Canonize Align::pad fn
workingjubilee Jul 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 19 additions & 26 deletions pgrx/src/datum/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,6 @@ impl<'a, T: FromDatum> Array<'a, T> {
.map(|nonnull| NullKind::Bits(unsafe { &*nonnull.as_ptr() }))
.unwrap_or(NullKind::Strict(nelems));

// The array-walking code assumes this is always the case, is it?
if let Layout { size: Size::Fixed(n), align, .. } = elem_layout {
let n: usize = n.into();
assert!(
n % (align.as_usize()) == 0,
"typlen does NOT include padding for fixed-width layouts!"
);
}

// do a little two-step before jumping into the Cha-Cha Slide and figure out
// which implementation is correct for the type of element in this Array.
let slide_impl: ChaChaSlideImpl<T> = match elem_layout.pass {
Expand All @@ -154,11 +145,11 @@ impl<'a, T: FromDatum> Array<'a, T> {
// determined at runtime based on the length of the string
Size::CStr => Box::new(casper::PassByCStr),

// Array elements are fixed sizes yet Postgres wants us to pass around by reference
Size::Fixed(size) => Box::new(casper::PassByFixed {
align: elem_layout.align.as_usize(),
size: size as usize,
}),
// Array elements are fixed sizes yet the data is "pass-by-reference"
// Most commonly, this is because of elements larger than a Datum.
Size::Fixed(size) => {
Box::new(casper::PassByFixed::from_size_and_align(size, elem_layout.align))
}
},
};

Expand Down Expand Up @@ -384,6 +375,7 @@ fn as_slice<'a, T: Sized + FromDatum>(array: &'a Array<'_, T>) -> Result<&'a [T]
}

mod casper {
use crate::layout::Align;
use crate::{pg_sys, varlena, Array, FromDatum};

// it's a pop-culture reference (https://en.wikipedia.org/wiki/Cha_Cha_Slide) not some fancy crypto thing you nerd
Expand Down Expand Up @@ -495,9 +487,19 @@ mod casper {
}

pub(super) struct PassByFixed {
pub(super) align: usize,
pub(super) size: usize,
pub(super) padded_size: usize,
}

impl PassByFixed {
pub(super) fn from_size_and_align(size: u16, align: Align) -> Self {
let align = align.as_usize();
let size = size as usize;
let align_mask = size & (align - 1);
let align_offset = if align_mask != 0 { align - align_mask } else { 0 };
PassByFixed { padded_size: size + align_offset }
}
}

impl<T: FromDatum> ChaChaSlide<T> for PassByFixed {
#[inline]
unsafe fn bring_it_back_now(&self, array: &Array<T>, ptr: *const u8) -> Option<T> {
Expand All @@ -507,16 +509,7 @@ mod casper {

#[inline]
unsafe fn hop_size(&self, _ptr: *const u8) -> usize {
// SAFETY: we were told our size upon construction
let varsize = self.size;

// the Postgres realignment code may seem different in form,
// but it's the same in function, just micro-optimized
let align = self.align;
let align_mask = varsize & (align - 1);
let align_offset = if align_mask != 0 { align - align_mask } else { 0 };

varsize + align_offset
self.padded_size
}
}
}
Expand Down