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

Fix memory leak caused by NSLocalizedString #891

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fthdgn
Copy link

@fthdgn fthdgn commented Apr 2, 2024

I detected very high memory usage on some screens of my app. I investigated the problem and found that the problem is NSLocalizedString(_:tableName:bundle:value:comment:) calls on R.swift.

This PR replaces NSLocalizedString(_:tableName:bundle:value:comment:) calls with String.init(localized:table:bundle:locale:comment:) on supported platforms.

Benchmarks

Steps

  • On Xcode 15.3, create a new iOS app (Storyboard/Swift)
  • Create MyStrings.string Strings File (Legacy)
  • Edit content of MyStrings.string to my_key = "My Text";
  • Edit content of ViewController.swift to code below.
  • Integrate R.swift with SPM according to documentation
  • Modify comments on ViewController.swift and SPM package repo/commit to enable different scenarios
  • Run the simulator on iPhone 15 iOS 17.4 simulator until timers stopped (30 seconds)
  • Monitor the memory usage on Memory Report
import UIKit

class ViewController: UIViewController {
    private let label: UILabel = .init()

    private var timer: Timer?
    private var stopTimer: Timer?

    override func viewDidLoad() {
        super.viewDidLoad()
        label.translatesAutoresizingMaskIntoConstraints = false
        view.addSubview(label)
        label.centerXAnchor.constraint(equalTo: view.centerXAnchor).isActive = true
        label.centerYAnchor.constraint(equalTo: view.centerYAnchor).isActive = true

        timer = .scheduledTimer(withTimeInterval: 0.001, repeats: true, block: { [weak self] _ in
            let date = Date.now.ISO8601Format(.init(includingFractionalSeconds: true))
                        
            //let myValue = R.string.myStrings.my_key()
            
            //let myValue = NSLocalizedString("my_key", tableName: "MyStrings", comment: "")
            
            let myValue = String(localized: .init("my_key"), table: "MyStrings")
           
            self?.label.text = myValue + ": " + date
        })
        stopTimer = .scheduledTimer(withTimeInterval: 30, repeats: false, block: { [weak self] _ in
            self?.timer?.invalidate()
        })
    }
}

Results

  • NSLocalizedString(_:tableName:bundle:value:comment:)
    High is 52.5 MB. It was rising steadily until the timer is stopped.

  • String.init(localized:table:bundle:locale:comment:)
    High is 30.5 MB. It was 30 MB from the start to the end.

  • R.string.myStrings.my_key() for mac-cain13/R.swift master 8d26021c6c71a513505e722f2cc82a6ad4f7f087
    High is 57.3 MB. It was rising steadily until the timer is stopped.

  • R.string.myStrings.my_key() for fthdgn/R.swift nslocalizedstring-memory-leak 9f8f893d35f1d21e64abb8d991243099e3d5cdf0
    High is 30.4 MB. It was around 30 MB from the start to the end.

SwiftUI

import SwiftUI

struct ContentView: View {
    @State private var currentDate = Date.now
    let timer = Timer.publish(every: 0.001, on: .main, in: .common).autoconnect()
    
    var body: some View {
        VStack {
            Text("my_key", tableName: "MyStrings")
            //Text(NSLocalizedString("my_key", tableName: "MyStrings", comment: ""))
            //Text(R.string.myStrings.my_key)
            Text(currentDate.ISO8601Format(.init(includingFractionalSeconds: true)))
        }
        .padding()
        .onReceive(timer) { input in
            currentDate = input
        }
    }
}

SwiftUI has similiar results.

  • Text.init(_:tableName:bundle:comment:) uses around 24 MB.
  • Text(_:) with input NSLocalizedString(_:tableName:bundle:value:comment:)'s memory usage increases steadily.
  • Text(_:StringResource) from R.swift on master branch behaves like NSLocalizedString and memory usage keeps increasing.
  • Text(_:StringResource) from R.swift on this PR branch uses around 24 MB

@tomlokhorst
Copy link
Collaborator

Thanks, nice find!

Looking at the diff, it appears the default value behaviour differs from the original and the two new branches.

@fthdgn
Copy link
Author

fthdgn commented Apr 2, 2024

String.init one does not have any value parameter.
I am not sure purpose of that parameter. On my cases it unused. I will try to find a case where its value is essantial.

@fthdgn fthdgn force-pushed the nslocalizedstring-memory-leak branch from 9f8f893 to b590a5c Compare April 2, 2024 22:35
@fthdgn
Copy link
Author

fthdgn commented Apr 2, 2024

I found the usa case of value parameter.
It is used when a language file does not provide translation for a key.

I used String.init(localized:defaultValue:table:bundle:locale:comment:) function.

Repository owner deleted a comment from SaravanakumarNatarajan2931 Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants