Skip to content

Commit 145f4f4

Browse files
committed
Auto merge of #51630 - joshlf:map-split-perf, r=<try>
Optimize RefCell refcount tracking Address the performance concern raised in #51466 (comment) cc @dtolnay @nnethercote @rust-lang/wg-compiler-performance cc @RalfJung @jhjourdan for soundness concerns Can somebody kick off a perf run on this? I'm not sure how that's done, but I understand it has to be started manually. The idea of this change is to switch to representing mutable refcount as values below 0 to eliminate some branching that was required with the old algorithm.
2 parents ed39523 + 583e283 commit 145f4f4

File tree

1 file changed

+21
-26
lines changed

1 file changed

+21
-26
lines changed

src/libcore/cell.rs

+21-26
Original file line numberDiff line numberDiff line change
@@ -570,20 +570,19 @@ impl Display for BorrowMutError {
570570
}
571571
}
572572

573-
// Values [1, MIN_WRITING-1] represent the number of `Ref` active. Values in
574-
// [MIN_WRITING, MAX-1] represent the number of `RefMut` active. Multiple
575-
// `RefMut`s can only be active at a time if they refer to distinct,
576-
// nonoverlapping components of a `RefCell` (e.g., different ranges of a slice).
573+
// Positive values represent the number of `Ref` active. Negative values
574+
// represent the number of `RefMut` active. Multiple `RefMut`s can only be
575+
// active at a time if they refer to distinct, nonoverlapping components of a
576+
// `RefCell` (e.g., different ranges of a slice).
577577
//
578578
// `Ref` and `RefMut` are both two words in size, and so there will likely never
579579
// be enough `Ref`s or `RefMut`s in existence to overflow half of the `usize`
580-
// range. Thus, a `BorrowFlag` will probably never overflow. However, this is
581-
// not a guarantee, as a pathological program could repeatedly create and then
582-
// mem::forget `Ref`s or `RefMut`s. Thus, all code must explicitly check for
583-
// overflow in order to avoid unsafety.
584-
type BorrowFlag = usize;
580+
// range. Thus, a `BorrowFlag` will probably never overflow or underflow.
581+
// However, this is not a guarantee, as a pathological program could repeatedly
582+
// create and then mem::forget `Ref`s or `RefMut`s. Thus, all code must
583+
// explicitly check for overflow and underflow in order to avoid unsafety.
584+
type BorrowFlag = isize;
585585
const UNUSED: BorrowFlag = 0;
586-
const MIN_WRITING: BorrowFlag = (!0)/2 + 1; // 0b1000...
587586

588587
impl<T> RefCell<T> {
589588
/// Creates a new `RefCell` containing `value`.
@@ -1022,12 +1021,12 @@ impl<'b> BorrowRef<'b> {
10221021
#[inline]
10231022
fn new(borrow: &'b Cell<BorrowFlag>) -> Option<BorrowRef<'b>> {
10241023
let b = borrow.get();
1025-
if b >= MIN_WRITING {
1024+
if b < UNUSED {
10261025
None
10271026
} else {
10281027
// Prevent the borrow counter from overflowing into
10291028
// a writing borrow.
1030-
assert!(b < MIN_WRITING - 1);
1029+
assert!(b != isize::max_value());
10311030
borrow.set(b + 1);
10321031
Some(BorrowRef { borrow })
10331032
}
@@ -1038,7 +1037,7 @@ impl<'b> Drop for BorrowRef<'b> {
10381037
#[inline]
10391038
fn drop(&mut self) {
10401039
let borrow = self.borrow.get();
1041-
debug_assert!(borrow < MIN_WRITING && borrow != UNUSED);
1040+
debug_assert!(borrow > UNUSED);
10421041
self.borrow.set(borrow - 1);
10431042
}
10441043
}
@@ -1052,7 +1051,7 @@ impl<'b> Clone for BorrowRef<'b> {
10521051
debug_assert!(borrow != UNUSED);
10531052
// Prevent the borrow counter from overflowing into
10541053
// a writing borrow.
1055-
assert!(borrow < MIN_WRITING - 1);
1054+
assert!(borrow != isize::max_value());
10561055
self.borrow.set(borrow + 1);
10571056
BorrowRef { borrow: self.borrow }
10581057
}
@@ -1251,12 +1250,8 @@ impl<'b> Drop for BorrowRefMut<'b> {
12511250
#[inline]
12521251
fn drop(&mut self) {
12531252
let borrow = self.borrow.get();
1254-
debug_assert!(borrow >= MIN_WRITING);
1255-
self.borrow.set(if borrow == MIN_WRITING {
1256-
UNUSED
1257-
} else {
1258-
borrow - 1
1259-
});
1253+
debug_assert!(borrow < UNUSED);
1254+
self.borrow.set(borrow + 1);
12601255
}
12611256
}
12621257

@@ -1266,10 +1261,10 @@ impl<'b> BorrowRefMut<'b> {
12661261
// NOTE: Unlike BorrowRefMut::clone, new is called to create the initial
12671262
// mutable reference, and so there must currently be no existing
12681263
// references. Thus, while clone increments the mutable refcount, here
1269-
// we simply go directly from UNUSED to MIN_WRITING.
1264+
// we explicitly only allow going from UNUSED to UNUSED - 1.
12701265
match borrow.get() {
12711266
UNUSED => {
1272-
borrow.set(MIN_WRITING);
1267+
borrow.set(UNUSED - 1);
12731268
Some(BorrowRefMut { borrow: borrow })
12741269
},
12751270
_ => None,
@@ -1284,10 +1279,10 @@ impl<'b> BorrowRefMut<'b> {
12841279
#[inline]
12851280
fn clone(&self) -> BorrowRefMut<'b> {
12861281
let borrow = self.borrow.get();
1287-
debug_assert!(borrow >= MIN_WRITING);
1288-
// Prevent the borrow counter from overflowing.
1289-
assert!(borrow != !0);
1290-
self.borrow.set(borrow + 1);
1282+
debug_assert!(borrow < UNUSED);
1283+
// Prevent the borrow counter from underflowing.
1284+
assert!(borrow != isize::min_value());
1285+
self.borrow.set(borrow - 1);
12911286
BorrowRefMut { borrow: self.borrow }
12921287
}
12931288
}

0 commit comments

Comments
 (0)