Skip to content

GitLab

T
tangled
Commit 1e87b9ae authored by François Doray's avatar François Doray Committed by Commit Bot
Browse files
Options

Revert "[Base] Use background mode for ThreadPriority::BACKGROUND threads... 

Revert "[Base] Use background mode for ThreadPriority::BACKGROUND threads (behind feature) (reland)."

This reverts commit d88c3ec1.

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: default avatarFrançois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585999}
parent 3a41274e
Hide whitespace changes
Inline Side-by-side
Showing with 18 additions and 96 deletions
base/BUILD.gn
View file @ 1e87b9ae
......@@ -939,7 +939,6 @@ jumbo_component("base") {
"threading/platform_thread_linux.cc",
"threading/platform_thread_mac.mm",
"threading/platform_thread_win.cc",
"threading/platform_thread_win.h",
"threading/post_task_and_reply_impl.cc",
"threading/post_task_and_reply_impl.h",
"threading/scoped_blocking_call.cc",
......
base/threading/platform_thread_unittest.cc
View file @ 1e87b9ae
......@@ -7,7 +7,6 @@
#include "base/compiler_specific.h"
#include "base/macros.h"
#include "base/synchronization/waitable_event.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/platform_thread.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -16,7 +15,6 @@
#include "base/threading/platform_thread_internal_posix.h"
#elif defined(OS_WIN)
#include <windows.h>
#include "base/threading/platform_thread_win.h"
#endif
namespace base {
......@@ -290,7 +288,16 @@ class ThreadPriorityTestThread : public FunctionTestThread {
DISALLOW_COPY_AND_ASSIGN(ThreadPriorityTestThread);
};
void TestSetCurrentThreadPriority() {
} // namespace
// 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;
PlatformThreadHandle handle;
......@@ -304,30 +311,6 @@ void TestSetCurrentThreadPriority() {
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,
// with the exception of Mac OS X, iOS and Fuchsia.
#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_IOS) && \
......
base/threading/platform_thread_win.cc
View file @ 1e87b9ae
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/threading/platform_thread_win.h"
#include "base/threading/platform_thread.h"
#include <stddef.h>
......@@ -15,7 +15,6 @@
#include "base/threading/thread_id_name_manager.h"
#include "base/threading/thread_restrictions.h"
#include "base/win/scoped_handle.h"
#include "base/win/windows_version.h"
#include <windows.h>
......@@ -23,14 +22,6 @@ namespace base {
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
// a MSDN article: http://msdn2.microsoft.com/en-us/library/xcb2z8hs.aspx
const DWORD kVCThreadNameException = 0x406D1388;
......@@ -153,11 +144,6 @@ bool CreateThreadInternal(size_t stack_size,
} // namespace
namespace features {
const Feature kWindowsThreadModeBackground{"WindowsThreadModeBackground",
FEATURE_DISABLED_BY_DEFAULT};
} // namespace features
// static
PlatformThreadId PlatformThread::CurrentId() {
return ::GetCurrentThreadId();
......@@ -277,22 +263,10 @@ bool PlatformThread::CanIncreaseCurrentThreadPriority() {
// static
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;
switch (priority) {
case ThreadPriority::BACKGROUND:
desired_priority = use_thread_mode_background
? THREAD_MODE_BACKGROUND_BEGIN
: THREAD_PRIORITY_LOWEST;
desired_priority = THREAD_PRIORITY_LOWEST;
break;
case ThreadPriority::NORMAL:
desired_priority = THREAD_PRIORITY_NORMAL;
......@@ -316,22 +290,13 @@ void PlatformThread::SetCurrentThreadPriority(ThreadPriority priority) {
desired_priority);
DPLOG_IF(ERROR, !success) << "Failed to set thread priority to "
<< desired_priority;
// Sanity check that GetCurrentThreadPriority() is consistent with
// SetCurrentThreadPriority().
DCHECK_EQ(GetCurrentThreadPriority(), priority);
}
// static
ThreadPriority PlatformThread::GetCurrentThreadPriority() {
const int priority =
int priority =
::GetThreadPriority(PlatformThread::CurrentHandle().platform_handle());
switch (priority) {
case kWin7BackgroundThreadModePriority:
DCHECK_EQ(win::GetVersion(), win::VERSION_WIN7);
FALLTHROUGH;
case kWin8AboveBackgroundThreadModePriority:
case THREAD_PRIORITY_LOWEST:
return ThreadPriority::BACKGROUND;
case THREAD_PRIORITY_NORMAL:
......@@ -341,11 +306,12 @@ ThreadPriority PlatformThread::GetCurrentThreadPriority() {
case THREAD_PRIORITY_TIME_CRITICAL:
return ThreadPriority::REALTIME_AUDIO;
case THREAD_PRIORITY_ERROR_RETURN:
PCHECK(false) << "GetThreadPriority error";
DPCHECK(false) << "GetThreadPriority error";
FALLTHROUGH;
default:
NOTREACHED() << "Unexpected priority: " << priority;
return ThreadPriority::NORMAL;
}
NOTREACHED();
return ThreadPriority::NORMAL;
}
} // namespace base
base/threading/platform_thread_win.h deleted 100644 → 0
View file @ 3a41274e
// 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