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

Better autorelease ergonomics #86

Closed
madsmtm opened this issue Dec 3, 2021 · 8 comments · Fixed by #120
Closed

Better autorelease ergonomics #86

madsmtm opened this issue Dec 3, 2021 · 8 comments · Fixed by #120
Labels
enhancement New feature or request
Milestone

Comments

@madsmtm
Copy link
Owner

madsmtm commented Dec 3, 2021

The ergonomics of using autoreleased references are not great, especially when you are creating your own class. We should try to do better!

I see mainly three use-cases (for library / binding creators; end users probably won't touch this part of the library):

  1. Returning a reference (shared in the example, could also be mutable)
    pub fn description<'pool>(&self, pool: &'pool AutoreleasePool) -> &'pool NSString {
        let desc: *const NSString = msg_send![self, description];
        pool.ptr_as_ref(desc)
    }
  2. Exposing a retained version of the above:
    pub fn description_retained<'pool>(&self, pool: &'pool AutoreleasePool) -> Id<NSString, Shared> {
        // SAFETY: The NSString is valid and not mutably owned anywhere
        // The unsafe is required because the `&NSString` could have come from an `Id<NSString, Owned>`.
        unsafe { Id::retain(self.description(pool)) }
    }
    // Or
    pub fn description_retained(&self) -> Id<NSString, Shared> {
        autoreleasepool(|pool| {
            unsafe { Id::retain(self.description(pool)) }
        })
    }
  3. Returning an autoreleased object in a custom / overridden method:
    extern "C" valid_attributes(_this: &Object, _sel: Sel) -> *mut NSArray {
        let array = Id<NSArray, Owned>::from(vec![...]);
        array.autorelease(pool) // Uhh, we don't have a pool from anywhere; it is implicit in Objective-C
    }
    let mut decl = ClassDecl::new("MyView", class!(NSView)).unwrap();
    decl.add_method(
        sel!(validAttributesForMarkedText),
        valid_attributes as extern "C" fn(&Object, Sel) -> *mut NSArray,
    );

See related: gfx-rs/metal-rs#222

@madsmtm madsmtm added the enhancement New feature or request label Dec 3, 2021
@madsmtm
Copy link
Owner Author

madsmtm commented Dec 3, 2021

