[Perf improvement] Fixing Content.Medium creation for the unwrapping of custom modifiers.

[rafael-assis]

Hi @nalexn.
We found the issue described below as we continued working on profiling performance bottlenecks in ViewInspectorand their effects in our test codebase at Airbnb.

It's a simple fix and we'd love to get your thoughts on it.
Thank you so much again for collaborating with us! 🙏

The problem: Content.Medium.appending() funcs are called an excessive number of times

After #273 was merged, we noticed that we had a few tests that were still taking a long time to run.
We profiled and traced those tests in Instruments again and noticed that the methods that add environmentModifiers and environmentObjects to the medium (and their callers) were responsible for most of the running time of those tests.

• Content.Medium.appending(environmentModifier:)
• Content.Medium.appending(environmentObject:)

We then tracked the number of environmentModifiers and environmentObjects contained in the medium property of the InspectableView's content, and noticed a disproportional number of items in those arrays compared to the number of custom modifiers applied to our test view.

As an example, consider the following test:

struct AccessibilityEnabledEnvironmentModifier: ViewModifier {

    func body(content: Self.Content) -> some View {
        content
            .environment(\.accessibilityEnabled, true)
    }
}

final class ModifiedContentTests: XCTestCase {
    func testMultipleCustomEnvironmentModifiers() throws {
        let view = Text("str")
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())
            .modifier(AccessibilityEnabledEnvironmentModifier())

        let sut = try view.inspect().text()
        XCTAssertEqual(sut.content.medium.environmentModifiers.count, 15)
    }
}

❌ The assertion on the test failed as the environmentModifiers.count that should be 15 turns out to be 32767.

In further experiments, we concluded that the Big O notation that represents the complexity of the test increased from the expected linear time O(n) to O(2ⁿ). (where n is the number of custom environment modifiers applied to the Text("str") View.

We noticed that the issue does not happen when the .environment() modifier call is applied directly to the view:

func testMultipleEnvironmentModifiers() throws {
        let view = Text("str")
            .environment(\.font, .headline)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)
            .environment(\.accessibilityEnabled, true)

        let sut = try view.inspect().text()
        XCTAssertEqual(sut.content.medium.environmentModifiers.count, 15)
    }

✅ The test above passes.

The fix: Making custom modifier logic linear as the unwrapping traversal

The cause of the issue is that the code that handles the unwrapping of custom modifiers in func unwrappedModifiedContent() duplicates all the properties that were previously passed by the unwrapping of its container View.

The duplication happens because the items in the medium that were originally used to extract the current modifier content view are also returned in the InspectableView's Content.Medium by the find call.

The same items will then be appended again to the original medium that will be passed as a parameter to the recursive call that unwraps the modifier's child content.

The fix consists of simply recreating the medium with the 3 properties (transitiveModifiers,environmentModifiers and environmentObject) reset to the values of the Content.Medium in the InspectableView (viewModifierContent variable) returned by the find call.

Results in tests for our production code

We could see dramatic improvements in running time of the tests that flagged this issue in our CI environment:

Test ID	Before fix (seconds)	After fix (seconds)
Test 1	1.9	1.142
Test 2	6.622	5.653
Test 3	115.327	0.929
Test 4	78.488	2.29
Test 5	83.076	3.538
Test 6	107.458	0.885
Test 7	46.893	0.638

Testing

We are adding 6 test cases to validate the application of custom modifiers and SwiftUI built-in modifiers in scenarios where the 3 properties of the Content.Medium struct are affected by this issue.

Content.Medium property	SwiftUI built-in modifier	custom modifier
transitiveModifiers	testMultipleTransitiveModifiers	testMultipleCustomTransitiveModifiers
environmentModifiers	testMultipleEnvironmentModifiers	testMultipleCustomEnvironmentModifiers
environmentObjects	testMultipleEnvironmentObjects	testMultipleCustomEnvironmentObjectModifiers

The tests to validate the direct application of the built-in SwiftUI modifiers were added only for comparison purposes. They show the correct behavior and are used as a reference for the correct results of the custom modifier application scenarios that were fixed.

Please review:

@nalexn
@bachand

[rafael-assis]

This test fails this assertion in the state of f52f300 (the first commit of this branch).

The transitiveViewModifiers.count returns 32767 items.

[rafael-assis]

This test fails this assertion in the state of f52f300 (the first commit of this branch).

The environmentModifiers.count returns 32767 items.

[rafael-assis]

This test fails this assertion in the state of f52f300 (the first commit of this branch).

The environmentObjects.count returns 32767 items.

[bachand]

Thank you @rafael-assis for the rigorous investigation, detailed description, and targeted fix 💪 We are excited to integrate this fix internally!

[bachand]

👌

[nalexn]

Great work here, much appreciated!

[bachand]

Thank you for building such a useful library, @nalexn ! We are happy to contribute.

[rafael-assis]

Great work here, much appreciated!

We appreciate your collaboration @nalexn!
Happy new year! 🎆