Scrolling perfomance on iOS8

[etolstoy]

I've encountered a strange behaviour on iOS8. When I scroll the collectionView on the device with iOS8 GM, it performs with some lags, while on another device with iOS 7.1 everything is fine. Is it a well-known issue?

[jessesquires]

thanks @igrekde - no one has reported this yet. there are other GM bugs too. hopefully this will resolve itself.

[etolstoy]

@jessesquires I've tested one of the apps using your library - Parse chat and noticed the same issue.

[rad182]

I too experience this performance issue

[madewulf]

Yep, I saw that too.

[etolstoy]

BTW, this problem can be seen even in the demo application.

[jessesquires]

Hey guys -- thanks for the updates!

We addressed scrolling performance issues awhile back in v5.0.3, but it looks like the issue is back. I'm not 100% sure how to resolve this, so any help is much appreciated!

Also, the release-6.0 branch now has a stable demo. I've made some changes here that should help with scrolling. It feels like it has improved to me, but it still doesn't seem as smooth as iOS 7.0. Let me know what you think!

[etolstoy]

@jessesquires I've tested the demo app in release-6 branch, but I still experience the same perfomance issues - maybe a slightly better than in the master.

[madewulf]

@jessesquires FYI, I just spent a couple hours trying to find where the problem is. All I can say right now is that if you comment out this line:

https://github.com/jessesquires/JSQMessagesViewController/blob/release-6.0/JSQMessagesViewController/Controllers/JSQMessagesViewController.m#L433

everything begins to run more smoothly on my iPhone5. Obviously, this is not an option :-) That said, it seems to rule out problems with rendering the bubbles or the avatars images, at least.

In my test, I simply added 50 outgoing messages in a loop to populate the conversation. The profile shows a lot of layouting activity for the UICollectionView, so my guess is simply that finding a solution for the constraint system of the cell takes too much time, which happens only when the constraints have changed (so, in my test case, when the text has changed).

Hope this helps.

I thought that this SO answer might help too, if you did not see it already: http://stackoverflow.com/questions/18746929/using-auto-layout-in-uitableview-for-dynamic-cell-layouts-variable-row-heights#comment35309097_18746930

For the record, I just spent a few days trying to get a smooth UITableview running with cells layed out using Autolayout, and I never managed to get something satisfactory. I'm guessing that autolayout is still a bit too CPU intensive right now. Doing it "manually" in LayoutSubviews, in the other hands, did give excellent results. Just a data point.

[jessesquires]

Thanks so much for the investigation @madewulf ! Very interesting.

What makes this worse is that there are lots of optimizations already in place:

1. we cache computed bubble sizes for use later
2. we have a custom invalidation context
3. we prevent constraint updates unless they have changed

These yielded great scrolling results under iOS 7. So something has changed with iOS 8.

Good to know that is it not the bubble or avatar images. That would have been my first guess.

I wonder if the issue is with data detectors? We should try turning them off.

[LeoNatan]

I profiled the app, and it seems the issue here is the use of AutoLayout. Once everything related to auto layout is turned off, performance is suddenly very smooth.

