0

[TaskScheduler]: Implement AssertLongCPUWorkAllowed.

Use case: An expensive function wants to make sure it's not running on the UI
thread so the expensive operations don't block that thread.
This assertion fails on threads other than scheduler workers.

Bug: 879181
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I2a7ec2442c2020da4d68605c2476a01d25a45a33
Reviewed-on: https://chromium-review.googlesource.com/1221787
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593191}
This commit is contained in:
Etienne Pierre-Doray
2018-09-21 15:28:07 +00:00
committed by Commit Bot
parent aada02dfce
commit 5b5cddb4d2
7 changed files with 50 additions and 12 deletions

@ -23,6 +23,9 @@ LazyInstance<ThreadLocalBoolean>::Leaky
LazyInstance<ThreadLocalBoolean>::Leaky g_base_sync_primitives_disallowed =
LAZY_INSTANCE_INITIALIZER;
LazyInstance<ThreadLocalBoolean>::Leaky g_cpu_intensive_work_disallowed =
LAZY_INSTANCE_INITIALIZER;
} // namespace
void AssertBlockingAllowed() {
@ -114,10 +117,23 @@ void ResetThreadRestrictionsForTesting() {
g_blocking_disallowed.Get().Set(false);
g_singleton_disallowed.Get().Set(false);
g_base_sync_primitives_disallowed.Get().Set(false);
g_cpu_intensive_work_disallowed.Get().Set(false);
}
} // namespace internal
void AssertLongCPUWorkAllowed() {
DCHECK(!g_cpu_intensive_work_disallowed.Get().Get())
<< "Function marked as CPU intensive was called from a scope that "
"disallows this kind of work! Consider making this work asynchronous.";
}
void DisallowUnresponsiveTasks() {
DisallowBlocking();
DisallowBaseSyncPrimitives();
g_cpu_intensive_work_disallowed.Get().Set(true);
}
ThreadRestrictions::ScopedAllowIO::ScopedAllowIO()
: was_allowed_(SetIOAllowed(true)) {}

@ -169,8 +169,8 @@ class ThreadTestHelper;
// a socket, rename or delete a file, enumerate files in a directory, etc.
// Acquiring a low contention lock is not considered a blocking call.
// Asserts that blocking calls are allowed in the current scope. Prefer using
// ScopedBlockingCall instead, which also serves as a precise annotation of the
// Asserts that blocking calls are allowed in the current scope. Prefer using
// ScopedBlockingCall instead, which also serves as a precise annotation of the
// scope that may/will block.
//
// Style tip: It's best if you put AssertBlockingAllowed() checks as close to
@ -407,6 +407,21 @@ INLINE_IF_DCHECK_IS_OFF void ResetThreadRestrictionsForTesting()
} // namespace internal
// "Long CPU" work refers to any code that takes more than 100 ms to
// run when there is no CPU contention and no hard page faults and therefore,
// is not suitable to run on a thread required to keep the browser responsive
// (where jank could be visible to the user).
// Asserts that running long CPU work is allowed in the current scope.
INLINE_IF_DCHECK_IS_OFF void AssertLongCPUWorkAllowed()
EMPTY_BODY_IF_DCHECK_IS_OFF;
// Disallows all tasks that may be unresponsive on the current thread. This
// disallows blocking calls, waiting on a //base sync primitive and long CPU
// work.
INLINE_IF_DCHECK_IS_OFF void DisallowUnresponsiveTasks()
EMPTY_BODY_IF_DCHECK_IS_OFF;
class BASE_EXPORT ThreadRestrictions {
public:
// Constructing a ScopedAllowIO temporarily allows IO for the current

@ -134,4 +134,15 @@ TEST_F(ThreadRestrictionsTest,
scoped_allow_base_sync_primitives_for_testing;
}
TEST_F(ThreadRestrictionsTest, LongCPUWorkAllowedByDefault) {
AssertLongCPUWorkAllowed();
}
TEST_F(ThreadRestrictionsTest, DisallowUnresponsiveTasks) {
DisallowUnresponsiveTasks();
EXPECT_DCHECK_DEATH(AssertBlockingAllowed());
EXPECT_DCHECK_DEATH(internal::AssertBaseSyncPrimitivesAllowed());
EXPECT_DCHECK_DEATH(AssertLongCPUWorkAllowed());
}
} // namespace base

@ -1050,9 +1050,8 @@ int BrowserMainLoop::PreMainMessageLoopRun() {
}
// If the UI thread blocks, the whole UI is unresponsive.
// Do not allow disk IO from the UI thread.
base::ThreadRestrictions::SetIOAllowed(false);
base::ThreadRestrictions::DisallowWaiting();
// Do not allow unresponsive tasks from the UI thread.
base::DisallowUnresponsiveTasks();
return result_code_;
}

@ -100,8 +100,7 @@ void BrowserProcessSubThread::Init() {
#endif
if (!is_blocking_allowed_for_testing_) {
base::DisallowBlocking();
base::DisallowBaseSyncPrimitives();
base::DisallowUnresponsiveTasks();
}
}

@ -145,9 +145,8 @@ int WebMainLoop::PreMainMessageLoopRun() {
}
// If the UI thread blocks, the whole UI is unresponsive.
// Do not allow disk IO from the UI thread.
base::ThreadRestrictions::SetIOAllowed(false);
base::ThreadRestrictions::DisallowWaiting();
// Do not allow unresponsive tasks from the UI thread.
base::DisallowUnresponsiveTasks();
return result_code_;
}

@ -233,8 +233,7 @@ void WebThreadImpl::Init() {
// Though this thread is called the "IO" thread, it actually just routes
// messages around; it shouldn't be allowed to perform any blocking disk
// I/O.
base::ThreadRestrictions::SetIOAllowed(false);
base::ThreadRestrictions::DisallowWaiting();
base::DisallowUnresponsiveTasks();
}
}