New issue
Advanced search Search tips

Issue 902441 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 831835
issue 901483

Blocking:
issue 872820



Sign in to add a comment

Experiment with task and thread priorities

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

Issue description

We should experiment with disabling task and thread priorities as well as making thread priorities stronger (THREAD_MODE_BACKGROUND_BEGIN on Windows) to assess the impact of prioritization on performance.
 
Blockedon: 901483 831835
Blocking: 872820
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 8

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

commit 4cbbe2d8dec6506a415d7524222f8dda6e0a09c8
Author: Francois Doray <fdoray@chromium.org>
Date: Thu Nov 08 00:54:58 2018

Make ScopedTaskEnvironment the first member of QuicHttpProxyBackendStreamTest.

The documentation for ScopedTaskEnvironment recommends making it the
first member of the test fixture:
  https://cs.chromium.org/chromium/src/base/test/scoped_task_environment.h?l=54-59&rcl=376db7a96e0287f0d010acd18e7a379d190cac71

We are chaning this particular case because we want to make the
constructor of ScopedTaskEnvironment set a global variable that is
read by PlatformThread::SetCurrentThreadPriority(). If
ScopedTaskEnvironment is initialized late in the test execution,
setting this global variable races with a call to
PlatfromThread::SetCurrentThreadPriority() on the thread created
from this stack:
  base::(anonymous namespace)::CreateThread
  base::PlatformThread::CreateWithPriority
  base::Thread::StartWithOptions
  net::test::QuicHttpProxyBackendStreamTest::CreateTestBackendProxy
  net::test::QuicHttpProxyBackendStreamTest::SetUp

Bug: 902441
Change-Id: I4869410f43383bd7f94f68fc7790e3305f3d1b4a
Reviewed-on: https://chromium-review.googlesource.com/c/1320654
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606259}
[modify] https://crrev.com/4cbbe2d8dec6506a415d7524222f8dda6e0a09c8/net/tools/quic/quic_http_proxy_backend_stream_test.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 9

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

commit 5b44e33faf93bb62914b44272e60a3ac28c2b38f
Author: Francois Doray <fdoray@chromium.org>
Date: Fri Nov 09 22:08:13 2018

TaskScheduler: Use a Feature to control whether all tasks are USER_BLOCKING.

Previously, we read a variation param from the "BrowserScheduler" study
to determine whether all tasks should have USER_BLOCKING priority.
With this CL, we instead use a base::Feature.

Benefits:
- Study that controls the feature doesn't have to be "BrowserScheduler".
- Enabling/disabling a feature is less verbose than setting a variation
  params with the new GCL config format.

Bug: 902441
Change-Id: Ib38dcfdf097c1ca92784a22c3501b644b077f744
Reviewed-on: https://chromium-review.googlesource.com/c/1321032
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606988}
[modify] https://crrev.com/5b44e33faf93bb62914b44272e60a3ac28c2b38f/base/task/task_features.cc
[modify] https://crrev.com/5b44e33faf93bb62914b44272e60a3ac28c2b38f/base/task/task_features.h
[modify] https://crrev.com/5b44e33faf93bb62914b44272e60a3ac28c2b38f/base/task/task_scheduler/task_scheduler_impl.cc
[modify] https://crrev.com/5b44e33faf93bb62914b44272e60a3ac28c2b38f/base/task/task_scheduler/task_scheduler_impl_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, 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

Labels: -Pri-3 Pri-1

Sign in to add a comment