Improve performance of `setNameWithFormat:`

[hfossli]

Close this issue at will. I felt it was cleaner to continue the discussion in a new thread instead of discussing in a closed thread from 2014 https://github.com/ReactiveCocoa/ReactiveCocoa/issues/1083 on the swift repo.

I think I found a way to improve the performance of setNameWithFormat using blocks. Memory and performance could potentially be improved when setNameWithFormat is active (typically debug builds). I haven't had any problems with this my self, but reading that issue thread https://github.com/ReactiveCocoa/ReactiveCocoa/issues/1083 made me think it's a problem for some people.

Well, instead of creating the strings directly we can defer (and cache) this operation: https://gist.github.com/hfossli/3682cd8769bd4d167f6e05ee4b34af8c

I wrote this test. And my prototype passes that test fine. This should mean better performance and lower memory usage.

@interface Foo : NSObject
@property (nonatomic, assign) int descriptionRequestedCount;
@end

@implementation Foo

- (NSString *)description
{
    self.descriptionRequestedCount++;
    return @"<Foo 0xFOOBAR>";
}

@end

@interface RACSignalTest : XCTestCase
@end
@implementation RACSignalTest

- (void)testWithSignal
{
    Foo *foo = [Foo new];
    RACSignal *signal = [[RACSignal new] someOperatorWithObject:foo];
    XCTAssertTrue(foo.descriptionRequestedCount == 0, @"");
    XCTAssertEqualObjects([signal name], @"transformed using `someOperatorWithObject` with object <Foo 0xFOOBAR>");
    XCTAssertTrue(foo.descriptionRequestedCount == 1, @"");
    XCTAssertEqualObjects([signal name], @"transformed using `someOperatorWithObject` with object <Foo 0xFOOBAR>");
    XCTAssertTrue(foo.descriptionRequestedCount == 1, @"");
}

@end

[mdiep]

The objection to this approach is that it keeps objects alive.

[hfossli]

Are you thinking about the caching?

- (instancetype)setNameBlock:(NSString *(^)(void))nameBlock
{
    __block NSString *cache;
    _nameBlock = [^NSString *{
        if(cache == nil)
        {
            cache = nameBlock();
        }
        return cache;
    } copy];
    return self;
}

Caching can be omitted.

[mdiep]

foo is kept alive in your example:

- (void)testWithSignal
{
    Foo *foo = [Foo new];

    RACSignal *signal = [[RACSignal new] someOperatorWithObject:foo];
    XCTAssertTrue(foo.descriptionRequestedCount == 0, @"");
    XCTAssertEqualObjects([signal name], @"transformed using `someOperatorWithObject` with object <Foo 0xFOOBAR>");
    XCTAssertTrue(foo.descriptionRequestedCount == 1, @"");
    XCTAssertEqualObjects([signal name], @"transformed using `someOperatorWithObject` with object <Foo 0xFOOBAR>");
    XCTAssertTrue(foo.descriptionRequestedCount == 1, @"");
}

[hfossli]

You're right. That means the convenience macro is not a good idea as each object would need to weakly captured.

And my feeling is that this is probably too cumbersome

- (instancetype)someOperatorWithObject:(NSObject *)object
{    
    @weakify(object);
    return [self setNameBlock:^NSString *{ 
        @strongify(object);
        return [NSString stringWithFormat:@"transformed using `someOperatorWithObject` with object %@", [object description]];
    }];
}

[mdiep]

You could probably build the @weakify/@strongify into a macro that sets the name. Feel free to open a PR if you'd like to work on that.

[hfossli]

Yeah, I could do a PR if you guys think this is a good idea. Don't want to invest time and then get it rejected if you fundamentally disagree about the concept :)

[mdiep]

No, that seems like an acceptable approach to me.