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

Performance when push to ChatViewController not good #51

Closed
hhoangna opened this issue May 28, 2023 · 12 comments
Closed

Performance when push to ChatViewController not good #51

hhoangna opened this issue May 28, 2023 · 12 comments
Assignees
Labels
help wanted Extra attention is needed invalid This doesn't seem right

Comments

@hhoangna
Copy link

Hi @ekazaev!
I used to apply ChatLayout to my ChatViewController. When push from list conversations to ChatViewController screen, i think i have a problem. It seem a little slow to show messages in Chat screen. I have run Time Profile by Instruments. I don't know why there functions and classes get more time.

RPReplay_Final1685277349.MP4
image image
@ekazaev
Copy link
Owner

ekazaev commented May 31, 2023

Hi @hhoangna

In general there is nothing wrong with those functions. You are actually initialising a lot of views in one go. Potentially your ChatReplyCell worth having a look though. It seems to be the heaviest one. But there is nothing to do with the library itself.

I see you also have a flickering when chat view is being pushed to the UINavigationController - if you want to get rid of it - you need to setup collection first time in the first call of viewDidLayoutSubviews

@ekazaev ekazaev self-assigned this May 31, 2023
@ekazaev ekazaev added the help wanted Extra attention is needed label May 31, 2023
@hhoangna
Copy link
Author

I try to setup collection in viewDidLayoutSubviews but nothing change :(
I know that a lot of views initial but i think they are nessessory.

@ekazaev
Copy link
Owner

ekazaev commented Jun 1, 2023

@hhoangna Lets first try to understand straight what are you doing. Are you trying to fix some performance issue related to the amount of the views you are creating or the visual glitch that the chat doesn’t appear immediately when the chat controller is being pushed into navigation controller and blame it on performance?

  1. If you are trying to solve the visual glitch - then I can point you to the core of the problem. You are trying to update the collection view while it is being in the Core Animation transaction initiated by the animated push transition into the navigation controller. To avoid it you need to correctly set it in viewDidLayoutSubviews without triggering extra animation curve. To support that theory I can also point that You can also see unexpected animation on your custom navigation title view in the video you attached. It is also caused by the fact that you are updating UI within such animation curve.
    image
    Instruments wont help you there. When UI has performance issues it blocks the main thread and animation while on your video we can see that the animation is smooth. So it is not a performance issue.

  2. On other hand, If you are trying to solve just some performance issue in general - I can not help you there. It is your code, I am not familiar with it. I can only assure you that it is doable as I have seen the apps in production that use ChatLayout without visible performance issues.

So if we are talking about 1 - can you please provide some code what you are doing in viewDidLayoutSubviews so I can examine it? I can’t guarantee that I will be able to help as it can be triggered by some other piece of your app.

@hhoangna
Copy link
Author

hhoangna commented Jun 2, 2023

Yup. Still a lot of issues with my app. Such as animation while push to chat screen. I can't find any code line make that animation.
Can we add contact each other? I will sent you my source code to preview.

@ekazaev
Copy link
Owner

ekazaev commented Jun 7, 2023

@hhoangna I appreciate the honour but Id prefer to avoid such a close interaction unless it is going to be a paid service. I have my own work to do otherwise.

But I assure you that what I said above that what is happening is that you are creating an extra animation loop while the controller is being pushed into the UINavigationController. Check that you propagate the first data within the viewDidLayoutSubviews. If your code is losely based on the Example app check that in this piece at this moment you return true

collectionView.reload(using: changeSet,
                                  interrupt: { changeSet in
                                      guard WHATEVER else {
                                          return true // FIRST TIME YOU MUST RETURN TRUE TO AVOID IT GOING TO performBatchUpdates
                                      }
                                      return false
                                  },
                                  onInterruptedReload: {
                                      // IF YOU RETURN TRUE ABOVE - THIS CODE SHOULD POPULATE THE COLLECTION WITHOUT CREATING AN EXTRA ANIMATION TRANSACTION
                                      let positionSnapshot = ChatLayoutPositionSnapshot(indexPath: IndexPath(item: 0, section: sections.count - 1), kind: .footer, edge: .bottom)
                                      self.collectionView.reloadData()
                                      // We want so that user on reload appeared at the very bottom of the layout
                                      self.chatLayout.restoreContentOffset(with: positionSnapshot)
                                  },

@hhoangna
Copy link
Author

hhoangna commented Jun 7, 2023

I have fixed animation when push Navigation Controller. It's a animation default of system when push from tabbarController.selectedViewController.navigationController.push(vc, animation: true), then have no animation now.

The first load data in my code is same as your Example
collectionView.reload(using: changeSet, interrupt: { changeSet in if shouldInterrupt { return true } else { if self.currentControllerActions.options.contains(.loadingInitialMessages) { guard changeSet.sectionInserted.isEmpty else { return true } return false } else { return false } } }

Nothing change, i think all problem is from initialing more subviews in custom chat cell

@ekazaev
Copy link
Owner

ekazaev commented Jun 8, 2023

@hhoangna Can you put a breakpoint inside of onInterruptedReload and make sure that when view controller is just starts to be pushed into a UINavigationController?
As I said. YOu are initialising views in the main thread. So it can create a delay the ui appearance but not make ui to flicker. Your problem is the animation transition.

@ekazaev
Copy link
Owner

ekazaev commented Jun 8, 2023

@hhoangna

Here is quick and dirty demo from the example app. As you can see it is perfectly being pushed into the navigation controller without any delays of flickering.
push

As I said you need to populate collection first time from the viewDidLayoutSubviews.

But here is another trick. I am not sure if you noticed but! loadInitialMessages in the Example app is an asynchronous function for the demo. It mean that it will return messages at anytime during the animation but not immediately. Such function doesn't suit you for the initial population - you need to create one that will work synchronously for you and return cells at the begingin! of the animation. So such function should look like func getOriginalMessages() -> [Section]. After obtaining the original messages within the same runloop you must update the collection and you wont see flickering when the chat view controller is being pushed. As I said before - you are looking for the problem in the wrong place.

@ekazaev ekazaev added the invalid This doesn't seem right label Jun 8, 2023
@hhoangna
Copy link
Author

hhoangna commented Jun 9, 2023

Thanks you so much. I'll try

@hhoangna hhoangna closed this as completed Jun 9, 2023
@ekazaev
Copy link
Owner

ekazaev commented Jun 9, 2023

@hhoangna Good luck

@hhoangna
Copy link
Author

Hi there :D
Sorry because of reopen this issue

I had try create new synchronously function to get data and binding to collection view. And put them in viewDidLayoutSubviews. It's working, flickering was removed. But it's seem to get more time before navigation push to chat screen (about ~ 500ms)
And they have weird animation like video below:

RPReplay_Final1686675933.MOV

I don't know why. Then i crop a part of my project source code to sent to you. This can not run. Please review code and give me some advices. Thanks you so much
Chat.zip

@hhoangna hhoangna reopened this Jun 13, 2023
@ekazaev
Copy link
Owner

ekazaev commented Jun 14, 2023

@hhoangna I will have a look later at your code. But I still have the feeling that there is some unexpected animation happens during the push. Check it out.

But I can tell you where is the delay can happen.
First place that can cause the delay - measure your synchronious function how long does it take to convert your data model to cell model.
Second, potentially you have heavy cells structure, unoptimised image loading and so on and because there is quite a few of them on the screens - their structure may require optimisation.

As you can see on the gif I provided - there is no delay.

This is the case that you need to check in the Instruments. This thing you need to keep in mind that there is nothing to do with the ChatLayout itself. It is already optimised. The delay comes from your code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants