TaskScheduler: Use background mode for threads in the background pool on Windows. |
|||
Issue descriptionThreads in the background pool use SetThreadPriority(THREAD_PRIORITY_LOWEST). This lowers their CPU priority, but not their disk I/O and network I/O priority. We should experiment with instead using THREAD_MODE_BACKGROUND_BEGIN/THREAD_MODE_BACKGROUND_END, which is documented as the proper solution to prevent any interference with foreground activity. https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-setthreadpriority
,
Aug 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c7196c14b362446bd55e39b0a1da7d3f8783b61b commit c7196c14b362446bd55e39b0a1da7d3f8783b61b Author: Francois Doray <fdoray@chromium.org> Date: Wed Aug 22 17:06:31 2018 [Base] Use background mode for ThreadPriority::BACKGROUND threads (behind feature). This CL adds a feature to use THREAD_MODE_BACKGROUND_BEGIN instead of THREAD_PRIORITY_LOWEST for ThreadPriority::BACKGROUND threads. This lowers the disk and network I/O priority of the thread in addition to the CPU scheduling priority. MSDN recommends using this setting for threads that perform background work. https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-setthreadpriority Bug: 872820 Change-Id: Ia731fcf39c991ae30ca74055595a84385f27635f Reviewed-on: https://chromium-review.googlesource.com/1171482 Commit-Queue: François Doray <fdoray@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#585104} [modify] https://crrev.com/c7196c14b362446bd55e39b0a1da7d3f8783b61b/base/BUILD.gn [modify] https://crrev.com/c7196c14b362446bd55e39b0a1da7d3f8783b61b/base/threading/platform_thread_unittest.cc [modify] https://crrev.com/c7196c14b362446bd55e39b0a1da7d3f8783b61b/base/threading/platform_thread_win.cc [add] https://crrev.com/c7196c14b362446bd55e39b0a1da7d3f8783b61b/base/threading/platform_thread_win.h
,
Aug 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95ddb1cc994f38ae1a0966bd695ada682e7e5e67 commit 95ddb1cc994f38ae1a0966bd695ada682e7e5e67 Author: Sky Malice <skym@chromium.org> Date: Wed Aug 22 17:41:00 2018 Revert "[Base] Use background mode for ThreadPriority::BACKGROUND threads (behind feature)." This reverts commit c7196c14b362446bd55e39b0a1da7d3f8783b61b. Reason for revert: WebKit Win Builder failing, https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Win%20Builder/158247 speculatively reverting Original change's description: > [Base] Use background mode for ThreadPriority::BACKGROUND threads (behind feature). > > This CL adds a feature to use THREAD_MODE_BACKGROUND_BEGIN instead of > THREAD_PRIORITY_LOWEST for ThreadPriority::BACKGROUND threads. This > lowers the disk and network I/O priority of the thread in addition to > the CPU scheduling priority. MSDN recommends using this setting for > threads that perform background work. > https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-setthreadpriority > > Bug: 872820 > Change-Id: Ia731fcf39c991ae30ca74055595a84385f27635f > Reviewed-on: https://chromium-review.googlesource.com/1171482 > Commit-Queue: François Doray <fdoray@chromium.org> > Reviewed-by: Gabriel Charette <gab@chromium.org> > Cr-Commit-Position: refs/heads/master@{#585104} TBR=gab@chromium.org,fdoray@chromium.org Change-Id: Ia6a20e3a59b68ee0c845f26f343bb10582d03d72 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 872820 Reviewed-on: https://chromium-review.googlesource.com/1185342 Reviewed-by: Sky Malice <skym@chromium.org> Commit-Queue: Sky Malice <skym@chromium.org> Cr-Commit-Position: refs/heads/master@{#585142} [modify] https://crrev.com/95ddb1cc994f38ae1a0966bd695ada682e7e5e67/base/BUILD.gn [modify] https://crrev.com/95ddb1cc994f38ae1a0966bd695ada682e7e5e67/base/threading/platform_thread_unittest.cc [modify] https://crrev.com/95ddb1cc994f38ae1a0966bd695ada682e7e5e67/base/threading/platform_thread_win.cc [delete] https://crrev.com/e004339aec5845c5c9a9592af0b4170d42fdf906/base/threading/platform_thread_win.h
,
Aug 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d88c3ec18b44afe5a8f4a5b147e7e15dead47426 commit d88c3ec18b44afe5a8f4a5b147e7e15dead47426 Author: Francois Doray <fdoray@chromium.org> Date: Wed Aug 22 22:03:55 2018 [Base] Use background mode for ThreadPriority::BACKGROUND threads (behind feature) (reland). Relanding a CL that was reverted because of a variable that was unused in non-DCHECK builds and caused a compile failure. Original CL: Reviewed-on: https://chromium-review.googlesource.com/1171482 This CL adds a feature to use THREAD_MODE_BACKGROUND_BEGIN instead of THREAD_PRIORITY_LOWEST for ThreadPriority::BACKGROUND threads. This lowers the disk and network I/O priority of the thread in addition to the CPU scheduling priority. MSDN recommends using this setting for threads that perform background work. https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-setthreadpriority TBR=gab@chromium.org Bug: 872820 Change-Id: I212fb77bb035088595b4944ce145a74d333c38eb Reviewed-on: https://chromium-review.googlesource.com/1185529 Commit-Queue: François Doray <fdoray@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Reviewed-by: François Doray <fdoray@chromium.org> Cr-Commit-Position: refs/heads/master@{#585272} [modify] https://crrev.com/d88c3ec18b44afe5a8f4a5b147e7e15dead47426/base/BUILD.gn [modify] https://crrev.com/d88c3ec18b44afe5a8f4a5b147e7e15dead47426/base/threading/platform_thread_unittest.cc [modify] https://crrev.com/d88c3ec18b44afe5a8f4a5b147e7e15dead47426/base/threading/platform_thread_win.cc [add] https://crrev.com/d88c3ec18b44afe5a8f4a5b147e7e15dead47426/base/threading/platform_thread_win.h
,
Aug 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1e87b9aeba21ab5108409067f045861e0620a6a3 commit 1e87b9aeba21ab5108409067f045861e0620a6a3 Author: François Doray <fdoray@chromium.org> Date: Fri Aug 24 21:37:58 2018 Revert "[Base] Use background mode for ThreadPriority::BACKGROUND threads (behind feature) (reland)." This reverts commit d88c3ec18b44afe5a8f4a5b147e7e15dead47426. Reason for revert: PlatformThreadTest.SetCurrentThreadPriorityWithThreadModeBackground is Flaky: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=base_unittests&tests=SetCurrentThreadPriorityWithThreadModeBackground Original change's description: > [Base] Use background mode for ThreadPriority::BACKGROUND threads (behind feature) (reland). > > Relanding a CL that was reverted because of a variable that was > unused in non-DCHECK builds and caused a compile failure. > Original CL: Reviewed-on: https://chromium-review.googlesource.com/1171482 > > This CL adds a feature to use THREAD_MODE_BACKGROUND_BEGIN instead of > THREAD_PRIORITY_LOWEST for ThreadPriority::BACKGROUND threads. This > lowers the disk and network I/O priority of the thread in addition to > the CPU scheduling priority. MSDN recommends using this setting for > threads that perform background work. > https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-setthreadpriority > > TBR=gab@chromium.org > > Bug: 872820 > Change-Id: I212fb77bb035088595b4944ce145a74d333c38eb > Reviewed-on: https://chromium-review.googlesource.com/1185529 > Commit-Queue: François Doray <fdoray@chromium.org> > Commit-Queue: Gabriel Charette <gab@chromium.org> > Reviewed-by: Gabriel Charette <gab@chromium.org> > Reviewed-by: François Doray <fdoray@chromium.org> > Cr-Commit-Position: refs/heads/master@{#585272} TBR=gab@chromium.org,fdoray@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 872820, 877628 Change-Id: I6cf6625fc1f01106e8fff1e206a6a256f3a6160a Reviewed-on: https://chromium-review.googlesource.com/1188644 Commit-Queue: François Doray <fdoray@chromium.org> Reviewed-by: François Doray <fdoray@chromium.org> Cr-Commit-Position: refs/heads/master@{#585999} [modify] https://crrev.com/1e87b9aeba21ab5108409067f045861e0620a6a3/base/BUILD.gn [modify] https://crrev.com/1e87b9aeba21ab5108409067f045861e0620a6a3/base/threading/platform_thread_unittest.cc [modify] https://crrev.com/1e87b9aeba21ab5108409067f045861e0620a6a3/base/threading/platform_thread_win.cc [delete] https://crrev.com/3a41274e80c4b8b81733cd8b9c0f7e0bf6ae4b13/base/threading/platform_thread_win.h
,
Aug 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/25113733c29967022d59118621aabe491a7bcc13 commit 25113733c29967022d59118621aabe491a7bcc13 Author: Francois Doray <fdoray@chromium.org> Date: Mon Aug 27 17:59:13 2018 [Base] Use background mode for ThreadPriority::BACKGROUND threads (behind feature) (reland). The previous attempt to land this CL was reverted because a value that we didn't expect was returned by ::GetThreadPriority() on Win7 bots. This CL adds logging so we can find out what value is returned and add it to the expected values if need be. Previous attempts to land: https://chromium-review.googlesource.com/1171482 https://chromium-review.googlesource.com/1185529 NOTE: If this causes new test failures, we'll add the value returned by GetThreadPriority() to the switch/case of allowed values. This CL adds a feature to use THREAD_MODE_BACKGROUND_BEGIN instead of THREAD_PRIORITY_LOWEST for ThreadPriority::BACKGROUND threads. This lowers the disk and network I/O priority of the thread in addition to the CPU scheduling priority. MSDN recommends using this setting for threads that perform background work. https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-setthreadpriority TBR=gab@chromium.org Bug: 872820 Change-Id: I54de2f17c66985ac437ca7809d3b515b22c72544 Reviewed-on: https://chromium-review.googlesource.com/1190842 Commit-Queue: François Doray <fdoray@chromium.org> Reviewed-by: François Doray <fdoray@chromium.org> Cr-Commit-Position: refs/heads/master@{#586311} [modify] https://crrev.com/25113733c29967022d59118621aabe491a7bcc13/base/BUILD.gn [modify] https://crrev.com/25113733c29967022d59118621aabe491a7bcc13/base/threading/platform_thread_unittest.cc [modify] https://crrev.com/25113733c29967022d59118621aabe491a7bcc13/base/threading/platform_thread_win.cc [add] https://crrev.com/25113733c29967022d59118621aabe491a7bcc13/base/threading/platform_thread_win.h
,
Aug 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/444629035b93451873725951cd98b89197b18006 commit 444629035b93451873725951cd98b89197b18006 Author: Francois Doray <fdoray@chromium.org> Date: Tue Aug 28 20:32:02 2018 [Base] Support ::GetThreadPriority() returning THREAD_PRIORITY_IDLE. ::GetThreadPriority() can return THREAD_PRIORITY_IDLE after background thread mode has been enabled on Windows 7. This CL adds this to the list of expected values in PlatformThread::GetCurrentThreadPriority(). This CL also fixes nits pointed out by gab@ in https://chromium-review.googlesource.com/c/chromium/src/+/1190842 Bug: 872820 Change-Id: I16e2907eb36a0f037fd134934ba88c091673e1c4 Reviewed-on: https://chromium-review.googlesource.com/1194327 Commit-Queue: François Doray <fdoray@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#586836} [modify] https://crrev.com/444629035b93451873725951cd98b89197b18006/base/threading/platform_thread_win.cc
,
Aug 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a783b300982f1d75d5d679c7dbc3f22744112cf commit 8a783b300982f1d75d5d679c7dbc3f22744112cf Author: Francois Doray <fdoray@chromium.org> Date: Wed Aug 29 22:52:08 2018 [Base] Use background mode for ThreadPriority::BACKGROUND threads (behind feature) (reland). The previous attempt to land this CL was reverted because an early call to PlatformThread::SetCurrentThreadPriority() in headless caused the FeatureList to be accessed before it was initialized. This CL avoids accessing an uninitialized FeatureList if the target priority is not BACKGROUND. Diff between ps 1 and 5 shows changes made since revert. Previous attempts to land: https://chromium-review.googlesource.com/1171482 https://chromium-review.googlesource.com/1185529 https://chromium-review.googlesource.com/1190842 NOTE: If this causes new test failures, we'll add the value returned by GetThreadPriority() to the switch/case of allowed values. This CL adds a feature to use THREAD_MODE_BACKGROUND_BEGIN instead of THREAD_PRIORITY_LOWEST for ThreadPriority::BACKGROUND threads. This lowers the disk and network I/O priority of the thread in addition to the CPU scheduling priority. MSDN recommends using this setting for threads that perform background work. https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-setthreadpriority Bug: 872820 Change-Id: I2c9f17390eaaba2f8f454faa29836bbc37e3d47c Reviewed-on: https://chromium-review.googlesource.com/1195677 Commit-Queue: François Doray <fdoray@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#587349} [modify] https://crrev.com/8a783b300982f1d75d5d679c7dbc3f22744112cf/base/BUILD.gn [modify] https://crrev.com/8a783b300982f1d75d5d679c7dbc3f22744112cf/base/threading/platform_thread_unittest.cc [modify] https://crrev.com/8a783b300982f1d75d5d679c7dbc3f22744112cf/base/threading/platform_thread_win.cc [add] https://crrev.com/8a783b300982f1d75d5d679c7dbc3f22744112cf/base/threading/platform_thread_win.h
,
Sep 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/628c1e15a3143e0d9bb760fdf1207315992feb4d commit 628c1e15a3143e0d9bb760fdf1207315992feb4d Author: Dominic Battre <battre@chromium.org> Date: Mon Sep 10 12:10:02 2018 Disable flaky tests in SafeBrowsingPrefsTest.* Several tests crash flakily (see crbug.com/881476) on Windows. This CL disables those CLs. TBR=jialiul@chromium.org,fdoray@chromium.org,asvitkine@chromium.org Bug: 881476,872820 Change-Id: I73104af5fc5a9215c313e06c3ecc77dcf426100c Reviewed-on: https://chromium-review.googlesource.com/1215862 Reviewed-by: Dominic Battré <battre@chromium.org> Commit-Queue: Dominic Battré <battre@chromium.org> Cr-Commit-Position: refs/heads/master@{#589889} [modify] https://crrev.com/628c1e15a3143e0d9bb760fdf1207315992feb4d/components/safe_browsing/common/safe_browsing_prefs_unittest.cc
,
Oct 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b910c700ce7be682dd09cb0cab842f01e56ca76 commit 7b910c700ce7be682dd09cb0cab842f01e56ca76 Author: Francois Doray <fdoray@chromium.org> Date: Sat Oct 27 04:21:28 2018 Use USER_BLOCKING priority in SQLitePersistentCookieStore. We have verified that tasks posted by SQLitePersistentCookieStore are on the critical path of a page load. According to the documentation in task_traits.h, being on the critical path of a page load requires USER_BLOCKING priority. We keep this as an experiment because we can to assess the impact of this change when the WindowsThreadModeBackground feature is enabled. Bug: 872820, 878222 Change-Id: Iec368de8f59b34703688d8f8811aedbeddc50a8d Reviewed-on: https://chromium-review.googlesource.com/c/1303039 Reviewed-by: Victor Costan <pwnall@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#603308} [modify] https://crrev.com/7b910c700ce7be682dd09cb0cab842f01e56ca76/net/extras/sqlite/sqlite_persistent_cookie_store.cc
,
Nov 7
,
Nov 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1b876dd0841d6a1a44a5fbdadff2191012a1e0f1 commit 1b876dd0841d6a1a44a5fbdadff2191012a1e0f1 Author: Francois Doray <fdoray@chromium.org> Date: Mon Nov 12 17:09:37 2018 Base: Add feature to disable thread priorities. This will allow us to assess the impact of using thread priorities. The impact can be both positive (unimportant work gets out of the way) and negative (priority inversions when sharing locks across threads with different priorities). Note: This CL has extra complexity because we don't have guarantees that the FeatureList is initialized before threads are started in unit tests https://crbug.com/846380. Bug: 872820, 890978 , 902441, 846380 Change-Id: I8887c7b9e0eb77f7c11aa6e2be8af1ea7150b491 Reviewed-on: https://chromium-review.googlesource.com/c/1318686 Commit-Queue: François Doray <fdoray@chromium.org> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#607276} [modify] https://crrev.com/1b876dd0841d6a1a44a5fbdadff2191012a1e0f1/base/BUILD.gn [modify] https://crrev.com/1b876dd0841d6a1a44a5fbdadff2191012a1e0f1/base/task/task_scheduler/task_scheduler_impl.cc [add] https://crrev.com/1b876dd0841d6a1a44a5fbdadff2191012a1e0f1/base/threading/platform_thread.cc [modify] https://crrev.com/1b876dd0841d6a1a44a5fbdadff2191012a1e0f1/base/threading/platform_thread.h [modify] https://crrev.com/1b876dd0841d6a1a44a5fbdadff2191012a1e0f1/base/threading/platform_thread_fuchsia.cc [modify] https://crrev.com/1b876dd0841d6a1a44a5fbdadff2191012a1e0f1/base/threading/platform_thread_mac.mm [modify] https://crrev.com/1b876dd0841d6a1a44a5fbdadff2191012a1e0f1/base/threading/platform_thread_posix.cc [modify] https://crrev.com/1b876dd0841d6a1a44a5fbdadff2191012a1e0f1/base/threading/platform_thread_win.cc
,
Nov 12
,
Dec 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6e6f0a41bc5fb42b1119f1a464d05e9aab3a20ab commit 6e6f0a41bc5fb42b1119f1a464d05e9aab3a20ab Author: Francois Doray <fdoray@chromium.org> Date: Sat Dec 29 19:11:32 2018 TaskScheduler: Increase MayBlock threshold for background pools. In a background pool: - A blocking call can take more time. - The goal is to minimize impact on foreground work, not to maximize task execution throughput. For these 2 reasons, it is desirable to use a long threshold in background pools before a MAY_BLOCK ScopedBlockingCall causes an increase of the maximum number of tasks that can run concurrently. Bug: 872820 Change-Id: Ia5f934edb10067be307fbe99f4ef163054e4bdea Reviewed-on: https://chromium-review.googlesource.com/c/1388752 Commit-Queue: François Doray <fdoray@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#619252} [modify] https://crrev.com/6e6f0a41bc5fb42b1119f1a464d05e9aab3a20ab/base/task/task_features.h [modify] https://crrev.com/6e6f0a41bc5fb42b1119f1a464d05e9aab3a20ab/base/task/task_scheduler/scheduler_worker_pool_impl.cc
,
Jan 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d8c0fca2ac2d609e1963e311ebd5a832d98b8d96 commit d8c0fca2ac2d609e1963e311ebd5a832d98b8d96 Author: Etienne Pierre-doray <etiennep@chromium.org> Date: Mon Jan 07 19:51:30 2019 Cleanup CookieStorePriority experiment. SQLitePersistentCookieStore priority is now defaulted to USER_BLOCKING. Bug: 872820, 878222 Change-Id: Ie5c062b6718082534d61725601b7884ab29d9b49 Reviewed-on: https://chromium-review.googlesource.com/c/1398062 Commit-Queue: Victor Costan <pwnall@chromium.org> Reviewed-by: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/master@{#620433} [modify] https://crrev.com/d8c0fca2ac2d609e1963e311ebd5a832d98b8d96/net/extras/sqlite/sqlite_persistent_cookie_store.cc |
|||
►
Sign in to add a comment |
|||
Comment 1 by bugdroid1@chromium.org
, Aug 20