Ideas for making use-case 2 possible without unsafe:

  • Add Autoreleased<'pool, T, O: Ownership> helper struct. The ergonomics are unfortunately a bit degraded, since users will have to learn yet another struct. Also not sure what we should do regarding Encode/RefEncode on this?
  • fn from_autoreleased(obj: Autoreleased<'_, T, O>) -> Id<T, O> would be safe (probably?). Here we could possibly use objc_retainAutoreleasedReturnValue for higher performance, see Add Id::retain_autoreleased #81
  • Alternative: Autoreleased<T>, used as &'pool [mut] Autoreleased<T>? Not sure if it would help with the safety.

Ideas for use-case 3:

  • Add fn autoreleasepool_leaking that gives a fake &AutoreleasePool, which could be used as an escape-hatch. Still not very ergonomic!

@madsmtm
Copy link
Owner Author

madsmtm commented Dec 10, 2021

Not sure how Autoreleased<T, Unknown> would work (see #87), if at all, but it is something to consider

@madsmtm
Copy link
Owner Author

madsmtm commented Dec 31, 2021

See Contexts and capabilities (internals thread), this might be very useful for functions that require a reference to the innermost autoreleasepool:

mod rc {
    struct AutoreleasePool { ... }
    capability autoreleasepool<'p> = &'p AutoreleasePool;
}

fn get_data<'p>(bytes: &[u8]) -> &p NSData
with
    rc::autoreleasepool: &'p rc::AutoreleasePool,
{
    let bytes_ptr = bytes.as_ptr() as *const c_void;
    let obj: *const NSData = msg_send![
        cls!(NSData),
        dataWithBytes: bytes_ptr,
        length: bytes.len(),
    ]
    rc::autoreleasepool.ptr_as_ref(obj)
}

fn main() {
    let bytes = &[1, 2, 3];
    with rc::autoreleasepool = &rc::AutoreleasePool::new() {
        let data = get_data(bytes);
        println!("data: {:?}", data);
    }
}

@madsmtm
Copy link
Owner Author

madsmtm commented Jan 6, 2022

Another idea: what if Autoreleased<T, O> didn't have a lifetime parameter, but accessing the value required an active pool:

fn get_thing() -> Autoreleased<Object, Shared>;

autoreleasepool(|| {
    let x: &Object = get_thing().as_ref(pool);
});

Though writing this I realize that would be unsound, since you could move the Autoreleased struct into a different pool and attempt to access the value from there...

The reason that would have been cool is it would allow bypassing autoreleasepools entirely (assuming something like #81):

fn get_thing() -> Autoreleased<Object, Shared>;

let x: Id<Object, Shared> = Id::from_autoreleased(get_thing())

@madsmtm
Copy link
Owner Author

madsmtm commented Feb 22, 2022

I tried benchmarking objc_retainAutoreleasedReturnValue + objc_release (the so-called "optimized caller" setup), results below.

Number of instructions on macOS (using a Callgrind fork and injecting a deterministic allocator):

Expression NSData alloc withBytes NSString alloc init
objc_release(obj) 665 133
[obj autorelease] 688 138
@autoreleasepool { [obj autorelease] } 984 325
objc_release(objc_retainAutoreleasedReturnValue([obj autorelease])) 744 219
@autoreleasepool { objc_release(objc_retainAutoreleasedReturnValue([obj autorelease])) } 927 406

Number of instructions on GNUStep, measured with Callgrind (numbers are not comparable to macOS):

Expression [[NSData alloc] withBytes: "xyz" ...] [[NSString alloc] init]
objc_release(obj) 4766 460
[obj autorelease] 2835 519
@autoreleasepool { [obj autorelease] } 5286 749
objc_release(objc_retainAutoreleasedReturnValue([obj autorelease])) 5028 732
@autoreleasepool { objc_release(objc_retainAutoreleasedReturnValue([obj autorelease])) } 5276 962

Conclusion

The general trend I found (also verified using other methods) is that using objc_retainAutoreleasedReturnValue and converting the autoreleased object to an Id first is basically always preferable in terms of performance (assuming that you can then get by with calling @autoreleasepool less often), which makes sense, it is referred to as "optimized caller"!

(This is the first time I've really done any kind of performance analysis, so bear that in mind).

@madsmtm
Copy link
Owner Author

madsmtm commented Feb 22, 2022

So the direction we want to go in is probably: fewer lifetimes bound to autorelease pools (though they will still be required in edge cases like NSString::to_str), and using objc_retainAutoreleasedReturnValue to convert the value to a lifetime-less Id instead!

So this is both better for ergonomics and for performance, yay! Here #81 would solve use-case 1 and 2, #120 makes it even easier to use.

Ideas for use-case 3: maybe something like #112, or by adding an autorelease_return method to Id which calls objc_autoreleaseReturnValue, or maybe just as part of #30?

@madsmtm madsmtm added this to the objc2 v0.3 milestone Apr 2, 2022
@madsmtm madsmtm mentioned this issue Apr 3, 2022
12 tasks
@madsmtm
Copy link
Owner Author

madsmtm commented Apr 4, 2022

I went with #81 (Id::retain_autoreleased). Use-case 3 is moved to #30. Contexts and capabilities won't be here for a while, so that point is moot.

Outstanding will be closed by #120.

@madsmtm
Copy link
Owner Author

madsmtm commented Jan 28, 2024

I somewhat redid the benchmark using my new M2 Pro, though I had to do it in non-ARC Objective-C using Xcode's profiling tools, as Callgrind doesn't work on M1/M2 yet.

Using NSData* obj = [[NSData alloc] initWithBytes: "1234567890" length: 10];, with one million iterations, I got the following average data points per iteration:

Test CPU Instructions Retired CPU cycles
Leak the object 750 151
objc_release(obj) 884 167
@autoreleasepool { objc_autorelease(obj) } 1088 219
objc_autorelease(obj), @autoreleasepool around all iterations 1521 267
@autoreleasepool { obj = objc_autorelease(obj); __asm__("mov x29, x29"); objc_release(objc_retainAutoreleasedReturnValue(obj)) } 1111 245
obj = objc_autorelease(obj); __asm__("mov x29, x29"); objc_release(objc_retainAutoreleasedReturnValue(obj)), @autoreleasepool around all iterations 966 196

These results are consistent with the conclusion before: The fast autorelease scheme is preferred to putting the object in the autorelease pool, assuming you can get by with calling @autoreleasepool less often.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant