Investigating RAC Performance (with example project) #1503

Closed
twocentstudios opened this Issue Sep 26, 2014 · 30 comments

Comments

Projects
None yet
@twocentstudios

We've been rewriting portions of the Timehop iOS app in MVVM with ReactiveCocoa. So far we've replaced the settings, sign up, and login screens as well as some lower-level services. However, the primary screen of the app (essentially a table view with varying social media related content types) has presented performance problems in several different places that we haven't been able to work around. We're hoping to get some help narrowing down exactly where these performance problems lie.

I posted an issue a little while back about one of the specific issues. I ended up replacing all the RACCommands with blocks in the situation I described there.

Userlist

I've tried to strip down a subset of these problems and present them in the smallest possible example project I could. Userlist is a single table view app that shows a list of randomly generated names and avatars in stock UITableViewCells. I'm hoping this example project can help illuminate the issues we've been having in the Timehop app.

I would encourage anyone to browse the code, clone the project, and run it on your devices if you've got the time and are interested in helping investigate. I'm going to run through the basics below and talk about what I've found so far.

screen-shot

Basics

There's only one model object: User. It has a name string and an image URL.

The UserViewModel asynchronously converts the image URL into an image.

The UserCell displays the name and image of its assigned UserViewModel. Cells are reused normally.

UserController generates and returns an array of randomly generated User objects in a signal.

UsersViewModel fetches the Users array from UserController and maps this array to an array of UserViewModels.

UsersViewController handles the table view, observes the UserViewModel array, attaches UserViewModels to UserCells, and lets UserViewModels know when their cells are visible.

ImageController wraps SDWebImage image network fetching and caching and provides a signal interface.

Motivations

The view and view model hierarchy is significantly more complicated in Timehop than this example app. It has to handle tap events from deep within view hierarchies, display lots of images (sometimes in embedded UICollectionViews), and present several cell layouts too. The goal was to use very modular components at all levels of the view and view model hierarchies, maintain a strict separation of concerns between views and view models, and use ReactiveCocoa conveniences to dramatically reduce the amount and complexity of code in this module.

My approach so far has been to use as much ReactiveCocoa as I can, and after discovering performance problems, slowly replace ReactiveCocoa with imperative code in sections. Unfortunately, it still hasn't gotten us to a viable point yet.

Userlist purposefully makes liberal use of the ReactiveCocoa conveniences I'd like to use in the Timehop app (e.g. RACObserve, RAC bindings,RACCommands, RVM, etc.), in the hopes that someone can tell me I'm doing something wrong with them that's causing problems.

Goals

I've been testing on 5th gen iPod Touches and an iPhone 4S as the baseline. In a perfect world, I'd like to get to 60fps table view scrolling on both these devices and have the time from vending of models to creation of view models be under a second for someone with ~20 pieces of content. In reality, I'd just like to get to the point where any dropped frames are due to layout/display code that can be progressively enhanced. Or more specifically, it should perform "better" than the current Timehop app.

I'd expect the Userlist to easily do 60fps. My goal in writing this is to find a way to get it there and find a way to apply those lessons to Timehop.

Performance

Tentative Problem Spaces

I've been doing basic performance tests on the iPhone 4S with Userlist. Userlist presents a couple of the same performance problems I've seen in the Timehop MVVM prototype. As a quick summary, I've partially narrowed these down to:

  • Creating many non-trivial view models.
  • Creating and executing RACCommands in view models.
  • Observations of view model properties by cells.
  • Triggering subscriptions based on the active property of view models.

Test Setup

I've chosen 100 user objects to test with. 100 is a comfortable maximum for the amount of content a Timehop user has. 100 table cells also allows for room to judge scrolling performance. (An even better stress test is 500 user objects though. The particular pain points may be easier to identify with 500.)

Tests were performed with the project building in Release mode. The Core Animation and Time Profiler were added to Instruments. I ran the project on an iPhone 4S.

The first test was a basic load up, scroll down, scroll up, and repeat with a little time in between each scroll.

userlist-full-run
Instruments trace (updated with symbols)

In the second test I simply wanted a better visual separation of the user object generation section against the user view model generation. For a quick and dirty solution, I placed a 10 second delay RACSignal operation following the random user object generation to accomplish the seperation.

