Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2153733002:
[mojo-edk] Prevent AcceptIncomingMessages flushing aggressively (Closed)
|
Created:
4 years, 5 months ago by Ken Rockot(use gerrit already) Modified:
4 years, 5 months ago Reviewers:
Anand Mistry (off Chromium) CC:
Aaron Boodman, abarth-chromium, ben+mojo_chromium.org, chromium-reviews, darin (slow to review), qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[mojo-edk] Prevent AcceptIncomingMessages flushing aggressively
Changes AcceptIncomingMessages to only flush the incoming
message queue once synchronously. This prevents deadlock cycles
that can block incoming external messages by spinning the IO
thread indefinitely. It also prevents similar cycles from
chewing up CPU on other threads.
Also fixes some Watcher tests that are broken by
ProcessIncomingMessages behavior. This change made the
brokenness more apparent.
BUG=628492
R=amistry@chromium.org
Committed: https://crrev.com/5cdb85fdae4f9f9487d67d080b794ca633219303
Cr-Commit-Position: refs/heads/master@{#405776}
Patch Set 1 : . #
Total comments: 2
Patch Set 2 : -_- #
Messages
Total messages: 24 (18 generated)
The CQ bit was checked by rockot@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [mojo-edk] Prevent AcceptIncomingMessages from blocking the IO thread Changes AcceptIncomingMessages to less aggressively pump messages when running on the IO thread. This prevents it from blocking receipt of external messages. This change in turn highlighted some new brokenness in Watcher tests that is due to IO-thread ProcessIncomingMessage posting. Tests have been fixed. BUG=628492 R=amistry@chromium.org ========== to ========== [mojo-edk] Prevent AcceptIncomingMessages from blocking the IO thread Changes AcceptIncomingMessages to less aggressively pump messages when running on the IO thread. This prevents it from blocking receipt of external messages. This change in turn highlighted some new brokenness in Watcher tests that is due to IO-thread ProcessIncomingMessages posting. Tests have been fixed. BUG=628492 R=amistry@chromium.org ==========
The CQ bit was checked by rockot@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #1 (id:1) has been deleted
Patchset #1 (id:1) has been deleted
Description was changed from ========== [mojo-edk] Prevent AcceptIncomingMessages from blocking the IO thread Changes AcceptIncomingMessages to less aggressively pump messages when running on the IO thread. This prevents it from blocking receipt of external messages. This change in turn highlighted some new brokenness in Watcher tests that is due to IO-thread ProcessIncomingMessages posting. Tests have been fixed. BUG=628492 R=amistry@chromium.org ========== to ========== [mojo-edk] Prevent AcceptIncomingMessages flushing aggressively Changes AcceptIncomingMessages to only flush the incoming message queue once synchronously. This prevents deadlock cycles that can block incoming external messages by spinning the IO thread indefinitely. It also prevents similar cycles from chewing up CPU on other threads. BUG=628492 R=amistry@chromium.org ==========
Description was changed from ========== [mojo-edk] Prevent AcceptIncomingMessages flushing aggressively Changes AcceptIncomingMessages to only flush the incoming message queue once synchronously. This prevents deadlock cycles that can block incoming external messages by spinning the IO thread indefinitely. It also prevents similar cycles from chewing up CPU on other threads. BUG=628492 R=amistry@chromium.org ========== to ========== [mojo-edk] Prevent AcceptIncomingMessages flushing aggressively Changes AcceptIncomingMessages to only flush the incoming message queue once synchronously. This prevents deadlock cycles that can block incoming external messages by spinning the IO thread indefinitely. It also prevents similar cycles from chewing up CPU on other threads. Also fixes some Watcher tests that are broken by ProcessIncomingMessages behavior. This CL made the brokenness more apparent. BUG=628492 R=amistry@chromium.org ==========
I suppose this was inevitable
Description was changed from ========== [mojo-edk] Prevent AcceptIncomingMessages flushing aggressively Changes AcceptIncomingMessages to only flush the incoming message queue once synchronously. This prevents deadlock cycles that can block incoming external messages by spinning the IO thread indefinitely. It also prevents similar cycles from chewing up CPU on other threads. Also fixes some Watcher tests that are broken by ProcessIncomingMessages behavior. This CL made the brokenness more apparent. BUG=628492 R=amistry@chromium.org ========== to ========== [mojo-edk] Prevent AcceptIncomingMessages flushing aggressively Changes AcceptIncomingMessages to only flush the incoming message queue once synchronously. This prevents deadlock cycles that can block incoming external messages by spinning the IO thread indefinitely. It also prevents similar cycles from chewing up CPU on other threads. Also fixes some Watcher tests that are broken by ProcessIncomingMessages behavior. This change made the brokenness more apparent. BUG=628492 R=amistry@chromium.org ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
lgtm https://codereview.chromium.org/2153733002/diff/20001/mojo/edk/system/watch_u... File mojo/edk/system/watch_unittest.cc (right): https://codereview.chromium.org/2153733002/diff/20001/mojo/edk/system/watch_u... mojo/edk/system/watch_unittest.cc:141: LOG(ERROR) <<"YES CALLED ."; Remember to remove before submitting.
The CQ bit was checked by rockot@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks https://codereview.chromium.org/2153733002/diff/20001/mojo/edk/system/watch_u... File mojo/edk/system/watch_unittest.cc (right): https://codereview.chromium.org/2153733002/diff/20001/mojo/edk/system/watch_u... mojo/edk/system/watch_unittest.cc:141: LOG(ERROR) <<"YES CALLED ."; On 2016/07/15 at 09:55:04, Anand Mistry wrote: > Remember to remove before submitting. Done
The CQ bit was unchecked by rockot@chromium.org
The CQ bit was checked by rockot@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from amistry@chromium.org Link to the patchset: https://codereview.chromium.org/2153733002/#ps30001 (title: "-_-")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:30001)
Message was sent while issue was closed.
Description was changed from ========== [mojo-edk] Prevent AcceptIncomingMessages flushing aggressively Changes AcceptIncomingMessages to only flush the incoming message queue once synchronously. This prevents deadlock cycles that can block incoming external messages by spinning the IO thread indefinitely. It also prevents similar cycles from chewing up CPU on other threads. Also fixes some Watcher tests that are broken by ProcessIncomingMessages behavior. This change made the brokenness more apparent. BUG=628492 R=amistry@chromium.org ========== to ========== [mojo-edk] Prevent AcceptIncomingMessages flushing aggressively Changes AcceptIncomingMessages to only flush the incoming message queue once synchronously. This prevents deadlock cycles that can block incoming external messages by spinning the IO thread indefinitely. It also prevents similar cycles from chewing up CPU on other threads. Also fixes some Watcher tests that are broken by ProcessIncomingMessages behavior. This change made the brokenness more apparent. BUG=628492 R=amistry@chromium.org Committed: https://crrev.com/5cdb85fdae4f9f9487d67d080b794ca633219303 Cr-Commit-Position: refs/heads/master@{#405776} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/5cdb85fdae4f9f9487d67d080b794ca633219303 Cr-Commit-Position: refs/heads/master@{#405776} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
