Fix 232ms UI hang in call duration timer#836
Open
sampepose wants to merge 1 commit intoGetStream:developfrom
Open
Fix 232ms UI hang in call duration timer#836sampepose wants to merge 1 commit intoGetStream:developfrom
sampepose wants to merge 1 commit intoGetStream:developfrom
Conversation
Contributor
|
Hey @sampepose, thank you for this PR. We are currently in the process of resolving many performance issues (you can take see a sneak peek in the open PRs). We are now in a state where we have resolved all the hungs and for that reason I'm not going to accept this PR. I'm going to keep open though as I reminder to check again once the changes have been merged. Thanks, |
Author
|
Great, I see this was addressed in #825! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Issue Links
N/A - Performance improvement identified through profiling
🎯 Goal
Fix a 232ms UI hang that occurs every second during active calls, caused by expensive Swift runtime protocol conformance checking when accessing
@Publishedproperties in a timer callback.📝 Summary
startedAtvalue to avoid@Publishedproperty accessDispatchSourceTimerinstead ofFoundation.Timer@objc updateDuration()method by calculating duration inline🛠 Implementation
The issue was discovered through Instruments profiling, which showed
swift_dynamicCastand protocol conformance checking taking 232ms when the duration timer accessed the@Published var startedAtproperty.Root cause: Every second, the timer would access
startedAtthrough the@Publishedproperty wrapper, triggering expensive Swift runtime checks for protocol conformances.Solution:
_cachedStartedAtto store the date without going through@PublishedFoundation.Timer(main thread) toDispatchSourceTimer(background queue)durationupdate happens on main threadThis approach eliminates both the protocol conformance overhead AND keeps the timer work off the main thread.
🎨 Showcase
@Publishedaccess in hot path🧪 Manual Testing Notes
☑️ Contributor Checklist