creating-models-view-models
Instruments trace (updated with symbols)

I'll be the first to admit that I'm not particularly well versed in digging into Instruments (I've learned a lot with this project though!). I'm also not that familiar with the implementation details of ReactiveCocoa. Therefore, I won't speculate too much on what I think is going on.

Memory use is in line with what I would expect. I'm not getting thrashing memory warnings or anything. I imagine in a more complex project I would want to purge the images from each inactive UserViewModel when receiving a memory warning. But in this example, everything seems to be nominal.

Next Steps

Imperative Example Benchmark

One thing I haven't done yet is write Userlist in plain MVC without ReactiveCocoa as a benchmark. I've been hesitant because I'm unsure if it will be possible to compare apples-to-apples so to speak. Part of the allure of ReactiveCocoa is the experience writing and maintaining the code, along with the potential for fewer state-related bugs. Each of these is difficult to measure directly. I will go ahead and write it if my relative judgement of what is "slow" on 3-4 year old devices is off (not doubting that it is).

Review

Like I mentioned above, my goal is to tackle these performance issues one step at a time. I'd be very grateful for any feedback on the example project or any related lessons learned working with ReactiveCocoa and MVVM in large iOS projects.

Thanks in advance for any help! (And apologies for the length of this write up.)

@brow

This comment has been minimized.

Show comment
Hide comment
@brow

brow Sep 26, 2014

Contributor

I found in a past project that initializing an RACCommand is expensive -- too expensive to do once for each table cell's view model. Can you hoist loadAvatarImageCommand into UsersViewController so that you initialize that command only once, then let each UserViewController refer to that one instance? (You'll need to set allowsConcurrentExecution = YES, etc.)

Alternatively, can you avoid this use of RACCommand altogether? The redeeming virtue of RACCommand manifests when you bind a control's rac_command property, but it doesn't look like you're doing that with this command.

Of course, you should dig into Time Profiler's detail pane to confirm that this is really the (only) problem. Hopefully it is!

Contributor

brow commented Sep 26, 2014

I found in a past project that initializing an RACCommand is expensive -- too expensive to do once for each table cell's view model. Can you hoist loadAvatarImageCommand into UsersViewController so that you initialize that command only once, then let each UserViewController refer to that one instance? (You'll need to set allowsConcurrentExecution = YES, etc.)

Alternatively, can you avoid this use of RACCommand altogether? The redeeming virtue of RACCommand manifests when you bind a control's rac_command property, but it doesn't look like you're doing that with this command.

Of course, you should dig into Time Profiler's detail pane to confirm that this is really the (only) problem. Hopefully it is!

@Coneko

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Sep 26, 2014

@brow Thanks for the response. Indeed, I've found the same thing: RACCommands are expensive. However, one of the looming questions I haven't been able to answer with Instruments is why they're so expensive.

My concern is that I'll remove the RACCommand, but end up unintentionally rebuilding the entire RACCommand interface to gain the functionality I need and the same problems will crop up.

In the Userlist example, you're right, the RACCommand is overkill. In the Timehop project, I have an ImageViewModel that has additional functionality like showing download progress, tap to retry on failure, and tap to open in a modal.

I'm also more constrained in keeping the fetching logic in the individual view models because the ImageViewModels I mentioned above are unpredictably nested a couple layers deep into the cells' root view models. I can't quite wrap my head around how I'd pass the single RACCommand down the entire view model hierarchy or wire up the resulting signal to meet the requirements above. Maybe you have an example of that pattern somewhere?

@brow Thanks for the response. Indeed, I've found the same thing: RACCommands are expensive. However, one of the looming questions I haven't been able to answer with Instruments is why they're so expensive.

My concern is that I'll remove the RACCommand, but end up unintentionally rebuilding the entire RACCommand interface to gain the functionality I need and the same problems will crop up.

In the Userlist example, you're right, the RACCommand is overkill. In the Timehop project, I have an ImageViewModel that has additional functionality like showing download progress, tap to retry on failure, and tap to open in a modal.

I'm also more constrained in keeping the fetching logic in the individual view models because the ImageViewModels I mentioned above are unpredictably nested a couple layers deep into the cells' root view models. I can't quite wrap my head around how I'd pass the single RACCommand down the entire view model hierarchy or wire up the resulting signal to meet the requirements above. Maybe you have an example of that pattern somewhere?

