Prioritize thread pause()/resume() calls #10174
Conversation
RunLoop already has a queue, and has the ability to schedule weak mailboxes, so we can remove the duplicate code.
kkaefer
changed the title
|
Improves the timeout between the pause() call and actually pausing write operations, as per #10174. |
kkaefer
force-pushed
the
prioritize-thread-pause-resume
branch
from
5ab0102
to
e16c97c
Compare
Previously we had the capability to pause on the Thread object, which used regular tasks to pause the RunLoop by blocking it. Instead, we can now pause the entire RunLoop and prevent it from processing items. This means that a pause() call is no longer treated as a regular task. Instead, it will take precedence over scheduled tasks, which means that a pause() call takes effect much more instantly since the RunLoop doesn't process the queue before the pause() task. Having pause() take effect much quicker is useful for situations where stopping the loop quickly is important, e.g. when the application goes to the background on iOS, and we have to stop processing tasks that access the file system. It also reduces the length of the blocking pause() call since the time until the RunLoop is paused is shortened.
kkaefer
force-pushed
the
prioritize-thread-pause-resume
branch
from
e16c97c
to
329282f
Compare
There was a problem hiding this comment.
Is it necessary to support pause/resume from arbitrary threads? I shudder at trying to reason about pauses and resumes from multiple threads racing against each other. Allowing only the creating thread to pause/resume the created thread is much easier to understand. Could we preserve this restriction?
The previous implementation guaranteed that no activity could happen on the RunLoop (because it would be blocked inside process). This implementation guarantees that process won't do any work, but it doesn't guarantee that there is no other work happening on the underlying event loop. For example, I believe that events related to the addWatch/removeWatch methods in the default implementation will still be dispatched on a "paused" RunLoop. Is that the desired behavior?
| loop.run(); | ||
| } | ||
|
|
||
| TEST(Thread, TestImmediatePause) { |
There was a problem hiding this comment.
This looks like a timing-dependent test, which almost inevitably seem to fail intermittently.
include/mbgl/util/thread.hpp
Outdated
| lock.unlock(); | ||
|
|
||
| Mailbox::maybeReceive(mailbox); | ||
| loop->schedule(mailbox); |
|
Closing in favor of #11005 |
Previously we had the capability to pause on the Thread object, which used regular tasks to pause the RunLoop by blocking it. Instead, we can now pause the entire RunLoop and prevent it from processing items. This means that a pause() call is no longer treated as a regular task. Instead, it will take precedence over scheduled tasks, which means that a pause() call takes effect much more instantly since the RunLoop doesn't process the queue before the pause() task. Having pause() take effect much quicker is useful for situations where stopping the loop quickly is important, e.g. when the application goes to the background on iOS, and we have to stop processing tasks that access the file system. It also reduces the length of the blocking pause() call since the time until the RunLoop is paused is shortened.