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

change priority of DispatchQueue #2322

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

nnnlog
Copy link
Contributor

@nnnlog nnnlog commented Oct 3, 2024

Connection with issue(s)

Resolve issue #2321

Connected to #2321

Testing and Review Notes

In same environment noted as issue, ssl verifying should be completed fast. (should be complete as much as fast with not affecting main thread)

However, it might be worthwhile to further discuss whether to change the QoS policy from default to something with a higher priority, such as userInitated.

Screenshots or Videos

To Do

  • double check the original issue to confirm it is fully satisfied
  • add testing notes and screenshots in PR description to help guide reviewers
  • request the "UX" team perform a design review (if/when applicable)

@pichillilorenzo
Copy link
Owner

Thanks, probably this implementation of using .background was caused by my low experience on what DispatchQoS.QoSClass really is.
At that time, probably, I got fooled by the word "background" to fix the "This method should not be called on the main thread as it may lead to UI unresponsiveness" message, thinking the background was only the "right" option.
I don't know about other priorities. Having it higher, could maybe lead to UI unresponsiveness? Have you tried the other priorities?

In the meantime, I can merge this.

@pichillilorenzo pichillilorenzo merged commit ccfaa36 into pichillilorenzo:master Oct 23, 2024
@pichillilorenzo
Copy link
Owner

@all-contributors please add @nnnlog for code

Copy link
Contributor

@pichillilorenzo

I've put up a pull request to add @nnnlog! 🎉

@nnnlog
Copy link
Contributor Author

nnnlog commented Oct 24, 2024

Below are higher priorities than default of DispatchQos:

  • userInteractive : The quality-of-service class for user-interactive tasks, such as animations, event handling, or updating your app's user interface.
  • userInitiated : The quality-of-service class for tasks that prevent the user from actively using your app.

According to definition of each priority, the heavy task such as verifying ssl certificate should not be executed in userInteractive. So, I'll test about when applying userInteractive and default priority, and check the difference between two priorities.

@pichillilorenzo
Copy link
Owner

According to definition of each priority, the heavy task such as verifying ssl certificate should not be executed in userInteractive. So, I'll test about when applying userInteractive and default priority, and check the difference between two priorities.

Ok, thanks for the help!

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