@joshvera

This comment has been minimized.

Show comment
Hide comment
@joshvera

joshvera Sep 26, 2014

Member

@twocentstudios The reason RACCommand is expensive is because each one sets up about 6 signals internally. If you're not using the entire interface and you're creating one for every cell that adds up.

Like @brow said, I'd suggest reusing a single RACCommand by passing it to the child view model's constructors or by setting it on those view models if that's not possible.

The symbols are stripped so I can't discern much from the instruments trace. If you email me a trace with symbols I can take a look.

Member

joshvera commented Sep 26, 2014

@twocentstudios The reason RACCommand is expensive is because each one sets up about 6 signals internally. If you're not using the entire interface and you're creating one for every cell that adds up.

Like @brow said, I'd suggest reusing a single RACCommand by passing it to the child view model's constructors or by setting it on those view models if that's not possible.

The symbols are stripped so I can't discern much from the instruments trace. If you email me a trace with symbols I can take a look.

@joshvera joshvera added the question label Sep 26, 2014

@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Sep 26, 2014

@joshvera My bad about the symbols. I updated the post with traces that should have symbols.

Trace 1
Trace 2

I'll go ahead and refactor the image loading code from the UserViewModel into its own ImageViewModel and see if I can do it using as few signals as possible. I'll measure performance there and then keep adding features until things start to slow down again.

It sounds like the RACCommands are the cause of the load up slowness. Do you think they're also causing the scrolling slowness?

Related to RACCommands, it doesn't seem like using RVMViewModel as the base class for UserViewModels should hurt performance that much, especially if I decide to avoid the other signals it creates on the fly. Am I right about that?

@joshvera My bad about the symbols. I updated the post with traces that should have symbols.

Trace 1
Trace 2

I'll go ahead and refactor the image loading code from the UserViewModel into its own ImageViewModel and see if I can do it using as few signals as possible. I'll measure performance there and then keep adding features until things start to slow down again.

It sounds like the RACCommands are the cause of the load up slowness. Do you think they're also causing the scrolling slowness?

Related to RACCommands, it doesn't seem like using RVMViewModel as the base class for UserViewModels should hurt performance that much, especially if I decide to avoid the other signals it creates on the fly. Am I right about that?

@joshvera

This comment has been minimized.

Show comment
Hide comment
@joshvera

joshvera Sep 26, 2014

Member

@twocentstudios You're right about RVMViewModel.

Might I suggest getting rid of the command entirely? How about something like:

_networkImageSignal =  [[imageController 
    imageWithURL:self.user.avatarURL]
    replayLazily];

@weakify(self);
RAC(self, avatarImage) = [[[[[self 
    didBecomeActiveSignal]
    flattenMap:^(id _) {
        @strongify(self);
        return self.networkImageSignal;
    }]
    startWith:[UIImage imageNamed:@"user_avatar_placeholder"]]
    deliverOn:RACScheduler.mainThreadScheduler]
    takeUntil:self.rac_prepareForReuseSignal];

And calling -setActive: in -tableView:cellForRowAtIndexPath: instead of -tableView:willDisplayCell:forRowAtIndexPath:.

I'd also suggest something like https://github.com/path/FastImageCache instead of caching with replay signals or a property.

Member

joshvera commented Sep 26, 2014

@twocentstudios You're right about RVMViewModel.

Might I suggest getting rid of the command entirely? How about something like:

_networkImageSignal =  [[imageController 
    imageWithURL:self.user.avatarURL]
    replayLazily];

@weakify(self);
RAC(self, avatarImage) = [[[[[self 
    didBecomeActiveSignal]
    flattenMap:^(id _) {
        @strongify(self);
        return self.networkImageSignal;
    }]
    startWith:[UIImage imageNamed:@"user_avatar_placeholder"]]
    deliverOn:RACScheduler.mainThreadScheduler]
    takeUntil:self.rac_prepareForReuseSignal];

And calling -setActive: in -tableView:cellForRowAtIndexPath: instead of -tableView:willDisplayCell:forRowAtIndexPath:.

I'd also suggest something like https://github.com/path/FastImageCache instead of caching with replay signals or a property.

@jspahrsummers

This comment has been minimized.

Show comment
Hide comment
@jspahrsummers

jspahrsummers Sep 26, 2014

Member

-setActive: in -tableView:cellForRowAtIndexPath: instead of -tableView:willDisplayCell:forRowAtIndexPath:.

@twocentstudios Based on the trace, this suggestion of @joshvera's definitely seems like the most important thing you could do right now. It should almost halve the time being spent.

Member

jspahrsummers commented Sep 26, 2014

-setActive: in -tableView:cellForRowAtIndexPath: instead of -tableView:willDisplayCell:forRowAtIndexPath:.

@twocentstudios Based on the trace, this suggestion of @joshvera's definitely seems like the most important thing you could do right now. It should almost halve the time being spent.

@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Sep 26, 2014

@joshvera I'll throw that code in now and give it a shot. Thanks!

I've looked at Fast Image Cache before but haven't used it yet. My main hesitation is that they specifically say it's not meant to be used for larger images, which is pretty much all Timehop does.

@jspahrsummers Hmm... I made the willDisplay to cellForRow change and I'm not seeing much difference in the stack trace.

Full trace with cellForRow.

I was under the impression those two methods were called pretty closely back to back, both on the main thread.

My intention with the image loading code is to say "Hey cell, you don't need to care what's happening behind the image property. Just always display whatever image is behind that keypath." I thought that by isolating the cell from whatever the view model was doing to load the image, it would guarantee that the only things happening on the main thread would be:

  1. Display (something improved by FastImageCache and/or faster layout techniques).
  2. The cell's observation of the view model's property.
  3. The final delivery of the UIImage to the view model's property to the main thread.

That's just my hypothesis. Items 1, 2, and 3 happen when the view controller loads and the table view is scrolled the first time. Items 1 and 2 happen after all the images have loaded from the network and are cached in a property by each view model.

My next steps are to remove those commands and profile again. Thanks for looking into this with me, guys. The help is greatly appreciated!

@joshvera I'll throw that code in now and give it a shot. Thanks!

I've looked at Fast Image Cache before but haven't used it yet. My main hesitation is that they specifically say it's not meant to be used for larger images, which is pretty much all Timehop does.

@jspahrsummers Hmm... I made the willDisplay to cellForRow change and I'm not seeing much difference in the stack trace.

Full trace with cellForRow.

I was under the impression those two methods were called pretty closely back to back, both on the main thread.

My intention with the image loading code is to say "Hey cell, you don't need to care what's happening behind the image property. Just always display whatever image is behind that keypath." I thought that by isolating the cell from whatever the view model was doing to load the image, it would guarantee that the only things happening on the main thread would be:

  1. Display (something improved by FastImageCache and/or faster layout techniques).
  2. The cell's observation of the view model's property.
  3. The final delivery of the UIImage to the view model's property to the main thread.

That's just my hypothesis. Items 1, 2, and 3 happen when the view controller loads and the table view is scrolled the first time. Items 1 and 2 happen after all the images have loaded from the network and are cached in a property by each view model.

My next steps are to remove those commands and profile again. Thanks for looking into this with me, guys. The help is greatly appreciated!

@kastiglione

This comment has been minimized.

Show comment
Hide comment
@kastiglione

kastiglione Sep 26, 2014

Member

It should almost halve the time being spent.

@jspahrsummers How did you calculate this?

Member

kastiglione commented Sep 26, 2014

It should almost halve the time being spent.

@jspahrsummers How did you calculate this?

@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Sep 28, 2014

I replaced the RACCommand in UserViewModel with a more performance conscious single signal that only fires on the first didBecomeActive. The trace looks pretty good, and the on device performance feels nearly flawless.

// UserViewModel.m

_avatarImage = [UIImage imageNamed:@"user_avatar_placeholder"];

@weakify(self);

// Trigger image load when the view model becomes active.
// Currently ignoring load errors.
[[[[[[self didBecomeActiveSignal]
    take:1]
    flattenMap:^RACSignal *(UserViewModel *userViewModel) {
        return [imageController imageWithURL:userViewModel.user.avatarURL];
    }]
    ignore:nil]
    deliverOn:[RACScheduler mainThreadScheduler]]
    subscribeNext:^(UIImage *image) {
        @strongify(self);
        self.avatarImage = image;
    }];

userlist-full-run-3
Trace

Looking at the trace, you can see that UserViewModel load time has decreased. You can also see the performance of the observation and property setting within the cell after all the images have been loaded. Or in other words, the code:

        RAC(self.imageView, image) = RACObserve(self, viewModel.avatarImage);

Below is another trace of scrolling up and down constantly while images were still being loaded.

user-full-run-3b
Trace

I'm going to add a couple more features to image loading to try to determine what point performance starts to dip.

I replaced the RACCommand in UserViewModel with a more performance conscious single signal that only fires on the first didBecomeActive. The trace looks pretty good, and the on device performance feels nearly flawless.

// UserViewModel.m

_avatarImage = [UIImage imageNamed:@"user_avatar_placeholder"];

@weakify(self);

// Trigger image load when the view model becomes active.
// Currently ignoring load errors.
[[[[[[self didBecomeActiveSignal]
    take:1]
    flattenMap:^RACSignal *(UserViewModel *userViewModel) {
        return [imageController imageWithURL:userViewModel.user.avatarURL];
    }]
    ignore:nil]
    deliverOn:[RACScheduler mainThreadScheduler]]
    subscribeNext:^(UIImage *image) {
        @strongify(self);
        self.avatarImage = image;
    }];

userlist-full-run-3
Trace

Looking at the trace, you can see that UserViewModel load time has decreased. You can also see the performance of the observation and property setting within the cell after all the images have been loaded. Or in other words, the code:

        RAC(self.imageView, image) = RACObserve(self, viewModel.avatarImage);

Below is another trace of scrolling up and down constantly while images were still being loaded.

user-full-run-3b
Trace

I'm going to add a couple more features to image loading to try to determine what point performance starts to dip.

@Tiger6688

This comment has been minimized.

Show comment
Hide comment
@Tiger6688

Tiger6688 Sep 28, 2014

@twocentstudios
good job!
but the code in below line:
https://github.com/Tiger6688/Userlist/blob/master/userlist/UserController.m#L26

NSMutableArray *usersArray = [NSMutableArray array];
//some other code
[subscriber sendNext:[usersArray copy]]; // is the 'copy' necessary in ARC?

@twocentstudios
good job!
but the code in below line:
https://github.com/Tiger6688/Userlist/blob/master/userlist/UserController.m#L26

NSMutableArray *usersArray = [NSMutableArray array];
//some other code
[subscriber sendNext:[usersArray copy]]; // is the 'copy' necessary in ARC?

@jlawton

This comment has been minimized.

Show comment
Hide comment
@jlawton

jlawton Sep 28, 2014

Contributor

@Tiger6688 It's not to do with ARC. The usersArray is mutable, and the copy returns an immutable array. It's good practice, at least, to treat arrays you're passing around as immutable, and this just enforces that. It's unlikely to be a very expensive operation. You're right that it could be omitted with no ill effects here.

Contributor

jlawton commented Sep 28, 2014

@Tiger6688 It's not to do with ARC. The usersArray is mutable, and the copy returns an immutable array. It's good practice, at least, to treat arrays you're passing around as immutable, and this just enforces that. It's unlikely to be a very expensive operation. You're right that it could be omitted with no ill effects here.

@Tiger6688

This comment has been minimized.

Show comment
Hide comment
@Tiger6688

Tiger6688 Sep 28, 2014

@jlawton thanks for your clearness!

@jlawton thanks for your clearness!

@jspahrsummers

This comment has been minimized.

Show comment
Hide comment
@jspahrsummers

jspahrsummers Sep 30, 2014

Member

How did you calculate this?

@kastiglione Sorry, I didn't, really. I was just guesstimating based on the trace (and I was wrong).

Member

jspahrsummers commented Sep 30, 2014

How did you calculate this?

@kastiglione Sorry, I didn't, really. I was just guesstimating based on the trace (and I was wrong).

@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Oct 8, 2014

I finally got a chance to do some more investigation. The next thing on my list was separating out image fetching functionality into its own view model. I added image loading progress indication, tap handler with varying functionality based on the error state, and error retrying.

Relevant commits:
ImageViewModel.m
Project

