twostraws / ControlRoom

A macOS app to control the Xcode Simulator.
MIT License
5.75k stars 306 forks source link

Entering text in the Operator text field is slow #107

Closed laclouis5 closed 3 years ago

laclouis5 commented 3 years ago

Entering carrier name in 'Operator' text field of a simulator (the default one for me) is slow, around half a second per letter on my Intel MacBook Air 2015.

CPU usage increases to 100 % in Activity Monitor.

Each time you press a key the console logs 2021-01-30 15:59:12.656254+0100 simctl[86698:5943382] Metal API Validation Enabled.

Application compiled in Release Mode.

Profiling with the SwiftUI debugger shows that it triggers many .body recomputations for each typed letter. I feel like the problem is in the Preferences definition which is defined as an ObservedObject observing UserDefaults values. Its .objectWillChange() is triggered if any of its property is updated, which triggers .body recomputations in every view with depends on Preferences, even if those views do not use any Preferences property that changed.

arjun-dureja commented 3 years ago

Same thing is happening on my MacBook Pro 2018, on both 'Operator' and 'Open URL' text fields.

I found a fix for it by creating a separate @State variable in the View for the TextField text and then assigning that to corresponding Preferences var, instead of directly accessing the Preferences var from the TextField.

I'm not sure if this is the best solution to the problem so I'll let someone else chime in before opening a PR.

laclouis5 commented 3 years ago

The issue is with the Preferences definition.

Profiling with the SwiftUI debugger shows that it triggers hundreds of .body recomputations plus the filtering of simulators list (among other things) for each letter typed.

Preferences published property objectWillChange is triggered when any of the Published properties is updated, i.e. when any UserDefault value is updated, thus including the filtered text, the url, the push text, the carrier text, etc...

IMO there is a design flaw here, the properties in Preferences should not be coupled like they are currently because it creates a tight dependency and shared state among views.

One solution could be to break this class apart and only reference the relevant state in views with @AppStorage instead of referencing an entire ObservedObject. Some data is local, e.g. pushPayload and carrierName belong to the NetworkView and should be internal state instead of shared state.

For instance Preferences could be break apart into multiple ObservedObjects such as Preferences, NetworkPreferences, etc, to decouple it from the rest.

twostraws commented 3 years ago

Yes, I agree with your diagnosis and solution – do you want to have a go at this?

laclouis5 commented 3 years ago

Not a SwiftUI expert but I'll make an attempt!