Page MenuHomePhabricator

Fix priority inversion console warnings in Xcode 14
Closed, DeclinedPublic

Description

Thread Performance Checker: Thread running at QOS_CLASS_USER_INITIATED waiting on a lower QoS thread running at QOS_CLASS_DEFAULT. Investigate ways to avoid priority inversions

This occurs in WMFTaskGroup's wait() method, all calls of which seem to only reside in ReadingListSyncOperation. This should just be a matter of refactoring this code to remove the wait() calls. This area is difficult to follow and risky, so I'm spinning this off as a separate effort from T310091.

Some good documentation about this particular warning is here.

Event Timeline

LGoto triaged this task as Medium priority.Sep 19 2022, 6:34 PM
LGoto moved this task from Needs Triage to Engineering Backlog on the Wikipedia-iOS-App board.
LGoto added a project: ios-app-v7.1.

I'm getting a thread security warning on the WMFMockAppDelegate main function return. I don't think it's the same issue, but it may be worth checking if it still exists when picking up this ticket.

return UIApplicationMain(argc, argv, nil, delegateClass);

This method should not be called on the main thread as it may lead to UI unresponsiveness.

Mazevedo moved this task from Doing to Ready for Development on the ios-app-v7.1 board.

I experimented a bit for this task, but none of the options was side effect free. I decided not to move forward with this work, as I was not sure it could cause unexpected bugs since we still have some issues with the stability of reading lists T316710 and are currently testing sharing reading lists.
I'm listing some of the attempts, so it's documented and hopefully helpful when another engineer or I pick this ticket in the future.

The warning is caused by the taskGroup.wait() calls in a thread other than main. Since we have code that explicitly sends CoreData operations for the background, it's impossible to have them executed on the main thread. Changing this would require a complete overhaul of our current implementation.

  • Swift task groups (ref https://developer.apple.com/documentation/swift/taskgroup): this approach takes advantage of the existing task groups and the possibilities of concurrency on swift. This approach would be a good alternative to the current implementation, as it allows for safely performing the asynchronous tasks. However, a more significant refactoring of the code would be required since the background sync tasks were only sometimes properly executed with my approach of not rewriting large parts of the code.
  • Removing the wait statements resulted in larger syncs (syncing several lists/lists with many items) not fully syncing with no error propagation.
  • Rewriting code to force a synchronous code order: this option also resulted in some inconsistency in the execution of the background tasks. I experimented a bit with this task, but none of the options was side effect free.
LGoto lowered the priority of this task from Medium to Low.Apr 18 2023, 6:46 PM