about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2019-07-27 09:32:44 +0000
committerbors <bors@rust-lang.org>2019-07-27 09:32:44 +0000
commit0e9b465d729d07101b29b4d096d83edf9be82df0 (patch)
tree210c15e12ca6b9f32a5f75ab901cccc4d8c191b8
parent09e39897587dca70f0b15093d425a682c392349c (diff)
parent44c165074b1b034799c6e1b95a871cde46755632 (diff)
downloadrust-0e9b465d729d07101b29b4d096d83edf9be82df0.tar.gz
rust-0e9b465d729d07101b29b4d096d83edf9be82df0.zip
Auto merge of #62748 - luca-barbieri:optimize-refcell-borrow, r=RalfJung
Optimize RefCell read borrowing

Instead of doing two comparisons we can do only one with a bit of cleverness.

LLVM currently can't do this optimization itself on x86-64.
-rw-r--r--src/libcore/cell.rs20
1 files changed, 15 insertions, 5 deletions
diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs
index 0aaf5269a3d..8579dbf353e 100644
--- a/src/libcore/cell.rs
+++ b/src/libcore/cell.rs
@@ -1101,13 +1101,23 @@ struct BorrowRef<'b> {
 impl<'b> BorrowRef<'b> {
     #[inline]
     fn new(borrow: &'b Cell<BorrowFlag>) -> Option<BorrowRef<'b>> {
-        let b = borrow.get();
-        if is_writing(b) || b == isize::max_value() {
-            // If there's currently a writing borrow, or if incrementing the
-            // refcount would overflow into a writing borrow.
+        let b = borrow.get().wrapping_add(1);
+        if !is_reading(b) {
+            // Incrementing borrow can result in a non-reading value (<= 0) in these cases:
+            // 1. It was < 0, i.e. there are writing borrows, so we can't allow a read borrow
+            //    due to Rust's reference aliasing rules
+            // 2. It was isize::max_value() (the max amount of reading borrows) and it overflowed
+            //    into isize::min_value() (the max amount of writing borrows) so we can't allow
+            //    an additional read borrow because isize can't represent so many read borrows
+            //    (this can only happen if you mem::forget more than a small constant amount of
+            //    `Ref`s, which is not good practice)
             None
         } else {
-            borrow.set(b + 1);
+            // Incrementing borrow can result in a reading value (> 0) in these cases:
+            // 1. It was = 0, i.e. it wasn't borrowed, and we are taking the first read borrow
+            // 2. It was > 0 and < isize::max_value(), i.e. there were read borrows, and isize
+            //    is large enough to represent having one more read borrow
+            borrow.set(b);
             Some(BorrowRef { borrow })
         }
     }