My implementation of ImageViewModel is very non-reactive. I tried to keep the signal count low to start out with. I'm doing most of the property setting manually in subscribe blocks. There a few unnecessary observation signals I still have which may be able to be optimized out (RAC(self, loading) and RAC(self, hasError)).

One of my most suspected pain points is the bindings that keep the primary view model and its subview models active properties in sync (in this case, there's only one):

RAC(self.imageViewModel, active) = RACObserve(self, active);

I'm almost positive I'll have to do these bindings manually.

There's a simple progress indicator implemented in the cell, but in testing I found that image progress is only returned for certain image sources so it's not immediately visible for the lorem ipsum images.

Now to Instruments:

userlist-full-run-4
Trace

Initial load performance isn't too much worse. I'm not using too many more signals than the previous implementation, so that's as expected.

However, on the initial scroll it looks like we're just about starting to max out the CPU. We wait for all the images to load from the network like the previous runs. Then each speed scroll from top to bottom and bottom to top is hitting about 75% CPU usage whereas the previous implementation was about 50%.

I also repeated the "scroll continuously up and down while all images are still loading" test.

userlist-full-run-4b
Trace

Also seeing about 75% CPU usage during that run.

My next trial will be manually implementing the view model active passthrough and seeing what sort of speed up I see.

If anyone has any comments or suggestions regarding my implementation (of ImageViewModel or the rest of the project) or testing procedure please let me know. Similarly if you have recommendations as to what I should test next, I'm all ears. Thanks!

I finally got a chance to do some more investigation. The next thing on my list was separating out image fetching functionality into its own view model. I added image loading progress indication, tap handler with varying functionality based on the error state, and error retrying.

Relevant commits:
ImageViewModel.m
Project

My implementation of ImageViewModel is very non-reactive. I tried to keep the signal count low to start out with. I'm doing most of the property setting manually in subscribe blocks. There a few unnecessary observation signals I still have which may be able to be optimized out (RAC(self, loading) and RAC(self, hasError)).

One of my most suspected pain points is the bindings that keep the primary view model and its subview models active properties in sync (in this case, there's only one):

RAC(self.imageViewModel, active) = RACObserve(self, active);

I'm almost positive I'll have to do these bindings manually.

There's a simple progress indicator implemented in the cell, but in testing I found that image progress is only returned for certain image sources so it's not immediately visible for the lorem ipsum images.

Now to Instruments:

userlist-full-run-4
Trace

Initial load performance isn't too much worse. I'm not using too many more signals than the previous implementation, so that's as expected.

However, on the initial scroll it looks like we're just about starting to max out the CPU. We wait for all the images to load from the network like the previous runs. Then each speed scroll from top to bottom and bottom to top is hitting about 75% CPU usage whereas the previous implementation was about 50%.

I also repeated the "scroll continuously up and down while all images are still loading" test.

userlist-full-run-4b
Trace

Also seeing about 75% CPU usage during that run.

My next trial will be manually implementing the view model active passthrough and seeing what sort of speed up I see.

If anyone has any comments or suggestions regarding my implementation (of ImageViewModel or the rest of the project) or testing procedure please let me know. Similarly if you have recommendations as to what I should test next, I'm all ears. Thanks!

@Coneko

This comment has been minimized.

Show comment
Hide comment
@Coneko

Coneko Oct 8, 2014

Member

This is great! Thanks for looking at the performance so in-depth.

So setting up and tearing down bindings is the bottleneck right now?

Member

Coneko commented Oct 8, 2014

This is great! Thanks for looking at the performance so in-depth.

So setting up and tearing down bindings is the bottleneck right now?

@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Oct 8, 2014

@Coneko That's my hunch. The Instruments trace is a bit opaque to me. I just see lots of bind:, subscribe:, sendNext:, setKeyPath:, and schedule: all the way down the trace.

@Coneko That's my hunch. The Instruments trace is a bit opaque to me. I just see lots of bind:, subscribe:, sendNext:, setKeyPath:, and schedule: all the way down the trace.

@jspahrsummers

This comment has been minimized.

Show comment
Hide comment
@jspahrsummers

jspahrsummers Oct 8, 2014

Member

Yeah, that's rough. It does look like a lot of signal chain overhead from setting the value of active. 😕

I'm not sure where I'd begin optimizing this.

Member

jspahrsummers commented Oct 8, 2014

Yeah, that's rough. It does look like a lot of signal chain overhead from setting the value of active. 😕

I'm not sure where I'd begin optimizing this.

@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Oct 10, 2014

I made two changes on this run (commit):

  • Replace the view model base class (RVMViewModel) with NSObject.
  • Implement the active property on each view model manually.

To my surprise, I don't see a dramatic decrease in CPU usage (compared to the runs in my last comment).

userlist-full-run-5
Trace

userlist-full-run-5b
Trace

This means that most of the overhead is coming from the cell's observers/bindings or the image fetching signal itself. I'm not going to revert this change so I can keep eliminating signals one at a time.

I made two changes on this run (commit):

  • Replace the view model base class (RVMViewModel) with NSObject.
  • Implement the active property on each view model manually.

To my surprise, I don't see a dramatic decrease in CPU usage (compared to the runs in my last comment).

userlist-full-run-5
Trace

userlist-full-run-5b
Trace

This means that most of the overhead is coming from the cell's observers/bindings or the image fetching signal itself. I'm not going to revert this change so I can keep eliminating signals one at a time.

@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Oct 10, 2014

Removed observers/bindings from UserCell (commit).

Starting to look better.

userlist-full-run-6
Trace

Next is removing observers/binding from ImageView.

Removed observers/bindings from UserCell (commit).

Starting to look better.

userlist-full-run-6
Trace

Next is removing observers/binding from ImageView.

@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Oct 10, 2014

Removed observers/bindings from ImageView (commit). Spoiler alert: holy bananas it's fast.

This is without a doubt some of the ugliest code I've ever written. There's probably a more straightforward way to do imperative stuff with MVVM (delegates maybe?).

userlist-full-run-7
Trace

I'll hold off making any conclusions until tomorrow. But to get the ball rolling: what's the recommended way to do MVVM view to view model communication without ReactiveCocoa?

Removed observers/bindings from ImageView (commit). Spoiler alert: holy bananas it's fast.

This is without a doubt some of the ugliest code I've ever written. There's probably a more straightforward way to do imperative stuff with MVVM (delegates maybe?).

userlist-full-run-7
Trace

I'll hold off making any conclusions until tomorrow. But to get the ball rolling: what's the recommended way to do MVVM view to view model communication without ReactiveCocoa?

@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Oct 10, 2014

Although to be fair, I did make two other changes that potentially helped quite a bit:

I'll do a revert tomorrow and measure the effect of those two changes before removing the ImageViewModel bindings.

Although to be fair, I did make two other changes that potentially helped quite a bit:

I'll do a revert tomorrow and measure the effect of those two changes before removing the ImageViewModel bindings.

@Coneko

This comment has been minimized.

Show comment
Hide comment
@Coneko

Coneko Oct 10, 2014

Member

Can you try to add bindings using raw KVO and see how much overhead that adds?

It's not really an apples to apples comparison when you're only setting values, and not updating them, but raw KVO observing and +keyPathsForValuesAffectingValueForKey: would give you most of the commonly used functionality of RAC bindings minus the automatic removal.

KVOController or another thin wrapper around KVO would restore that and add a lot less overhead than RAC bindings.

I want to know if what to avoid performance-wise is RAC bindings, or any kind of bindings in general.

Member

Coneko commented Oct 10, 2014

Can you try to add bindings using raw KVO and see how much overhead that adds?

It's not really an apples to apples comparison when you're only setting values, and not updating them, but raw KVO observing and +keyPathsForValuesAffectingValueForKey: would give you most of the commonly used functionality of RAC bindings minus the automatic removal.

KVOController or another thin wrapper around KVO would restore that and add a lot less overhead than RAC bindings.

I want to know if what to avoid performance-wise is RAC bindings, or any kind of bindings in general.

@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Oct 11, 2014

@Coneko Good idea. I'll take a look at profiling both raw KVO and KVOController this weekend.

@Coneko Good idea. I'll take a look at profiling both raw KVO and KVOController this weekend.

@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Oct 28, 2014

It took me a while to get back to this, but I finally ran the KVOController tests. Relevant commit: timehop/Userlist@d7f6a7a

Performance is solid with KVO. The graphs aren't exactly comparable in the view model loading section and the image loading section because I left some RAC bindings for hasError and loading in ImageViewModel. But the scrolling portions where the the ImageViewModel is being bound to an ImageView look like there's very little overhead.

userlist-full-run-8
Trace

userlist-full-run-8b
Trace

My take aways:

  • KVO (either raw or with KVOController) is a good compromise of convenience and performance when binding view models to views.
  • Any time you're creating any sort of infinite (non-finite, determined at run-time) list of view models, it's best to use imperative strategies or raw KVO over RAC observations (RACObserve()) and bindings (RAC()) within those view models. Definitely don't use higher level constructs like RACCommands.

I'll leave this issue open for a couple days in case anyone else has other comments or suggestions. Thanks everyone.

It took me a while to get back to this, but I finally ran the KVOController tests. Relevant commit: timehop/Userlist@d7f6a7a

Performance is solid with KVO. The graphs aren't exactly comparable in the view model loading section and the image loading section because I left some RAC bindings for hasError and loading in ImageViewModel. But the scrolling portions where the the ImageViewModel is being bound to an ImageView look like there's very little overhead.

userlist-full-run-8
Trace

userlist-full-run-8b
Trace

My take aways:

  • KVO (either raw or with KVOController) is a good compromise of convenience and performance when binding view models to views.
  • Any time you're creating any sort of infinite (non-finite, determined at run-time) list of view models, it's best to use imperative strategies or raw KVO over RAC observations (RACObserve()) and bindings (RAC()) within those view models. Definitely don't use higher level constructs like RACCommands.

I'll leave this issue open for a couple days in case anyone else has other comments or suggestions. Thanks everyone.

@jspahrsummers

This comment has been minimized.

Show comment
Hide comment
@jspahrsummers

jspahrsummers Oct 29, 2014

Member

@twocentstudios Great investigation and write-up! Sorry to hear about RAC's failings here, although I'm glad you were able to find a workaround.

The Swift API (#1382) will take a dramatically different direction, and so it should be less affected by allocations or underlying KVO cost in cases like these.

Member

jspahrsummers commented Oct 29, 2014

@twocentstudios Great investigation and write-up! Sorry to hear about RAC's failings here, although I'm glad you were able to find a workaround.

The Swift API (#1382) will take a dramatically different direction, and so it should be less affected by allocations or underlying KVO cost in cases like these.

@twocentstudios

This comment has been minimized.

Show comment
Hide comment
@twocentstudios

twocentstudios Oct 29, 2014

@jspahrsummers No worries! My biggest disappointment is that I've gotten so used to the conveniences and idioms of RAC that I don't want to go back. I'll post again about any other interesting findings as I integrate this information back into my larger project.

I'm definitely looking forward to digging into the Swift API. It's awesome you all are blazing new trails with RAC.

@jspahrsummers No worries! My biggest disappointment is that I've gotten so used to the conveniences and idioms of RAC that I don't want to go back. I'll post again about any other interesting findings as I integrate this information back into my larger project.

I'm definitely looking forward to digging into the Swift API. It's awesome you all are blazing new trails with RAC.

@anton-matosov

This comment has been minimized.

Show comment
Hide comment
@anton-matosov

anton-matosov Sep 15, 2015

Did anyone had a chance to check Swift API performance with the reusable cells?

Did anyone had a chance to check Swift API performance with the reusable cells?

@NachoSoto

This comment has been minimized.

Show comment
Hide comment
@NachoSoto

NachoSoto Sep 15, 2015

Member

I recently went through some optimization work when using signals in UICollectionViewCells. Once I realized (thanks to @jspahrsummers) that the bottleneck was that I was using SignalProducer.buffer unnecessarily, and switched to Signal.pipe, I can't notice any performance problems :)

Member

NachoSoto commented Sep 15, 2015

I recently went through some optimization work when using signals in UICollectionViewCells. Once I realized (thanks to @jspahrsummers) that the bottleneck was that I was using SignalProducer.buffer unnecessarily, and switched to Signal.pipe, I can't notice any performance problems :)

@adc-amatosov

This comment has been minimized.

Show comment
Hide comment
@adc-amatosov

adc-amatosov Sep 15, 2015

Thanks! That sounds great!

Thanks! That sounds great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment