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

Activity Indicator: Remove branching of SwiftUI views to improve performance and readability #1957

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

imthath-m
Copy link
Contributor

@imthath-m imthath-m commented Jan 26, 2024

Platforms Impacted

  • iOS
  • macOS

Description of changes

Using conditional branching in SwiftUI views leads to performance issues as explained here in this WWDC video.

In ActivityIndicator, the View.modifyIf method is used multiple times which results in conditional branching of SwiftUI views. They cause performance issues and are not so easy to read. They can be easily avoided as shown in the changes.

Ideally, I would prefer that modifyIf extension is removed and condition branching of SwiftUI views is done explicitly so that developers can be easily made aware of its performance issues.

Binary change

Total increase: 0 bytes
Total decrease: -42,168 bytes

File Before After Delta
Total 3,11,48,816 bytes 3,11,06,648 bytes 🎉 -42,168 bytes
Full breakdown
File Before After Delta
__.SYMDEF 48,86,280 bytes 48,77,304 bytes 🎉 -8,976 bytes
SwiftUI+ViewModifiers.o 1,36,096 bytes 1,23,656 bytes 🎉 -12,440 bytes
ActivityIndicator.o 1,76,688 bytes 1,55,936 bytes 🎉 -20,752 bytes

Verification

Tested the demo app to ensure to no change in the ActivityIndicator's behaviour. It still appears and runs the same.

Visual Verification
Before After
Screenshot or description before this change Screenshot or description with this change

Pull request checklist

This PR has considered:

  • Light and Dark appearances
  • iOS supported versions (all major versions greater than or equal current target deployment version)
  • VoiceOver and Keyboard Accessibility
  • Internationalization and Right to Left layouts
  • Different resolutions (1x, 2x, 3x)
  • Size classes and window sizes (iPhone vs iPad, notched devices, multitasking, different window sizes, etc)
  • iPad Pointer interaction
  • SwiftUI consumption (validation or new demo scenarios needed)
  • Objective-C exposure (provide it only if needed)
Microsoft Reviewers: Open in CodeFlow

@imthath-m imthath-m requested a review from a team as a code owner January 26, 2024 10:52
@imthath-m
Copy link
Contributor Author

@microsoft-github-policy-service agree

Copy link
Contributor

@mischreiber mischreiber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a clean upgrade, and my testing bears it out as well. Thanks!

@imthath-m imthath-m force-pushed the acitivity-indicator branch from d53f0b6 to f761a3d Compare February 1, 2024 06:21
@imthath-m
Copy link
Contributor Author

Thanks for approving the changes.

Can you approve this workflow as well?
CleanShot 2024-02-01 at 12 06 39@2x

@mischreiber mischreiber merged commit 4669412 into microsoft:main Feb 1, 2024
6 checks passed
@mischreiber mischreiber mentioned this pull request Mar 6, 2024
12 tasks
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