2

I have a UITableViewCell that has a horizontal UIStackView inside it. Creating and adding in the UIViews to the stack view is causing a bit of a performance issue. There's a definite blip in the scrolling of the table view. I've profiled it in instruments and the call out to update the stack view is the second heaviest in the whole app.

The setup is as follows. I'm using MVP and in my presenter for the cell, I make a call out to updateStackView(with items: [Any]) when I setup the cell, and I setup the cell in the cellForRowAtIndexPath method.

My original updateStackView(with items: [Any]) was as follows:

func updateStackView(_ items: [Any]) {
    items.forEach { item in
        if let customViewModel = item as? CustomViewModel {
            myStackView.addArrangedSubview(CustomView(customViewModel))
        } else if item is Spacing {
            myStackView.addArrangedSubview(PipeSpacerView())
        }
    }
}

So this was the bottleneck, and I decided to move some stuff to the background as follows:

func updateStackView(_ items: [Any]) {
    DispatchQueue.global(qos: .background).async {
        items.forEach { item in
            if let customViewModel = item as? CustomViewModel {
                DispatchQueue.main.async {
                    self.myStackView.addArrangedSubview(CustomView(customViewModel))
                }
            } else if item is Spacing {
                DispatchQueue.main.async {
                    self.myStackView.addArrangedSubview(PipeSpacerView())
                }
            }
        }
    }
}

Kinda ugly as it has some deep nesting, but made a big performance impact.

I was happy with this until I had scrolled down to the bottom of my table view and started scrolling back up. I noticed that some of the CustomViews were in the stack view twice when the cell was being scrolled into view. Once the cell was fully in view the stack view corrected itself most of the time back to the correct number.

In my prepareForReuse I do specifically make a call out to the following function:

func resetDescriptionStackView() {
    descriptionStackView.arrangedSubviews.forEach { $0.removeFromSuperview() }
}

which had always worked before fine was it was all on the main thread.

Any help greatly appreciated. I'm pretty certain it's a threading issue as it's only started to rear its head as I moved stuff to the background.

AtulParmar
  • 4,358
  • 1
  • 24
  • 45
Mr.P
  • 1,390
  • 13
  • 35
  • 1
    Why use stack view at all? Why don't you just put its contents in separate cells? – mag_zbc Feb 07 '19 at 13:08
  • @mag_zbc sorry I wasn’t clear. It’s a horizontal stack view. I’ll update the question – Mr.P Feb 07 '19 at 13:10
  • 1
    Common way to do this is to use a horizontal collection view inside each table cell. – mag_zbc Feb 07 '19 at 13:12
  • i would attempt to NOT do this on the main thread, the main thread should only be used for UI updates. Try making a dispatch queue that is global. https://medium.com/@abhimuralidharan/understanding-threads-in-ios-5b8d7ab16f09 – Julian Silvestri Feb 07 '19 at 13:30

2 Answers2

1

First of all the .background Quality of service using mostly for big/hard background work (ex. send statistic data, sync data base with server, e.t.c), but when you want do some quick work on a different queue than main and then update UI proper way use .userInitiated or .utility. Also I recommend use your own OperationQueue with one of background Qos.
To make code better you can use "defer" statement (documentation) and separate code to small functions.

If you want add subview once you can add special id for your CustomView and Spacing and then check it. Full code:

//For each our CustomView and Spacing we set uniq id to tag property in init of each class.

        /// Operation queue that work with our UIStackView
        fileprivate lazy var stackViewOperationQueue: OperationQueue = {

            let queue = OperationQueue()
            queue.name = "StackView OperationQueue"
            queue.qualityOfService = .userInitiated
            return queue
        }()

        func updateStackView(_ items: [Any]) {

            stackViewOperationQueue.addOperation { [weak self] in
                items.forEach { self?.addArrangedSubviewToStackView(by: $0) }
            }
        }

       fileprivate func addArrangedSubviewToStackView(by item: Any) {

            var handler: () -> () = {}

            defer {
                DispatchQueue.main.async {
                    handler()
                }
            }

            if let customViewModel = item as? CustomViewModel {
                handler = { [weak self] in
                  let viewToAdd = CustomView(customViewModel)
                  self?.checkAndAddSubviewIfNeeded(viewToAdd)}
            }
            else if item is Spacing {
                handler = { [weak self] in
                  self?.checkAndAddSubviewIfNeeded(PipeSpacerView()) }
            }
        }

       func checkAndAddSubviewIfNeeded(_ subviewToAdd: UIView) {

           if !myStackView.arrangedSubviews.contains(where: { view in return view.tag == subviewToAdd.tag }) {
               self.myStackView.addArrangedSubview(subviewToAdd)
           }
       }

Hope it will help!

Ildar.Z
  • 666
  • 1
  • 7
  • 17
0

The problem you have is due to cell reuse by the table view.

Whats happening is that when you scroll down the current visible cells already ran their background task to create the views. By the time they finish the current visible cells have changed to display the current data but they are still the same cell instances used before by the table view. So now two closures will add views to the same stack view instance. (one closure before the cell has been reused and another closure after the cell has been reused)

What you can do is for each cell create a UUID store it in the cell, capture it in the background task closure and test if equal. if not the result of the task is not relevant as the cell has changed to present another visible row.

e.g:

class CustomViewCell: UITableViewCell
{
    private var cellId: String = ""

    // call this method for each cell in cellForItemAt implementation
    func configure(_ items: [Any])
    {
        cellId = UUID() // generates a randomized string

        updateStackView(items)      
    }

    private func updateStackView(_ items: [Any]) {
        let curretCellId = cellId
        DispatchQueue.global(qos: .background).async {
            items.forEach { item in
                if let customViewModel = item as? CustomViewModel {
                    DispatchQueue.main.async {
                        if (self.cellId == currentCellId)
                        {
                            self.myStackView.addArrangedSubview(CustomView(customViewModel))
                        }
                    }
                } else if item is Spacing {
                    DispatchQueue.main.async {
                        if (self.cellId == currentCellId)
                        {
                            self.myStackView.addArrangedSubview(PipeSpacerView())
                        }
                    }
                }
            }
        }
    }
}
giorashc
  • 13,691
  • 3
  • 35
  • 71