Custom UICollectionViewCell and memory leak when using UILabel

Question

I have a custom UICollectionViewCell. When scrolling left and right (it is a horizontal UICollectionView, memory usage increases by 0.2MB each time.

I believe I am correctly implementing prepareForReuse() in the cell object; within it I remove all subviews of the cell.

With didSet of an object on my collection view cell, I call setupViews() within my cell. I add a UIImageView with constraints and add it as a subview. This is fine.

However, when I use a UILabel(), this is when there seems to be a memory leak. When I look in Instruments, I can see: VM: UILabel (CALayer) is repeatedly created every time I scroll between two cells! This does not happen with the UIImageView.

Just in case it's relevant, here's my prepareForReuse method in my cell:

override func prepareForReuse() {
    super.prepareForReuse()

    self.moreButtonDelegate = nil

    for subview in subviews {
        subview.removeConstraints(subview.constraints)
        subview.removeFromSuperview()
    }

    self.removeFromSuperview() // BURN EVERYTHING
}

Here's my code:

private func setupViews() -> Void {
    let imageView = myImageView // A lazy class property returns this

    innerView.addSubview(imageView) // innerView is just another UIView within this cell

    // Now I add constraints for imageView
}

So with the above, there's no memory leak. Looks like ARC cleans everything up correctly as, even with an image, memory usage does not increase exponentially.

However, when I add this below imageView...

let address = UILabel()
address.translatesAutoresizingMaskIntoConstraints = false
address.text = "TEST"
address.font = UIFont.systemFont(ofSize: 22)
address.adjustsFontSizeToFitWidth = true

// Then I add constraints

I get a new VM: UILabel (CALayer) row appearing on every scroll between cells, and as a result memory usage jumps. Take a look:

What am I doing wrong? I'm using Xcode 9, iOS 11.2 simulator.

Answer 1

I'm not sure this will resolve your particular issue, but I believe you misunderstood prepareForReuse and I think there is a good chance that this might be what's wrong with your code. So lets take a look at your implementation:

override func prepareForReuse() {
    super.prepareForReuse()

    self.moreButtonDelegate = nil

    for subview in subviews {
        subview.removeConstraints(subview.constraints)
        subview.removeFromSuperview()
    }

    self.removeFromSuperview() // BURN EVERYTHING
}

I believe you are looking at the prepareForReuse totally wrong. The main point of the reuse is to reduce the overhead caused by creating the view of the cell (objects instantiation, creating the view hierarchy, layout, etc.). You don't want to burn everything! Instead, you want to keep as much of the contentView as possible. Ideally, you will change just the contents of the views (i.e.: text in UILabel, image in UIImageView, etc.), or maybe some properties (backgroundColor, etc.).

You can use prepareForReuse to cancel some heavyweight operations that you started to present the cell, but that might not have ended when the cell was removed from view and is supposed to be reused somewhere else. E.g., when you download content from web, user might scroll fast, and the cell goes away from the screen before the web image was downloaded and presented. Now if the cell gets reused, the old downloaded image will be most probably shown - so in prepareForReuse you can cancel this operation.

Conclusion - I believe in your case none of the operations you are doing in prepareForReuse really help - vice versa, because of that the collection view will have to again recreate the whole UI of the cell again from scratch (that means all the overhead of object instantiation, etc.). My first advice for you is to drop the whole prepareForReuse implementation.

Second, once you drop your prepareForReuse implementation, refactor the cell so that it will create the UI only once, ideally in its initializer:

class UI: UITableViewCell {
    override init(style: UITableViewCellStyle, reuseIdentifier: String?) {
        super.init(style: style, reuseIdentifier: reuseIdentifier)

        setupViews()
    }
}

Then, in cellForItemAt just configure its contents, that means set texts for labels, images for image views, etc.

In the end, keep in mind what the documentation says about it (my own emphasis):

Performs any clean up necessary to prepare the view for use again.

Do only what is really necessary to do, not a thing more.

Over the last year I have implemented many tableView and collectionView datasources, but I really needed to use prepareForReuse just twice (for the example with image download I mentioned above).

EDIT

Example of what I meant:

struct Model {
    var name: String = ""
}

class CustomCell: UITableViewCell {
    // create it once
    private let nameLabel = UILabel()

    override init(style: UITableViewCellStyle, reuseIdentifier: String?) {
        super.init(style: style, reuseIdentifier: reuseIdentifier)

        // setup view once
        setupViews()
    }

    required init?(coder aDecoder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }

    private func setupViews() {
        // add it to view
        self.contentView.addSubview(nameLabel)
        // setup configuration
        nameLabel.textColor = UIColor.red

        // lay it out
        nameLabel.translatesAutoresizingMaskIntoConstraints = false
        NSLayoutConstraint.activate([
            nameLabel.topAnchor.constraint(equalTo: self.contentView.topAnchor, constant: 8),
            nameLabel.bottomAnchor.constraint(equalTo: self.contentView.bottomAnchor, constant: -8),
            nameLabel.leftAnchor.constraint(equalTo: self.contentView.leftAnchor, constant: 8),
            nameLabel.rightAnchor.constraint(equalTo: self.contentView.rightAnchor, constant: -8),
            ])
    }

    // this is what you call in cellForRowAt
    func configure(for model: Model) {
        nameLabel.text = model.name
        // someImageView.image = model.image
        // etc.
    }

    override func prepareForReuse() {
        super.prepareForReuse()
        // if it is super important, reset the content, cancel operations, etc., but there is no reason to recreate the UI

        // so e.g. this might be ok (although in this case completely unnecessary):
        nameLabel.text = nil

        // but you definitely don't want to do this (that's done once at the cell initialization):
        // nameLabel = UILabel()
        // setupViews()
    }
}

class CustomTableViewController: UITableViewController {

    var models: [Model] = [Model(name: "Milan")]

    override func viewDidLoad() {
        super.viewDidLoad()
        tableView.register(CustomCell.self, forCellReuseIdentifier: "customCell")
    }

    override func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
        return models.count
    }

    override func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        let cell = tableView.dequeueReusableCell(withIdentifier: "customCell", for: indexPath) as! CustomCell
        // you just want to set the contents, not to recreate the UI components
        cell.configure(for: models[indexPath.row])
        return cell
    }
}

Moreover, always work with cell's contentView, not directly with the cell. Notice that I used this:

self.contentView.addSubview(nameLabel)

instead of this:

self.addSubview(nameLabel)

Official docs:

The content view of a UITableViewCell object is the default superview for content displayed by the cell. If you want to customize cells by simply adding additional views, you should add them to the content view so they will be positioned appropriately as the cell transitions into and out of editing mode.