[Perf improvement] Implementing memoization for sanitizeNamespace and replaceGenericParameters funcs.

[rafael-assis]

Hi @nalexn. Picking up were we left off on #263, we have identified another opportunity for performance improvements in ViewInspector. @bachand and I discussed it and we'd love to get your thoughts on it. Thank you so much for your time! 🙏

Context: performance improvements under larger SwiftUI View hierarchies

We have profiled and traced some of our slowest tests in Instruments showed that the 2 funcs defined in the String extension were the top offenders in terms of running time.

• String.replacingGenericParameters(_:)
• String.sanitizingNamespace()

The combined time of all calls to both of these functions accounted for almost 50% of the entire test running time:	Weight(s)	Weight(%)	Self Weight	Symbol Name
28.26 s	100.0%	0 s	UnitTestHost (16171)
8.08 s	28.5%	8.08 s	String.replacingGenericParameters(_:)
5.80 s	20.5%	5.80 s	String.sanitizingNamespace()

It was already known that these 2 functions are called thousands of times for a single test case in our codebase. This fact motivated an investigation on how many times these 2 funcs are called for the exact same input in order to inform a potential decision to memoize the calls.

A histogram of the calls to sanitizingNamespace for each input was built from a single feature integration test run. We found that for some types, the func sanitizingNamespace thousands of times for them. For example:

    - key : "SwiftUI.Environment<Swift.Optional<Swift.AnyHashable>>"
    - value : 3640

    - key : "SwiftUICoreUI.ScrollEventDispatch"
    - value : 144817

    - key : "Swift.Bool"
    - value : 3906

    - key : "SwiftUI.Environment<PrimitiveTypesCoreUI.AdaptiveTraitCollection>"
    - value : 19052

    - key : "(PrimitiveTypesCoreUI.InteractivityState) -> SwiftUI.ModifiedContent<SwiftUI._ViewModifier_Content<PrimitiveTypesCoreUI.Interactive<PrimitiveTypesCoreUI.TextStyleViewModifier>>, PrimitiveTypesCoreUI.TextStyleViewModifier>"
    - value : 17912

Changes

It was then clear that caching the output of these expensive string manipulation operations would be worth it. This is exactly what theses changes do. It's a simple memoization which stores the computed value to be returned directly instead of recalculating them repeatedly.

Testing

• Ran tests at Airbnb that use ViewInspector

Local runs of the the test used to profile ViewInspector's execution time showed a substantial improvement of running time. During the 2 Unit Tests CI runs of the changes in this PR, we noticed an improvement of about 86% (19 seconds). That particular test took ~22 seconds to run before the optimizations and ~3 seconds after the optimizations:

Although the unit tests in ViewInspector are more lightweight compared to the tests we run at Airbnb, we could also see a good improvement of the overall running time of all tests of the library:

Before	After

Please review:

@nalexn @michael-bachand

[nalexn]

That is a substantial performance improvement, good job! Before merging this in, I suggest an additional slight refactor of sanitizeNamespace in this fasion. The part where it removes "(extension in ...)" isn't yet confirmed to be helpful, but it'd be best to make sure your final version supports adding more patterns for prefix removal

[rafael-assis]

That is a substantial performance improvement, good job! Before merging this in, I suggest an additional slight refactor of sanitizeNamespace in this fasion. The part where it removes "(extension in ...)" isn't yet confirmed to be helpful, but it'd be best to make sure your final version supports adding more patterns for prefix removal

Thank you for the super quick turnaround, @nalexn! 🙏 I have updated sanitizeNamespace to support multiple patterns as you suggested.

I left the new pattern disabled for now with a comment. Let me know what you think of the approach.

[rafael-assis]

That is a substantial performance improvement, good job! Before merging this in, I suggest an additional slight refactor of sanitizeNamespace in https://github.com/nalexn/ViewInspector/issues/268#issuecomment-1806104270. The part where it removes "(extension in ...)" isn't yet confirmed to be helpful, but it'd be best to make sure your final version supports adding more patterns for prefix removal

Hi @nalexn. Do you think the changes I made fully addressed your suggestions? Let me know if you need me to make any further adjustment before you merge this PR.

Thank you!

[nalexn]

Merged. Thank you @rafael-assis and @bachand !