You should consider dropping auto layout completely. There is nothing that seems difficult to layout manually, especially in cells, where you already have all the heights (or can calculate them using framework methods.

[jessesquires]

Thanks @LeoNatan !

To reiterate, this was not an issue on iOS 7. This was introduced in iOS 8. :cry:

Dropping auto-layout is not an option — with size classes and iPhone 6, it should be clear that manual layouts will not suffice. There's a reason Apple has introduced these frameworks — this is the best way to implement adaptive UI. The previous version (v4.0.x) of this library did not use auto-layout — a decision that was the source of countless bugs, limitations, and one reason why v5.0 was a complete re-write. Auto-layout is the reason why this library still "just works" on both iPhone 6 and iPhone 6 Plus in portrait and landscape. (All all other devices :smile: )

However! I think (hope) we can fix this. Don't worry, this is high-priority.

Also — everyone's comments here are a huge help! Thanks so much! Keep them coming. :+1:

[LeoNatan]

I don't really see why in this case. The project is iOS7 and above, so TextKit can give you a very precise estimation of text size. For other things, the API asks the user for precise sizes. You know the container size, you know the layout size and you hear when these sizes change. All this amounts to actually quite a simple (but perhaps a little tedious) layoutSubviews. When the cell is sized correctly by the layout, performing internal layout is just as simple as pinning manually the avatar image and top and bottom to edges, and resizing the text area to fit the rest. For the bottom bar, the Auto Layout can remain, I doubt it has any real performance penalty.

I can help with this if you are interested.

[jessesquires]

@LeoNatan -- any/all help is much appreciated!

However, we should continue using auto-layout for the reasons above, but also because the 6.0 release will be finished soon. There are already significant changes in the library for 6.0 — overhauling all of the layout code is too risky this close to release.

For anyone wanting to submit a PR for this issue, please submit to branch release-6.0.

[LeoNatan]

Release 6 is shaping up to be a very good release. Very cool additions.

The library is not really usable at this point. Devices such as 4S, iPad 2 and mini (non retina) really are not able to push the CPU power needed to layout the scene. I will comment only on iOS8 because I did not test on below, nor is it a consolation. With media attachments, the problem becomes even more so relevant.

In my project, I am still in a technology research phase and POC, so I don't mind the performance hit right now. But I cannot start work with this project with the current performance limitations.

If nothing comes up regarding Auto Layout performance, I will either write a lib of my own or help rewrite this one if timetable permits. Please let me know when possible.

[jessesquires]

Hello all:

After some investigation with Instruments, I've found some issues regarding scrolling performance and fixed them. Still not 100% but I think it is much better. Please pull the latest from develop and let me know how it goes.

[jessesquires]

Profiling data from Instruments Timer Profiler:

1. Lots of time spent in collectionView: cellForItemAtIndexPath:
   • 44.8% dequeueReusableCellWithReuseIdentifier: forIndexPath:
   • 16.5% cell.textView.text = messageText;
   • 30.6% cell.cellTopLabel.attributedText = ...
2. Lots of time spent in prepareForReuse / applyLayoutAttributes:
   • 51.4% self.textViewFrameInsets = customAttributes.textViewFrameInsets;
3. Lots of time spent in messageBubbleSizeForItemAtIndexPath:
   • 90.1% boundingRectWithSize:

Clearly, there are some additional areas to optimize. Right now, I'm thinking applyLayoutAttributes: could be better.

If anyone else has additional information, please report back here. :+1:

[etolstoy]

Hi, @jessesquires. I've tested the latest version of the library - it seems that the scrolling perfomance has improved a little - but it still works badly, especially with a fast scrolling. Anyway, thanks a lot for your effort. I hope, that you'll completely fix the issue.

[LeoNatan]

For me on 5S it didn't improve sadly.

[jessesquires]

@LeoNatan - really? I'm testing mostly on an iPhone 5 and performance is not that bad. It's noticeably not as smooth as before, but it isn't terrible.

[jessesquires]

Hello all: a bit more progress here. Please checkout the latest on develop and let me know how this is working now.

[etolstoy]

@jessesquires I've tested the latest version - the perfomance is great!

[LeoNatan]

Now I'm having expectations ^^

[dereck009]

I can't say I'm happy with the performance. Scrolling has lags and janks even on a 5S which is troublesome considering I'd like to support devices back to 4S (as iOS 8 does).

Do you plan to improve scrolling after 6.0 is released (I'm guess you won't delay the launch just for this)?

Or is the "ok" performance on a 5S acceptable, cause it's not a priority?

[jessesquires]

@dereck009 - I'll continue making as much progress as I can on this issue (any help is appreciated! :smile:) until 6.0 is ready (which it almost is). I will likely release 6.0 without this issue completely resolved, then keep working to fix it.

[jessesquires]

@dereck009 -- just to confirm, you tested with the latest on develop?

[LeoNatan]

I merged the latest develop into my own and scrolling still stutters sadly.

[jessesquires]

@LeoNatan - but it's better than before? :pray:

[LeoNatan]

Sorry, not really. I saw your edits. They don't really improve things, because constraint update and layout passes are performed out of current run loop, so calling setNeedsLayout or setNeedsUpdateConstraints many times does not really hinder performance.

[LeoNatan]

Let's do it together, let's remove auto layout from cells! 😆

[dereck009]

Yes it was develop branch. I appreciate your work, it's really a great project.

Unfortunately I only have access to iPhone 4S with iOS 7 (not my phone), so I can't really test the problems iOS 8 caused there, I can only deduct from the 5S performance.

The 5.x.x wasn't smooth on the 4S either so I had some custom optimizations (like using TTTAttributedLabel instead of UITextView inside the bubbles) which made it 60 FPS. I don't have the time to deep dive into the new develop branch right now.

[ghazel]

Almost as an aside (I haven't tested the speed of the new branch at all), I was considering TTTAttributedLabel instead of UITextView inside the bubbles as well, since I believe it simplifies layout. If that also improves speed, maybe that's a good direction?

[dereck009]

Yes, I did some extensive testing & profiling and UITextView turned out to be a bottleneck (in v5.x.x). TTTAttributedLabel performed considerably faster.

I didn't run a full circle with compatibility testing, that's why I didn't create a PR, but it worked perfectly in my project.

[jessesquires]

TTTAttributedLabel is a good idea. That might be worth investigating.

[kartikthapar]

Checked scrolling on iPhone 4S and 5S — it looks pretty bad. I haven't really tested previous builds but this is coming straight after JSQMVC5.x. Further, the performance seems okay on iPhone 6 but it isn't as smooth as in other apps for example the Facebook Messages app on iOS 8 on iPhone 6.

[wdcurry]

I don't think "pretty bad" is fair. I am testing the demo on my 5 and it is definitely acceptable. I run messenger and the demo as sidexside as one can, and the average person is not even going to notice.

Can it be better? Yes. Is it bad? No.

[kartikthapar]

Damnit, I thought I edited that when I appended to my commented. It's definitely not 'pretty bad' — apologies for that. However it's definitely noticeable coming from 5.x. I just haven't tested previous builds so I don't know exactly how much better it has gotten in the last few days.

"demo as sidexside as one can, and the average person is not even going to notice" — I am not sure given I am running the example code straight off of the source. I think I need to run some tests.

[jessesquires]

Thanks for the feedback guys!

I'm strongly considering TTTAttributedLabel as it also solves other issues (namely, it doesn't have the problems associated with selectable). Also, I trust Matttt's code.

[jessesquires]

More code changes here to address this. Please let me know if this helps!

[jessesquires]

Looks like the layer rasterization provided only some minor improvements. I'm still seeing a bit of stuttering, particularly with data detectors

[dereck009]

Did you have a chance to try TTTAttributedLabel?

[chadpod]

Doing some profiling of my own I kept coming back to boundingRectWithSize: as the main issue for me. I looked into alternatives (Core Text, etc), but seems like this is a particularly troublesome area on iOS and none seemed worth pursuing. There is some caching of the message bubble sizes, but the cache gets cleared pretty regularly in normal chat view usage as to limit its usefulness in my opinion. Instead of caching via indexPath, I'm exploring caching using the message text as the dictionary key, or potentially a hash of the message text, if size becomes an issue. I'm invalidating the cache in shouldInvalidateLayoutForBoundsChange:. You obviously lose caching on media cell types, which I'm not using at this point so I haven't bothered addressing. I'm currently keeping this all local to the flow layout, but am considering keeping external to JSQ.

Just had another idea that seems like it should work generically. Most, if not all of us, probably have a unique identifier for each chat message. Pretty much just gets you around storing arbitrarily long messages as keys, but also makes the solution work for media messages as well.

[ghazel]

@chadpod that looks easy. messageData.text.hash does seem like it would be better than the message text itself. We insert using performBatchUpdates, and that causes invalidateLayoutWithContext to be called for every insert, which wipes out the cache. Switching off indexPath based caching (and removing the cache clearing from invalidateLayoutWithContext and inserting it in shouldInvalidateLayoutForBoundsChange as you said) would fix that, too.

[jessesquires]

@chadpod / @ghazel - That is an excellent idea. Using the message hash as the key. I'll see how this works out.

[jessesquires]

Hello all - #584 and #322 have been resolved. Scrolling feels slightly more improved. Please pull the latest from develop and let me know how things go on your end.

[jessesquires]

All - I just implemented better caching for bubble sizes using message item hashes and NSCache.

Please pull from develop and let me know how this works for you.

[gblazex]

It's pretty bad for me (iPhone 5S). I think we need to try TTTAttributedLabel.

It's not a constant low FPS scrolling, but big hangs like GC pauses out of nowhere (obviously not because of GC, but they look like it).

[LeoNatan]

Unfortunately, no change on 5S or 6 Plus. It's bad. The lack of smooth scroll is jarring.

[LeoNatan]

I ran the following experiment - I changed the <textView>...</textView> to <label>...</label> in the two xib files, changed the outlet to UILabel, commented out some code that was not found for text views and set the number of lines to 0 on the labels. I must say, there is no noticeable performance increase, so hoping TTTAttributedLabel will fix the issue is false hope I fear.

[LeoNatan]

Removing auto layout from the two xibs (incoming + outgoing), albeit without any other attempts to layout, makes scrolling butter smooth.

[jessesquires]

@LeoNatan / @galambalazs - I think scrolling have been greatly improved since this issue was first opened. (But still not perfect.) Can either of you post a demo project that shows the problems you're having?

I'm mostly testing on a 5S and iPad4 and it's not that bad.