Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Id::retain_autoreleased #81

Merged
merged 6 commits into from
Mar 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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\
Expand Down
6 changes: 3 additions & 3 deletions objc2-foundation/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>> {
Expand Down Expand Up @@ -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![
Expand All @@ -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");
};
Expand Down
4 changes: 2 additions & 2 deletions objc2-foundation/src/dictionary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand All @@ -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()
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion objc2-foundation/src/enumerator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down
2 changes: 1 addition & 1 deletion objc2-foundation/src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down
4 changes: 2 additions & 2 deletions objc2-foundation/src/process_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
}
}
6 changes: 3 additions & 3 deletions objc2-foundation/src/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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) }
}
}

Expand Down
2 changes: 2 additions & 0 deletions objc2/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions objc2/benches/autorelease.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down
120 changes: 119 additions & 1 deletion objc2/src/rc/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
79 changes: 79 additions & 0 deletions objc2/tests/id_retain_autoreleased.rs
Original file line number Diff line number Diff line change
@@ -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);
});
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
.section __TEXT,__text,regular,pure_instructions
.ios_version_min 7, 0
.syntax unified
.globl _handle
.p2align 2
Expand Down
Loading