Merge pull request #81 from madsmtm/optimized-autorelease-retain

Add `Id::retain_autoreleased`

An optimized version of `Id::retain` that uses `objc_retainAutoreleasedReturnValue`.

.github/workflows/ci.yml

@@ -302,6 +302,13 @@ jobs:
         args: --features ${{ env.FEATURES }} ${{ env.TESTARGS }}
 
     - name: Test in release mode
+      if: ${{ !matrix.dinghy }}
+      uses: actions-rs/cargo@v1
+      with:
+        command: test
+        args: --no-default-features ${{ env.TESTARGS }} --release
+
+    - name: Test in release mode with features
       if: ${{ !matrix.dinghy }}
       uses: actions-rs/cargo@v1
       with:
@@ -346,10 +353,12 @@ jobs:
         xcrun simctl boot $SIM_ID
 
         # Build
-        cargo dinghy build
+        cargo dinghy --device=$SIM_ID build
 
         # Run tests
         cargo dinghy --device=$SIM_ID test --no-default-features
+        cargo dinghy --device=$SIM_ID test --release
+
         # Enable a few features. We're doing it this way because cargo dingy
         # doesn't support specifying features from a workspace.
         sed -i -e '/\[features\]/a\

objc2-foundation/src/array.rs

@@ -140,7 +140,7 @@ impl<T: Message> NSArray<T, Shared> {
     pub fn get_retained(&self, index: usize) -> Id<T, Shared> {
         let obj = self.get(index).unwrap();
         // SAFETY: The object is originally shared (see `where` bound).
-        unsafe { Id::retain(obj as *const T as *mut T).unwrap_unchecked() }
+        unsafe { Id::retain_autoreleased(obj as *const T as *mut T).unwrap_unchecked() }
     }
 
     pub fn to_shared_vec(&self) -> Vec<Id<T, Shared>> {
@@ -247,7 +247,7 @@ impl<T: Message, O: Ownership> NSMutableArray<T, O> {
     pub fn replace(&mut self, index: usize, obj: Id<T, O>) -> Id<T, O> {
         let old_obj = unsafe {
             let obj = self.get(index).unwrap();
-            Id::retain(obj as *const T as *mut T).unwrap_unchecked()
+            Id::retain_autoreleased(obj as *const T as *mut T).unwrap_unchecked()
         };
         unsafe {
             let _: () = msg_send![
@@ -262,7 +262,7 @@ impl<T: Message, O: Ownership> NSMutableArray<T, O> {
     #[doc(alias = "removeObjectAtIndex:")]
     pub fn remove(&mut self, index: usize) -> Id<T, O> {
         let obj = if let Some(obj) = self.get(index) {
-            unsafe { Id::retain(obj as *const T as *mut T).unwrap_unchecked() }
+            unsafe { Id::retain_autoreleased(obj as *const T as *mut T).unwrap_unchecked() }
         } else {
             panic!("removal index should be < len");
         };

objc2-foundation/src/dictionary.rs

@@ -103,7 +103,7 @@ impl<K: Message, V: Message> NSDictionary<K, V> {
     pub fn keys_array(&self) -> Id<NSArray<K, Shared>, Shared> {
         unsafe {
             let keys = msg_send![self, allKeys];
-            Id::retain(keys).unwrap()
+            Id::retain_autoreleased(keys).unwrap()
         }
     }
 
@@ -130,7 +130,7 @@ impl<K: Message, V: Message> NSDictionary<K, V> {
     pub fn into_values_array(dict: Id<Self, Owned>) -> Id<NSArray<V, Owned>, Shared> {
         unsafe {
             let vals = msg_send![dict, allValues];
-            Id::retain(vals).unwrap()
+            Id::retain_autoreleased(vals).unwrap()
         }
     }
 }

objc2-foundation/src/enumerator.rs

@@ -23,7 +23,7 @@ impl<'a, T: Message> NSEnumerator<'a, T> {
     /// ownership.
     pub unsafe fn from_ptr(ptr: *mut Object) -> Self {
         Self {
-            id: unsafe { Id::retain(ptr) }.unwrap(),
+            id: unsafe { Id::retain_autoreleased(ptr) }.unwrap(),
             item: PhantomData,
         }
     }

objc2-foundation/src/object.rs

@@ -24,7 +24,7 @@ impl NSObject {
         unsafe {
             let result: *mut NSString = msg_send![self, description];
             // TODO: Verify that description always returns a non-null string
-            Id::retain(result).unwrap()
+            Id::retain_autoreleased(result).unwrap()
         }
     }
 

objc2-foundation/src/process_info.rs

@@ -22,11 +22,11 @@ impl NSProcessInfo {
         // currentThread is @property(strong), what does that mean?
         let obj: *mut Self = unsafe { msg_send![Self::class(), processInfo] };
         // TODO: Always available?
-        unsafe { Id::retain(obj).unwrap() }
+        unsafe { Id::retain_autoreleased(obj).unwrap() }
     }
 
     pub fn process_name(&self) -> Id<NSString, Shared> {
         let obj: *mut NSString = unsafe { msg_send![Self::class(), processName] };
-        unsafe { Id::retain(obj).unwrap() }
+        unsafe { Id::retain_autoreleased(obj).unwrap() }
     }
 }

objc2-foundation/src/thread.rs

@@ -20,7 +20,7 @@ impl NSThread {
         // TODO: currentThread is @property(strong), what does that mean?
         let obj: *mut Self = unsafe { msg_send![Self::class(), currentThread] };
         // TODO: Always available?
-        unsafe { Id::retain(obj).unwrap() }
+        unsafe { Id::retain_autoreleased(obj).unwrap() }
     }
 
     /// Returns the [`NSThread`] object representing the main thread.
@@ -29,7 +29,7 @@ impl NSThread {
         let obj: *mut Self = unsafe { msg_send![Self::class(), mainThread] };
         // The main thread static may not have been initialized
         // This can at least fail in GNUStep!
-        unsafe { Id::retain(obj).expect("Could not retrieve main thread.") }
+        unsafe { Id::retain_autoreleased(obj).expect("Could not retrieve main thread.") }
     }
 
     /// Returns `true` if the thread is the main thread.
@@ -41,7 +41,7 @@ impl NSThread {
     /// The name of the thread.
     pub fn name(&self) -> Option<Id<NSString, Shared>> {
         let obj: *mut NSString = unsafe { msg_send![self, name] };
-        unsafe { Id::retain(obj) }
+        unsafe { Id::retain_autoreleased(obj) }
     }
 }
 

objc2/CHANGELOG.md

@@ -13,6 +13,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * Added `Bool::as_bool` (more descriptive name than `Bool::is_true`).
 * Added convenience method `Id::as_ptr`.
 * The `objc2-encode` dependency is now exposed as `objc2::encode`.
+* Added `Id::retain_autoreleased` to allow following Cocoas memory management
+  rules more efficiently.
 
 ### Changed
 * **BREAKING**: Changed signature of `Id::new` and `Id::retain` from

objc2/benches/autorelease.rs

@@ -1,7 +1,6 @@
 use core::ffi::c_void;
 use std::mem::ManuallyDrop;
 
-use objc2::ffi;
 use objc2::rc::{autoreleasepool, Id, Shared};
 use objc2::runtime::{Class, Object, Sel};
 use objc2::{class, msg_send, sel};
@@ -94,8 +93,7 @@ fn autoreleased_nsstring() -> *mut Object {
 }
 
 fn retain_autoreleased(obj: *mut Object) -> Id<Object, Shared> {
-    let obj = unsafe { ffi::objc_retainAutoreleasedReturnValue(obj.cast()) };
-    unsafe { Id::new(obj.cast()).unwrap_unchecked() }
+    unsafe { Id::retain_autoreleased(obj.cast()).unwrap_unchecked() }
 }
 
 fn autoreleased_nsdata_pool_cleanup() -> *mut Object {
@@ -133,6 +131,7 @@ macro_rules! main_with_warmup {
             )+
         }
 
+        // Required to get DYLD to resolve the stubs on x86_64
         fn warmup() {
             $(
                 warmup_fns::$f();

objc2/src/rc/id.rs

@@ -210,6 +210,9 @@ impl<T: Message, O: Ownership> Id<T, O> {
     /// some API, and you would like to ensure that the object stays around
     /// so that you can work with it.
     ///
+    /// If said API is a normal Objective-C method, you probably want to use
+    /// [`Id::retain_autoreleased`] instead.
+    ///
     /// This is rarely used to construct owned [`Id`]s, see [`Id::new`] for
     /// that.
     ///
@@ -249,6 +252,122 @@ impl<T: Message, O: Ownership> Id<T, O> {
         unsafe { Self::new(res as *mut T) }
     }
 
+    /// Retains a previously autoreleased object pointer.
+    ///
+    /// This is useful when calling Objective-C methods that return
+    /// autoreleased objects, see [Cocoa's Memory Management Policy][mmRules].
+    ///
+    /// This has exactly the same semantics as [`Id::retain`], except it can
+    /// sometimes avoid putting the object into the autorelease pool, possibly
+    /// yielding increased speed and reducing memory pressure.
+    ///
+    /// Note: This relies heavily on being inlined right after [`msg_send!`],
+    /// be careful not accidentally require instructions between these.
+    ///
+    /// [mmRules]: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/mmRules.html
+    ///
+    /// # Safety
+    ///
+    /// Same as [`Id::retain`].
+    #[doc(alias = "objc_retainAutoreleasedReturnValue")]
+    #[inline(always)]
+    pub unsafe fn retain_autoreleased(ptr: *mut T) -> Option<Id<T, O>> {
+        // Add magic nop instruction to participate in the fast autorelease
+        // scheme.
+        //
+        // See `callerAcceptsOptimizedReturn` in `objc-object.h`:
+        // https://github.com/apple-oss-distributions/objc4/blob/objc4-838/runtime/objc-object.h#L1209-L1377
+        //
+        // We will unconditionally emit these instructions, even if they end
+        // up being unused (for example because we're unlucky with inlining,
+        // some other work is done between the objc_msgSend and this, or the
+        // runtime version is too old to support it).
+        //
+        // It may seem like there should be a better way to do this, but
+        // emitting raw assembly is exactly what Clang and Swift does:
+        // swiftc: https://github.com/apple/swift/blob/swift-5.5.3-RELEASE/lib/IRGen/GenObjC.cpp#L148-L173
+        // Clang: https://github.com/llvm/llvm-project/blob/889317d47b7f046cf0e68746da8f7f264582fb5b/clang/lib/CodeGen/CGObjC.cpp#L2339-L2373
+        //
+        // Resources:
+        // - https://www.mikeash.com/pyblog/friday-qa-2011-09-30-automatic-reference-counting.html
+        // - https://www.galloway.me.uk/2012/02/how-does-objc_retainautoreleasedreturnvalue-work/
+        // - https://github.com/gfx-rs/metal-rs/issues/222
+        // - https://news.ycombinator.com/item?id=29311736
+        // - https://stackoverflow.com/a/23765612
+        //
+        // SAFETY:
+        // Based on https://doc.rust-lang.org/stable/reference/inline-assembly.html#rules-for-inline-assembly
+        //
+        // We don't care about the value of the register (so it's okay to be
+        // undefined), and its value is preserved.
+        //
+        // nomem: No reads or writes to memory are performed (this `mov`
+        //   operates entirely on registers).
+        // preserves_flags: `mov` doesn't modify any flags.
+        // nostack: We don't touch the stack.
+
+        // Only worth doing on the Apple runtime.
+        // Not supported on TARGET_OS_WIN32.
+        #[cfg(all(apple, not(target_os = "windows")))]
+        {
+            // Supported since macOS 10.7.
+            #[cfg(target_arch = "x86_64")]
+            {} // x86_64 looks at the next call instruction
+
+            // Supported since macOS 10.8.
+            #[cfg(target_arch = "arm")]
+            unsafe {
+                core::arch::asm!("mov r7, r7", options(nomem, preserves_flags, nostack))
+            };
+
+            // Supported since macOS 10.10.
+            #[cfg(target_arch = "aarch64")]
+            unsafe {
+                core::arch::asm!("mov fp, fp", options(nomem, preserves_flags, nostack))
+            };
+
+            // Supported since macOS 10.12.
+            #[cfg(target_arch = "x86")]
+            unsafe {
+                core::arch::asm!("mov ebp, ebp", options(nomem, preserves_flags, nostack))
+            };
+        }
+
+        let ptr = ptr as *mut ffi::objc_object;
+
+        // SAFETY: Same as `retain`, `objc_retainAutoreleasedReturnValue` is
+        // just an optimization.
+        let res = unsafe { ffi::objc_retainAutoreleasedReturnValue(ptr) };
+
+        // Ideally, we'd be able to specify that the above call should never
+        // be tail-call optimized (become a `jmp` instruction instead of a
+        // `call`); Rust doesn't really have a way of doing this currently, so
+        // we just emit a simple `nop` to make such tail-call optimizations
+        // less likely to occur.
+        //
+        // This is brittle! We should find a better solution!
+        #[cfg(all(apple, not(target_os = "windows"), target_arch = "x86_64"))]
+        {
+            // SAFETY: Similar to above.
+            unsafe { core::arch::asm!("nop", options(nomem, preserves_flags, nostack)) };
+            // TODO: Possibly more efficient alternative?
+            // #![feature(asm_sym)]
+            // core::arch::asm!(
+            //     "mov rdi, rax",
+            //     "call {}",
+            //     sym objc2::ffi::objc_retainAutoreleasedReturnValue,
+            //     inout("rax") obj,
+            //     clobber_abi("C"),
+            // );
+        }
+
+        debug_assert_eq!(
+            res, ptr,
+            "objc_retainAutoreleasedReturnValue did not return the same pointer"
+        );
+        unsafe { Self::new(res as *mut T) }
+    }
+
     #[inline]
     fn autorelease_inner(self) -> *mut T {
         // Note that this (and the actual `autorelease`) is not an associated
@@ -267,7 +386,6 @@ impl<T: Message, O: Ownership> Id<T, O> {
         res as *mut T
     }
 
-    // TODO: objc_retainAutoreleasedReturnValue
     // TODO: objc_autoreleaseReturnValue
     // TODO: objc_retainAutorelease
     // TODO: objc_retainAutoreleaseReturnValue

objc2/tests/id_retain_autoreleased.rs

@@ -0,0 +1,79 @@
+use std::ffi::c_void;
+
+use objc2::rc::{autoreleasepool, Id, Shared};
+use objc2::runtime::Object;
+use objc2::{class, msg_send};
+
+fn retain_count(obj: &Object) -> usize {
+    unsafe { msg_send![obj, retainCount] }
+}
+
+fn create_data(bytes: &[u8]) -> Id<Object, Shared> {
+    let bytes_ptr = bytes.as_ptr() as *const c_void;
+    unsafe {
+        // let obj: *mut Object = msg_send![
+        //     class!(NSMutableData),
+        //     dataWithBytes: bytes_ptr,
+        //     length: bytes.len(),
+        // ];
+        //
+        // On x86 (and perhaps others), dataWithBytes does not tail call
+        // `autorelease` and hence the return address points into that instead
+        // of our code, making the fast autorelease scheme fail.
+        //
+        // So instead, we call `autorelease` manually here.
+        let obj: *mut Object = msg_send![class!(NSMutableData), alloc];
+        let obj: *mut Object = msg_send![
+            obj,
+            initWithBytes: bytes_ptr,
+            length: bytes.len(),
+        ];
+        let obj: *mut Object = msg_send![obj, autorelease];
+        // All code between the `msg_send!` and the `retain_autoreleased` must
+        // be able to be optimized away for this to work.
+        Id::retain_autoreleased(obj).unwrap()
+    }
+}
+
+#[test]
+fn test_retain_autoreleased() {
+    #[cfg(gnustep)]
+    unsafe {
+        objc2::__gnustep_hack::get_class_to_force_linkage()
+    };
+
+    #[cfg(apple)]
+    #[link(name = "Foundation", kind = "framework")]
+    extern "C" {}
+
+    autoreleasepool(|_| {
+        // Run once to allow DYLD to resolve the symbol stubs.
+        // Required for making `retain_autoreleased` work on x86_64.
+        let _data = create_data(b"12");
+
+        // When compiled in release mode / with optimizations enabled,
+        // subsequent usage of `retain_autoreleased` will succeed in retaining
+        // the autoreleased value!
+        let expected = if cfg!(gnustep) {
+            1
+        } else if cfg!(any(
+            debug_assertions,
+            feature = "exception",
+            feature = "verify_message"
+        )) {
+            2
+        } else {
+            1
+        };
+
+        let data = create_data(b"34");
+        assert_eq!(retain_count(&data), expected);
+
+        let data = create_data(b"56");
+        assert_eq!(retain_count(&data), expected);
+
+        // Here we manually clean up the autorelease, so it will always be 1.
+        let data = autoreleasepool(|_| create_data(b"78"));
+        assert_eq!(retain_count(&data), 1);
+    });
+}

tests/assembly/test_msg_send_zero_cost/expected/armv7-apple-ios.s

@@ -1,5 +1,4 @@
 	.section	__TEXT,__text,regular,pure_instructions
-	.ios_version_min 7, 0
 	.syntax unified
 	.globl	_handle
 	.p2align	2