New issue
Advanced search Search tips

Issue 901483 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 902441
issue 903062
issue 903239



Sign in to add a comment

Base: SetCurrentThreadPriority(BACKGROUND) fails in a background process on Windows

Project Member Reported by fdoray@chromium.org, Nov 2

Issue description

I have observed that calling :SetThreadPriority(THREAD_MODE_BACKGROUND_BEGIN) in a IDLE_PRIORITY_CLASS process has no effect on the return value of ::GetThreadPriority() or on the base priority displayed in Process Explorer. It does however SET the I/O priority to low.

Because of that, PlatformThread::SetCurrentThreadPriority() should call ::SetThreadPriority(THREAD_PRIORITY_LOWEST) when it observes that ::GetThreadPriority() doesn't return the expected value after a call to :SetThreadPriority(THREAD_MODE_BACKGROUND_BEGIN), to ensure that base priority is lowered appropriately.
 
Blocking: 902441
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5cb9cc29dc6efdf31814e5df7648eb2333767f5e

commit 5cb9cc29dc6efdf31814e5df7648eb2333767f5e
Author: Francois Doray <fdoray@chromium.org>
Date: Wed Nov 07 23:23:52 2018

Base: Ensure that SetCurrentThreadPriority(BACKGROUND) sets CPU priority on Windows.

Previously, SetCurrentThreadPriority(BACKGROUND) did not affect CPU
priority on Windows in a IDLE process when the
"WindowsThreadModeBackground" feature was enabled. It only affected I/O
and memory priorities.

With this CL, a second call to ::SetThreadPriority() is made to
ensure that CPU priority is affected.

Bug:  901483 
Change-Id: I22640c8fa56eeec489fbf0fe516359b828e39541
Reviewed-on: https://chromium-review.googlesource.com/c/1318438
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606200}
[modify] https://crrev.com/5cb9cc29dc6efdf31814e5df7648eb2333767f5e/base/BUILD.gn
[modify] https://crrev.com/5cb9cc29dc6efdf31814e5df7648eb2333767f5e/base/threading/platform_thread_unittest.cc
[modify] https://crrev.com/5cb9cc29dc6efdf31814e5df7648eb2333767f5e/base/threading/platform_thread_win.cc
[modify] https://crrev.com/5cb9cc29dc6efdf31814e5df7648eb2333767f5e/base/threading/platform_thread_win.h
[add] https://crrev.com/5cb9cc29dc6efdf31814e5df7648eb2333767f5e/base/threading/platform_thread_win_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/726b2b57a3142afe2d612f8bd5b4525935088985

commit 726b2b57a3142afe2d612f8bd5b4525935088985
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Thu Nov 08 12:55:18 2018

Revert "Base: Ensure that SetCurrentThreadPriority(BACKGROUND) sets CPU priority on Windows."

This reverts commit 5cb9cc29dc6efdf31814e5df7648eb2333767f5e.

Reason for revert: test PlatformThreadWinTest.SetBackgroundThreadModeFailsInIdlePriorityProcess which was added on this CL fails on some bots, 
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20%2832%29%20Tests/40510

Original change's description:
> Base: Ensure that SetCurrentThreadPriority(BACKGROUND) sets CPU priority on Windows.
> 
> Previously, SetCurrentThreadPriority(BACKGROUND) did not affect CPU
> priority on Windows in a IDLE process when the
> "WindowsThreadModeBackground" feature was enabled. It only affected I/O
> and memory priorities.
> 
> With this CL, a second call to ::SetThreadPriority() is made to
> ensure that CPU priority is affected.
> 
> Bug:  901483 
> Change-Id: I22640c8fa56eeec489fbf0fe516359b828e39541
> Reviewed-on: https://chromium-review.googlesource.com/c/1318438
> Commit-Queue: François Doray <fdoray@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#606200}

TBR=gab@chromium.org,fdoray@chromium.org

Change-Id: I6ea74d6d1fa14a7b668292e7c7ef75c5ace643e6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  901483 
Reviewed-on: https://chromium-review.googlesource.com/c/1325990
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606429}
[modify] https://crrev.com/726b2b57a3142afe2d612f8bd5b4525935088985/base/BUILD.gn
[modify] https://crrev.com/726b2b57a3142afe2d612f8bd5b4525935088985/base/threading/platform_thread_unittest.cc
[modify] https://crrev.com/726b2b57a3142afe2d612f8bd5b4525935088985/base/threading/platform_thread_win.cc
[modify] https://crrev.com/726b2b57a3142afe2d612f8bd5b4525935088985/base/threading/platform_thread_win.h
[delete] https://crrev.com/0d1087c7da1b20c8cb3de32890b05102513b6200/base/threading/platform_thread_win_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0be8c42f6ac8527d89245df06a4a38f8fc2e4958

commit 0be8c42f6ac8527d89245df06a4a38f8fc2e4958
Author: Francois Doray <fdoray@chromium.org>
Date: Mon Nov 12 17:01:37 2018

Base: Ensure that SetCurrentThreadPriority(BACKGROUND) sets CPU priority on Windows (reland).

Previously landed as
https://chromium-review.googlesource.com/c/1318438.
Reverted because
PlatformThreadWinTest.SetBackgroundThreadModeFailsInIdlePriorityProcess
failed on Windows 7. This reland adds more possible return values for
::GetThreadPriority() on Windows 7 (see diff between ps1 and ps7).

Previously, SetCurrentThreadPriority(BACKGROUND) did not affect CPU
priority on Windows in a IDLE process when the
"WindowsThreadModeBackground" feature was enabled. It only affected I/O
and memory priorities.

With this CL, a second call to ::SetThreadPriority() is made to
ensure that CPU priority is affected.

TBR=gab@chromium.org

Bug:  901483 
Change-Id: I21ebc6465b3b8d10a956714d55ef88b28bf89fff
Reviewed-on: https://chromium-review.googlesource.com/c/1326423
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607269}
[modify] https://crrev.com/0be8c42f6ac8527d89245df06a4a38f8fc2e4958/base/BUILD.gn
[modify] https://crrev.com/0be8c42f6ac8527d89245df06a4a38f8fc2e4958/base/threading/platform_thread_unittest.cc
[modify] https://crrev.com/0be8c42f6ac8527d89245df06a4a38f8fc2e4958/base/threading/platform_thread_win.cc
[modify] https://crrev.com/0be8c42f6ac8527d89245df06a4a38f8fc2e4958/base/threading/platform_thread_win.h
[add] https://crrev.com/0be8c42f6ac8527d89245df06a4a38f8fc2e4958/base/threading/platform_thread_win_unittest.cc

Status: Fixed (was: Assigned)
Blocking: 903239
Blocking: 903062

Sign in to add a comment