nalexn / ViewInspector

Runtime introspection and unit testing of SwiftUI views
MIT License
2.09k stars 145 forks source link

[iOS 17 perf degradation fix] Changing sanitizingNamespace implementation to use regular expression pattern matching. #263

Closed rafael-assis closed 9 months ago

rafael-assis commented 9 months ago

Hi @nalexn. we have identified this iOS 17.0 issue here at Airbnb and we'd love to get your input. I believe we have a good solution (described below). Let myself and @bachand know your thoughts on it. Thank you! 🙏

Context: performance degradation on iOS 17.0

On our validation exercises to adopt Xcode 15 in the Airbnb development environment, we noticed a significant degradation in performance when running tests that used ViewInspector.

We identified that that degradation only happened when running against an iOS 17.0 simulator. We measured the multiple sets of tests running from Xcode 15 and comparing results between 2 iPhone 14 simulators, one running iOS 16.4 the other running iOS 17.0. Overall we could see same sets of tests running against iOS 17.0 would take around 3x longer compared to the runs against iOS 16.4.

Measuring execution times of the full call stack we noticed that the logic of replacing strings like ".(unknown context at $1541f4030)" from the type name in sanitizingNamespace() was one of the top offenders. In large hierarchies and tests that heavily relied on find() and findAll() calls, sanitizingNamespace() could get called more than 30,000 times for a single run.

In those particular scenarios, the aggregated time spent on the all executions of sanitizingNamespace()got close to 8x slower when running on iOS 17.0 compared to the same runs on iOS 16.4.

Changes

We have experimented a different approach for the logic that replaces those ".(unknown context at " strings from the namespaces that uses pattern matching with the NSRegularExpression type.

The results on our test runs with against larger view hierarchies showed that this new implementation has similar run times on iOS 16.4 as the previous implementations. On iOS 17.0 it improved dramatically by matching the times of iOS 16.4 and some times even showing slightly better performance on these measured run times.

Testing

The full set of ViewInspector tests was executed before and after the fix in both iOS 16.4 and iOS 17.0. The scenario for those tests is pretty lightweight, which made the tests run times vary a little, but no degradation was detected in either iOS versions:

Before After
iOS 16.4 4.486 seconds ViewInspector_iOS16_4_before 4.608 seconds ViewInspector_iOS16_4_after
iOS 17.0 5.336 seconds ViewInspector_iOS17_0_before 4.929 seconds ViewInspector_iOS17_0_after

Review

@bachand @nalexn

nalexn commented 9 months ago

Many thanks @rafael-assis . Merged

rafael-assis commented 9 months ago

Many thanks @rafael-assis . Merged

Great! Thank you so much for merging it so quickly @nalexn! 🙏 We'll keep experimenting in our codebase and let you know if we can find other perf issues (and fixes for them).

bachand commented 9 months ago

Thank you @rafael-assis and @nalexn !