Skip to content

Commit 7fcca37

Browse files
bors[bot]quininer
andauthored
Merge #140
140: Improve `imp_pl` size r=matklad a=quininer I have observed that the program size has increased a lot after enabling the `parking_lot` feature. `parking_lot` will actively inline methods, which leads to some unnecessary duplication of code. see Amanieu/parking_lot#236 This PR introduces an `InlineLessRawMutex` wrapper to avoid this problem. `initialize` is a cold path, I don't think this will cause performance problems. Co-authored-by: quininer <quininer@live.com>
2 parents 2a769dd + 44ea2f2 commit 7fcca37

File tree

3 files changed

+35
-12
lines changed

3 files changed

+35
-12
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## 1.7.1
44

55
- Fix `race::OnceBox<T>` to also impl `Default` even if `T` doesn't impl `Default`.
6+
- Improve code size.
67

78
## 1.7.0
89

src/imp_pl.rs

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,10 @@ impl<T> OnceCell<T> {
4646
where
4747
F: FnOnce() -> Result<T, E>,
4848
{
49-
let _guard = self.mutex.lock();
50-
if !self.is_initialized() {
49+
let mut f = Some(f);
50+
let mut res: Result<(), E> = Ok(());
51+
let slot: *mut Option<T> = self.value.get();
52+
initialize_inner(&self.mutex, &self.is_initialized, &mut || {
5153
// We are calling user-supplied function and need to be careful.
5254
// - if it returns Err, we unlock mutex and return without touching anything
5355
// - if it panics, we unlock mutex and propagate panic without touching anything
@@ -56,15 +58,22 @@ impl<T> OnceCell<T> {
5658
// but that is more complicated
5759
// - finally, if it returns Ok, we store the value and store the flag with
5860
// `Release`, which synchronizes with `Acquire`s.
59-
let value = f()?;
60-
// Safe b/c we have a unique access and no panic may happen
61-
// until the cell is marked as initialized.
62-
let slot: &mut Option<T> = unsafe { &mut *self.value.get() };
63-
debug_assert!(slot.is_none());
64-
*slot = Some(value);
65-
self.is_initialized.store(true, Ordering::Release);
66-
}
67-
Ok(())
61+
let f = f.take().unwrap_or_else(|| unsafe { hint::unreachable_unchecked() });
62+
match f() {
63+
Ok(value) => unsafe {
64+
// Safe b/c we have a unique access and no panic may happen
65+
// until the cell is marked as initialized.
66+
debug_assert!((*slot).is_none());
67+
*slot = Some(value);
68+
true
69+
},
70+
Err(err) => {
71+
res = Err(err);
72+
false
73+
}
74+
}
75+
});
76+
res
6877
}
6978

7079
/// Get the reference to the underlying value, without checking if the cell
@@ -102,6 +111,18 @@ impl<T> OnceCell<T> {
102111
}
103112
}
104113

114+
// Note: this is intentionally monomorphic
115+
#[inline(never)]
116+
fn initialize_inner(mutex: &Mutex<()>, is_initialized: &AtomicBool, init: &mut dyn FnMut() -> bool) {
117+
let _guard = mutex.lock();
118+
119+
if !is_initialized.load(Ordering::Acquire) {
120+
if init() {
121+
is_initialized.store(true, Ordering::Release);
122+
}
123+
}
124+
}
125+
105126
#[test]
106127
fn test_size() {
107128
use std::mem::size_of;

src/imp_std.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ impl<T> OnceCell<T> {
8989
let mut res: Result<(), E> = Ok(());
9090
let slot: *mut Option<T> = self.value.get();
9191
initialize_inner(&self.state_and_queue, &mut || {
92-
let f = f.take().unwrap();
92+
let f = f.take().unwrap_or_else(|| unsafe { unreachable_unchecked() });
9393
match f() {
9494
Ok(value) => {
9595
unsafe { *slot = Some(value) };
@@ -144,6 +144,7 @@ impl<T> OnceCell<T> {
144144

145145
// Corresponds to `std::sync::Once::call_inner`
146146
// Note: this is intentionally monomorphic
147+
#[inline(never)]
147148
fn initialize_inner(my_state_and_queue: &AtomicUsize, init: &mut dyn FnMut() -> bool) -> bool {
148149
let mut state_and_queue = my_state_and_queue.load(Ordering::Acquire);
149150

0 commit comments

Comments
 (0)