Commit 25113733 authored by Francois Doray's avatar Francois Doray Committed by Commit Bot

[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: default avatarFrançois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586311}
parent ef0b4a27
...@@ -939,6 +939,7 @@ jumbo_component("base") { ...@@ -939,6 +939,7 @@ jumbo_component("base") {
"threading/platform_thread_linux.cc", "threading/platform_thread_linux.cc",
"threading/platform_thread_mac.mm", "threading/platform_thread_mac.mm",
"threading/platform_thread_win.cc", "threading/platform_thread_win.cc",
"threading/platform_thread_win.h",
"threading/post_task_and_reply_impl.cc", "threading/post_task_and_reply_impl.cc",
"threading/post_task_and_reply_impl.h", "threading/post_task_and_reply_impl.h",
"threading/scoped_blocking_call.cc", "threading/scoped_blocking_call.cc",
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/synchronization/waitable_event.h" #include "base/synchronization/waitable_event.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/platform_thread.h" #include "base/threading/platform_thread.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -15,6 +16,7 @@ ...@@ -15,6 +16,7 @@
#include "base/threading/platform_thread_internal_posix.h" #include "base/threading/platform_thread_internal_posix.h"
#elif defined(OS_WIN) #elif defined(OS_WIN)
#include <windows.h> #include <windows.h>
#include "base/threading/platform_thread_win.h"
#endif #endif
namespace base { namespace base {
...@@ -288,16 +290,7 @@ class ThreadPriorityTestThread : public FunctionTestThread { ...@@ -288,16 +290,7 @@ class ThreadPriorityTestThread : public FunctionTestThread {
DISALLOW_COPY_AND_ASSIGN(ThreadPriorityTestThread); DISALLOW_COPY_AND_ASSIGN(ThreadPriorityTestThread);
}; };
} // namespace void TestSetCurrentThreadPriority() {
// Test changing a created thread's priority (which has different semantics on
// some platforms).
#if defined(OS_FUCHSIA)
// TODO(crbug.com/851759): Thread priorities are not implemented in Fuchsia.
#define MAYBE_SetCurrentThreadPriority DISABLED_SetCurrentThreadPriority
#else
#define MAYBE_SetCurrentThreadPriority SetCurrentThreadPriority
#endif
TEST(PlatformThreadTest, MAYBE_SetCurrentThreadPriority) {
ThreadPriorityTestThread thread; ThreadPriorityTestThread thread;
PlatformThreadHandle handle; PlatformThreadHandle handle;
...@@ -311,6 +304,30 @@ TEST(PlatformThreadTest, MAYBE_SetCurrentThreadPriority) { ...@@ -311,6 +304,30 @@ TEST(PlatformThreadTest, MAYBE_SetCurrentThreadPriority) {
ASSERT_FALSE(thread.IsRunning()); ASSERT_FALSE(thread.IsRunning());
} }
} // namespace
// Test changing a created thread's priority.
#if defined(OS_FUCHSIA)
// TODO(crbug.com/851759): Thread priorities are not implemented in Fuchsia.
#define MAYBE_SetCurrentThreadPriority DISABLED_SetCurrentThreadPriority
#else
#define MAYBE_SetCurrentThreadPriority SetCurrentThreadPriority
#endif
TEST(PlatformThreadTest, MAYBE_SetCurrentThreadPriority) {
TestSetCurrentThreadPriority();
}
#if defined(OS_WIN)
// Test changing a created thread's priority, with the
// kWindowsThreadModeBackground feature enabled.
TEST(PlatformThreadTest, SetCurrentThreadPriorityWithThreadModeBackground) {
test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(
features::kWindowsThreadModeBackground);
TestSetCurrentThreadPriority();
}
#endif // defined(OS_WIN)
// This tests internal PlatformThread APIs used under some POSIX platforms, // This tests internal PlatformThread APIs used under some POSIX platforms,
// with the exception of Mac OS X, iOS and Fuchsia. // with the exception of Mac OS X, iOS and Fuchsia.
#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_IOS) && \ #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_IOS) && \
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "base/threading/platform_thread.h" #include "base/threading/platform_thread_win.h"
#include <stddef.h> #include <stddef.h>
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "base/threading/thread_id_name_manager.h" #include "base/threading/thread_id_name_manager.h"
#include "base/threading/thread_restrictions.h" #include "base/threading/thread_restrictions.h"
#include "base/win/scoped_handle.h" #include "base/win/scoped_handle.h"
#include "base/win/windows_version.h"
#include <windows.h> #include <windows.h>
...@@ -22,6 +23,14 @@ namespace base { ...@@ -22,6 +23,14 @@ namespace base {
namespace { namespace {
// The value returned by ::GetThreadPriority() after background thread mode is
// enabled on Windows 7.
constexpr int kWin7BackgroundThreadModePriority = 4;
// The value returned by ::GetThreadPriority() after background thread mode is
// enabled on Windows 8+.
constexpr int kWin8AboveBackgroundThreadModePriority = -4;
// The information on how to set the thread name comes from // The information on how to set the thread name comes from
// a MSDN article: http://msdn2.microsoft.com/en-us/library/xcb2z8hs.aspx // a MSDN article: http://msdn2.microsoft.com/en-us/library/xcb2z8hs.aspx
const DWORD kVCThreadNameException = 0x406D1388; const DWORD kVCThreadNameException = 0x406D1388;
...@@ -144,6 +153,11 @@ bool CreateThreadInternal(size_t stack_size, ...@@ -144,6 +153,11 @@ bool CreateThreadInternal(size_t stack_size,
} // namespace } // namespace
namespace features {
const Feature kWindowsThreadModeBackground{"WindowsThreadModeBackground",
FEATURE_DISABLED_BY_DEFAULT};
} // namespace features
// static // static
PlatformThreadId PlatformThread::CurrentId() { PlatformThreadId PlatformThread::CurrentId() {
return ::GetCurrentThreadId(); return ::GetCurrentThreadId();
...@@ -263,10 +277,22 @@ bool PlatformThread::CanIncreaseCurrentThreadPriority() { ...@@ -263,10 +277,22 @@ bool PlatformThread::CanIncreaseCurrentThreadPriority() {
// static // static
void PlatformThread::SetCurrentThreadPriority(ThreadPriority priority) { void PlatformThread::SetCurrentThreadPriority(ThreadPriority priority) {
const bool use_thread_mode_background =
base::FeatureList::IsEnabled(features::kWindowsThreadModeBackground);
if (use_thread_mode_background && priority != ThreadPriority::BACKGROUND) {
// Exit background mode if the new priority is not BACKGROUND. This is a
// no-op if not in background mode.
::SetThreadPriority(PlatformThread::CurrentHandle().platform_handle(),
THREAD_MODE_BACKGROUND_END);
}
int desired_priority = THREAD_PRIORITY_ERROR_RETURN; int desired_priority = THREAD_PRIORITY_ERROR_RETURN;
switch (priority) { switch (priority) {
case ThreadPriority::BACKGROUND: case ThreadPriority::BACKGROUND:
desired_priority = THREAD_PRIORITY_LOWEST; desired_priority = use_thread_mode_background
? THREAD_MODE_BACKGROUND_BEGIN
: THREAD_PRIORITY_LOWEST;
break; break;
case ThreadPriority::NORMAL: case ThreadPriority::NORMAL:
desired_priority = THREAD_PRIORITY_NORMAL; desired_priority = THREAD_PRIORITY_NORMAL;
...@@ -290,13 +316,22 @@ void PlatformThread::SetCurrentThreadPriority(ThreadPriority priority) { ...@@ -290,13 +316,22 @@ void PlatformThread::SetCurrentThreadPriority(ThreadPriority priority) {
desired_priority); desired_priority);
DPLOG_IF(ERROR, !success) << "Failed to set thread priority to " DPLOG_IF(ERROR, !success) << "Failed to set thread priority to "
<< desired_priority; << desired_priority;
// Sanity check that GetCurrentThreadPriority() is consistent with
// SetCurrentThreadPriority().
DCHECK_EQ(GetCurrentThreadPriority(), priority);
} }
// static // static
ThreadPriority PlatformThread::GetCurrentThreadPriority() { ThreadPriority PlatformThread::GetCurrentThreadPriority() {
int priority = const int priority =
::GetThreadPriority(PlatformThread::CurrentHandle().platform_handle()); ::GetThreadPriority(PlatformThread::CurrentHandle().platform_handle());
switch (priority) { switch (priority) {
case kWin7BackgroundThreadModePriority:
DCHECK_EQ(win::GetVersion(), win::VERSION_WIN7);
FALLTHROUGH;
case kWin8AboveBackgroundThreadModePriority:
case THREAD_PRIORITY_LOWEST: case THREAD_PRIORITY_LOWEST:
return ThreadPriority::BACKGROUND; return ThreadPriority::BACKGROUND;
case THREAD_PRIORITY_NORMAL: case THREAD_PRIORITY_NORMAL:
...@@ -306,12 +341,11 @@ ThreadPriority PlatformThread::GetCurrentThreadPriority() { ...@@ -306,12 +341,11 @@ ThreadPriority PlatformThread::GetCurrentThreadPriority() {
case THREAD_PRIORITY_TIME_CRITICAL: case THREAD_PRIORITY_TIME_CRITICAL:
return ThreadPriority::REALTIME_AUDIO; return ThreadPriority::REALTIME_AUDIO;
case THREAD_PRIORITY_ERROR_RETURN: case THREAD_PRIORITY_ERROR_RETURN:
DPCHECK(false) << "GetThreadPriority error"; PCHECK(false) << "GetThreadPriority error";
FALLTHROUGH;
default:
NOTREACHED() << "Unexpected priority: " << priority;
return ThreadPriority::NORMAL;
} }
NOTREACHED() << "GetCurrentThreadPriority returned " << priority << ".";
return ThreadPriority::NORMAL;
} }
} // namespace base } // namespace base
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef BASE_THREADING_PLATFORM_THREAD_WIN_H_
#define BASE_THREADING_PLATFORM_THREAD_WIN_H_
#include "base/threading/platform_thread.h"
#include "base/base_export.h"
#include "base/feature_list.h"
namespace base {
namespace features {
// 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
BASE_EXPORT extern const Feature kWindowsThreadModeBackground;
} // namespace features
} // namespace base
#endif // BASE_THREADING_PLATFORM_THREAD_WIN_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment