Skip to content

Commit 1d0597c

Browse files
committed
Revert "Auto merge of rust-lang#99136 - CAD97:layout-faster, r=scottmcm"
This reverts commit 87588a2, reversing changes made to c80dde4.
1 parent eaab930 commit 1d0597c

File tree

2 files changed

+21
-38
lines changed

2 files changed

+21
-38
lines changed

library/core/src/alloc/layout.rs

+20-28
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,3 @@
1-
// Seemingly inconsequential code changes to this file can lead to measurable
2-
// performance impact on compilation times, due at least in part to the fact
3-
// that the layout code gets called from many instantiations of the various
4-
// collections, resulting in having to optimize down excess IR multiple times.
5-
// Your performance intuition is useless. Run perf.
6-
71
use crate::cmp;
82
use crate::fmt;
93
use crate::mem::{self, ValidAlign};
@@ -68,13 +62,6 @@ impl Layout {
6862
return Err(LayoutError);
6963
}
7064

71-
// SAFETY: just checked that align is a power of two.
72-
Layout::from_size_valid_align(size, unsafe { ValidAlign::new_unchecked(align) })
73-
}
74-
75-
/// Internal helper constructor to skip revalidating alignment validity.
76-
#[inline]
77-
const fn from_size_valid_align(size: usize, align: ValidAlign) -> Result<Self, LayoutError> {
7865
// (power-of-two implies align != 0.)
7966

8067
// Rounded up size is:
@@ -89,12 +76,13 @@ impl Layout {
8976
//
9077
// Above implies that checking for summation overflow is both
9178
// necessary and sufficient.
92-
if size > isize::MAX as usize - (align.as_nonzero().get() - 1) {
79+
if size > isize::MAX as usize - (align - 1) {
9380
return Err(LayoutError);
9481
}
9582

96-
// SAFETY: Layout::size invariants checked above.
97-
Ok(Layout { size, align })
83+
// SAFETY: the conditions for `from_size_align_unchecked` have been
84+
// checked above.
85+
unsafe { Ok(Layout::from_size_align_unchecked(size, align)) }
9886
}
9987

10088
/// Creates a layout, bypassing all checks.
@@ -108,8 +96,8 @@ impl Layout {
10896
#[must_use]
10997
#[inline]
11098
pub const unsafe fn from_size_align_unchecked(size: usize, align: usize) -> Self {
111-
// SAFETY: the caller is required to uphold the preconditions.
112-
unsafe { Layout { size, align: ValidAlign::new_unchecked(align) } }
99+
// SAFETY: the caller must ensure that `align` is a power of two.
100+
Layout { size, align: unsafe { ValidAlign::new_unchecked(align) } }
113101
}
114102

115103
/// The minimum size in bytes for a memory block of this layout.
@@ -138,9 +126,10 @@ impl Layout {
138126
#[inline]
139127
pub const fn new<T>() -> Self {
140128
let (size, align) = size_align::<T>();
141-
// SAFETY: if the type is instantiated, rustc already ensures that its
142-
// layout is valid. Use the unchecked constructor to avoid inserting a
143-
// panicking codepath that needs to be optimized out.
129+
// SAFETY: the align is guaranteed by Rust to be a power of two and
130+
// the size+align combo is guaranteed to fit in our address space. As a
131+
// result use the unchecked constructor here to avoid inserting code
132+
// that panics if it isn't optimized well enough.
144133
unsafe { Layout::from_size_align_unchecked(size, align) }
145134
}
146135

@@ -152,6 +141,7 @@ impl Layout {
152141
#[inline]
153142
pub fn for_value<T: ?Sized>(t: &T) -> Self {
154143
let (size, align) = (mem::size_of_val(t), mem::align_of_val(t));
144+
debug_assert!(Layout::from_size_align(size, align).is_ok());
155145
// SAFETY: see rationale in `new` for why this is using the unsafe variant
156146
unsafe { Layout::from_size_align_unchecked(size, align) }
157147
}
@@ -186,6 +176,7 @@ impl Layout {
186176
pub unsafe fn for_value_raw<T: ?Sized>(t: *const T) -> Self {
187177
// SAFETY: we pass along the prerequisites of these functions to the caller
188178
let (size, align) = unsafe { (mem::size_of_val_raw(t), mem::align_of_val_raw(t)) };
179+
debug_assert!(Layout::from_size_align(size, align).is_ok());
189180
// SAFETY: see rationale in `new` for why this is using the unsafe variant
190181
unsafe { Layout::from_size_align_unchecked(size, align) }
191182
}
@@ -289,7 +280,8 @@ impl Layout {
289280
// > less than or equal to `isize::MAX`)
290281
let new_size = self.size() + pad;
291282

292-
// SAFETY: padded size is guaranteed to not exceed `isize::MAX`.
283+
// SAFETY: self.align is already known to be valid and new_size has been
284+
// padded already.
293285
unsafe { Layout::from_size_align_unchecked(new_size, self.align()) }
294286
}
295287

@@ -312,7 +304,7 @@ impl Layout {
312304
let alloc_size = padded_size.checked_mul(n).ok_or(LayoutError)?;
313305

314306
// The safe constructor is called here to enforce the isize size limit.
315-
Layout::from_size_valid_align(alloc_size, self.align).map(|layout| (layout, padded_size))
307+
Layout::from_size_align(alloc_size, self.align()).map(|layout| (layout, padded_size))
316308
}
317309

318310
/// Creates a layout describing the record for `self` followed by
@@ -363,14 +355,14 @@ impl Layout {
363355
#[stable(feature = "alloc_layout_manipulation", since = "1.44.0")]
364356
#[inline]
365357
pub fn extend(&self, next: Self) -> Result<(Self, usize), LayoutError> {
366-
let new_align = cmp::max(self.align, next.align);
358+
let new_align = cmp::max(self.align(), next.align());
367359
let pad = self.padding_needed_for(next.align());
368360

369361
let offset = self.size().checked_add(pad).ok_or(LayoutError)?;
370362
let new_size = offset.checked_add(next.size()).ok_or(LayoutError)?;
371363

372364
// The safe constructor is called here to enforce the isize size limit.
373-
let layout = Layout::from_size_valid_align(new_size, new_align)?;
365+
let layout = Layout::from_size_align(new_size, new_align)?;
374366
Ok((layout, offset))
375367
}
376368

@@ -391,7 +383,7 @@ impl Layout {
391383
pub fn repeat_packed(&self, n: usize) -> Result<Self, LayoutError> {
392384
let size = self.size().checked_mul(n).ok_or(LayoutError)?;
393385
// The safe constructor is called here to enforce the isize size limit.
394-
Layout::from_size_valid_align(size, self.align)
386+
Layout::from_size_align(size, self.align())
395387
}
396388

397389
/// Creates a layout describing the record for `self` followed by
@@ -405,7 +397,7 @@ impl Layout {
405397
pub fn extend_packed(&self, next: Self) -> Result<Self, LayoutError> {
406398
let new_size = self.size().checked_add(next.size()).ok_or(LayoutError)?;
407399
// The safe constructor is called here to enforce the isize size limit.
408-
Layout::from_size_valid_align(new_size, self.align)
400+
Layout::from_size_align(new_size, self.align())
409401
}
410402

411403
/// Creates a layout describing the record for a `[T; n]`.
@@ -416,7 +408,7 @@ impl Layout {
416408
pub fn array<T>(n: usize) -> Result<Self, LayoutError> {
417409
let array_size = mem::size_of::<T>().checked_mul(n).ok_or(LayoutError)?;
418410
// The safe constructor is called here to enforce the isize size limit.
419-
Layout::from_size_valid_align(array_size, ValidAlign::of::<T>())
411+
Layout::from_size_align(array_size, mem::align_of::<T>())
420412
}
421413
}
422414

library/core/src/mem/valid_align.rs

+1-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use crate::convert::TryFrom;
2-
use crate::intrinsics::assert_unsafe_precondition;
32
use crate::num::NonZeroUsize;
43
use crate::{cmp, fmt, hash, mem, num};
54

@@ -27,8 +26,7 @@ impl ValidAlign {
2726
/// It must *not* be zero.
2827
#[inline]
2928
pub(crate) const unsafe fn new_unchecked(align: usize) -> Self {
30-
// SAFETY: Precondition passed to the caller.
31-
unsafe { assert_unsafe_precondition!(align.is_power_of_two()) };
29+
debug_assert!(align.is_power_of_two());
3230

3331
// SAFETY: By precondition, this must be a power of two, and
3432
// our variants encompass all possible powers of two.
@@ -48,13 +46,6 @@ impl ValidAlign {
4846
pub(crate) fn log2(self) -> u32 {
4947
self.as_nonzero().trailing_zeros()
5048
}
51-
52-
/// Returns the alignment for a type.
53-
#[inline]
54-
pub(crate) fn of<T>() -> Self {
55-
// SAFETY: rustc ensures that type alignment is always a power of two.
56-
unsafe { ValidAlign::new_unchecked(mem::align_of::<T>()) }
57-
}
5849
}
5950

6051
impl fmt::Debug for ValidAlign {

0 commit comments

Comments
 (0)