Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2228)

Unified Diff: base/threading/sequenced_worker_pool.cc

Issue 18292012: base: Re-commit SequencedWorkerPool globally unique tokens with unused variable fix (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 7 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/threading/sequenced_worker_pool.h ('k') | base/threading/sequenced_worker_pool_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/threading/sequenced_worker_pool.cc
diff --git a/base/threading/sequenced_worker_pool.cc b/base/threading/sequenced_worker_pool.cc
index f98b23d5d6168a818178fe0c2e0a777edd618bd5..b2d50d5d0e67dc4e45f8901ee6cf8f9c4a5a2648 100644
--- a/base/threading/sequenced_worker_pool.cc
+++ b/base/threading/sequenced_worker_pool.cc
@@ -10,21 +10,22 @@
#include <utility>
#include <vector>
-#include "base/atomicops.h"
+#include "base/atomic_sequence_num.h"
#include "base/callback.h"
#include "base/compiler_specific.h"
#include "base/critical_closure.h"
#include "base/debug/trace_event.h"
+#include "base/lazy_instance.h"
#include "base/logging.h"
#include "base/memory/linked_ptr.h"
#include "base/message_loop/message_loop_proxy.h"
-#include "base/metrics/histogram.h"
#include "base/stl_util.h"
#include "base/strings/stringprintf.h"
#include "base/synchronization/condition_variable.h"
#include "base/synchronization/lock.h"
#include "base/threading/platform_thread.h"
#include "base/threading/simple_thread.h"
+#include "base/threading/thread_local.h"
#include "base/threading/thread_restrictions.h"
#include "base/time/time.h"
#include "base/tracked_objects.h"
@@ -33,6 +34,10 @@
#include "base/mac/scoped_nsautorelease_pool.h"
#endif
+#if !defined(OS_NACL)
+#include "base/metrics/histogram.h"
+#endif
+
namespace base {
namespace {
@@ -213,6 +218,10 @@ uint64 GetTaskTraceID(const SequencedTask& task,
static_cast<uint64>(reinterpret_cast<intptr_t>(pool));
}
+base::LazyInstance<base::ThreadLocalPointer<
+ SequencedWorkerPool::SequenceToken> > g_lazy_tls_ptr =
+ LAZY_INSTANCE_INITIALIZER;
+
} // namespace
// Worker ---------------------------------------------------------------------
@@ -382,8 +391,9 @@ class SequencedWorkerPool::Inner {
// The last sequence number used. Managed by GetSequenceToken, since this
// only does threadsafe increment operations, you do not need to hold the
- // lock.
- volatile subtle::Atomic32 last_sequence_number_;
+ // lock. This is class-static to make SequenceTokens issued by
+ // GetSequenceToken unique across SequencedWorkerPool instances.
+ static base::StaticAtomicSequenceNumber g_last_sequence_number_;
// This lock protects |everything in this class|. Do not read or modify
// anything without holding this lock. Do not block while holding this
@@ -479,6 +489,10 @@ SequencedWorkerPool::Worker::~Worker() {
}
void SequencedWorkerPool::Worker::Run() {
+ // Store a pointer to the running sequence in thread local storage for
+ // static function access.
+ g_lazy_tls_ptr.Get().Set(&running_sequence_);
+
// Just jump back to the Inner object to run the thread, since it has all the
// tracking information and queues. It might be more natural to implement
// using DelegateSimpleThread and have Inner implement the Delegate to avoid
@@ -497,7 +511,6 @@ SequencedWorkerPool::Inner::Inner(
const std::string& thread_name_prefix,
TestingObserver* observer)
: worker_pool_(worker_pool),
- last_sequence_number_(0),
lock_(),
has_work_cv_(&lock_),
can_shutdown_cv_(&lock_),
@@ -532,9 +545,9 @@ SequencedWorkerPool::Inner::~Inner() {
SequencedWorkerPool::SequenceToken
SequencedWorkerPool::Inner::GetSequenceToken() {
- subtle::Atomic32 result =
- subtle::NoBarrier_AtomicIncrement(&last_sequence_number_, 1);
- return SequenceToken(static_cast<int>(result));
+ // Need to add one because StaticAtomicSequenceNumber starts at zero, which
+ // is used as a sentinel value in SequenceTokens.
+ return SequenceToken(g_last_sequence_number_.GetNext() + 1);
}
SequencedWorkerPool::SequenceToken
@@ -615,7 +628,7 @@ bool SequencedWorkerPool::Inner::IsRunningSequenceOnCurrentThread(
ThreadMap::const_iterator found = threads_.find(PlatformThread::CurrentId());
if (found == threads_.end())
return false;
- return found->second->running_sequence().Equals(sequence_token);
+ return sequence_token.Equals(found->second->running_sequence());
}
// See https://code.google.com/p/chromium/issues/detail?id=168415
@@ -666,7 +679,9 @@ void SequencedWorkerPool::Inner::Shutdown(
if (testing_observer_)
testing_observer_->WillWaitForShutdown();
+#if !defined(OS_NACL)
TimeTicks shutdown_wait_begin = TimeTicks::Now();
+#endif
{
base::ThreadRestrictions::ScopedAllowWait allow_wait;
@@ -674,8 +689,10 @@ void SequencedWorkerPool::Inner::Shutdown(
while (!CanShutdown())
can_shutdown_cv_.Wait();
}
+#if !defined(OS_NACL)
UMA_HISTOGRAM_TIMES("SequencedWorkerPool.ShutdownDelayTime",
TimeTicks::Now() - shutdown_wait_begin);
+#endif
}
void SequencedWorkerPool::Inner::ThreadLoop(Worker* this_worker) {
@@ -872,8 +889,10 @@ SequencedWorkerPool::Inner::GetWorkStatus SequencedWorkerPool::Inner::GetWork(
std::vector<Closure>* delete_these_outside_lock) {
lock_.AssertAcquired();
+#if !defined(OS_NACL)
UMA_HISTOGRAM_COUNTS_100("SequencedWorkerPool.TaskCount",
static_cast<int>(pending_tasks_.size()));
+#endif
// Find the next task with a sequence token that's not currently in use.
// If the token is in use, that means another thread is running something
@@ -958,8 +977,10 @@ SequencedWorkerPool::Inner::GetWorkStatus SequencedWorkerPool::Inner::GetWork(
// Track the number of tasks we had to skip over to see if we should be
// making this more efficient. If this number ever becomes large or is
// frequently "some", we should consider the optimization above.
+#if !defined(OS_NACL)
UMA_HISTOGRAM_COUNTS_100("SequencedWorkerPool.UnrunnableTaskCount",
unrunnable_tasks);
+#endif
return status;
}
@@ -1088,8 +1109,20 @@ bool SequencedWorkerPool::Inner::CanShutdown() const {
blocking_shutdown_pending_task_count_ == 0;
}
+base::StaticAtomicSequenceNumber
+SequencedWorkerPool::Inner::g_last_sequence_number_;
+
// SequencedWorkerPool --------------------------------------------------------
+// static
+SequencedWorkerPool::SequenceToken
+SequencedWorkerPool::GetSequenceTokenForCurrentThread() {
+ SequencedWorkerPool::SequenceToken* token = g_lazy_tls_ptr.Get().Get();
+ if (!token)
+ return SequenceToken();
+ return *token;
+}
+
SequencedWorkerPool::SequencedWorkerPool(
size_t max_threads,
const std::string& thread_name_prefix)
« no previous file with comments | « base/threading/sequenced_worker_pool.h ('k') | base/threading/sequenced_worker_pool_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698