Commit 82952dfb authored by hubbe@chromium.org's avatar hubbe@chromium.org

make debug mode ~20% faster on linux

I was doing some benchmarking on tests on linux, and noticed that 10-20% of time was
in ThreadCheckerImpl::CalledOnValidThread. I looked into it and found that we're doing
a syscall to get the thread ID, which is not needed on any platform where pthread_t
is already an integer type.  Replacing that syscall with pthread_self() makes this particular
test (End2EndTest.BasicFakeSoftwareVideo) go from 488ms to 374ms, which is a 20% speedup.
While other tests might not use ThreadChecker quite as much, it should still be a pretty
good win across a large set of tests.

Review URL: https://codereview.chromium.org/292873002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272725 0039d316-1c4b-4281-b951-d872f2087c98
parent 5b8f779b
...@@ -13,34 +13,25 @@ ...@@ -13,34 +13,25 @@
namespace base { namespace base {
const PlatformThreadId kNoThreadId = static_cast<PlatformThreadId>(0);
Lock::Lock() : lock_() { Lock::Lock() : lock_() {
owned_by_thread_ = false;
owning_thread_id_ = kNoThreadId;
} }
Lock::~Lock() { Lock::~Lock() {
DCHECK(!owned_by_thread_); DCHECK(owning_thread_ref_.is_null());
DCHECK_EQ(kNoThreadId, owning_thread_id_);
} }
void Lock::AssertAcquired() const { void Lock::AssertAcquired() const {
DCHECK(owned_by_thread_); DCHECK(owning_thread_ref_ == PlatformThread::CurrentRef());
DCHECK_EQ(owning_thread_id_, PlatformThread::CurrentId());
} }
void Lock::CheckHeldAndUnmark() { void Lock::CheckHeldAndUnmark() {
DCHECK(owned_by_thread_); DCHECK(owning_thread_ref_ == PlatformThread::CurrentRef());
DCHECK_EQ(owning_thread_id_, PlatformThread::CurrentId()); owning_thread_ref_ = PlatformThreadRef();
owned_by_thread_ = false;
owning_thread_id_ = kNoThreadId;
} }
void Lock::CheckUnheldAndMark() { void Lock::CheckUnheldAndMark() {
DCHECK(!owned_by_thread_); DCHECK(owning_thread_ref_.is_null());
owned_by_thread_ = true; owning_thread_ref_ = PlatformThread::CurrentRef();
owning_thread_id_ = PlatformThread::CurrentId();
} }
} // namespace base } // namespace base
......
...@@ -80,11 +80,7 @@ class BASE_EXPORT Lock { ...@@ -80,11 +80,7 @@ class BASE_EXPORT Lock {
// All private data is implicitly protected by lock_. // All private data is implicitly protected by lock_.
// Be VERY careful to only access members under that lock. // Be VERY careful to only access members under that lock.
base::PlatformThreadRef owning_thread_ref_;
// Determines validity of owning_thread_id_. Needed as we don't have
// a null owning_thread_id_ value.
bool owned_by_thread_;
base::PlatformThreadId owning_thread_id_;
#endif // NDEBUG #endif // NDEBUG
// Platform specific underlying lock implementation. // Platform specific underlying lock implementation.
......
...@@ -23,12 +23,48 @@ ...@@ -23,12 +23,48 @@
namespace base { namespace base {
// Used for logging. Always an integer value.
#if defined(OS_WIN) #if defined(OS_WIN)
typedef DWORD PlatformThreadId; typedef DWORD PlatformThreadId;
#elif defined(OS_POSIX) #elif defined(OS_POSIX)
typedef pid_t PlatformThreadId; typedef pid_t PlatformThreadId;
#endif #endif
// Used for thread checking and debugging.
// Meant to be as fast as possible.
// These are produced by PlatformThread::CurrentRef(), and used to later
// check if we are on the same thread or not by using ==. These are safe
// to copy between threads, but can't be copied to another process as they
// have no meaning there. Also, the internal identifier can be re-used
// after a thread dies, so a PlatformThreadRef cannot be reliably used
// to distinguish a new thread from an old, dead thread.
class PlatformThreadRef {
public:
#if defined(OS_WIN)
typedef DWORD RefType;
#elif defined(OS_POSIX)
typedef pthread_t RefType;
#endif
PlatformThreadRef()
: id_(0) {
}
explicit PlatformThreadRef(RefType id)
: id_(id) {
}
bool operator==(PlatformThreadRef other) const {
return id_ == other.id_;
}
bool is_null() const {
return id_ == 0;
}
private:
RefType id_;
};
// Used to operate on threads.
class PlatformThreadHandle { class PlatformThreadHandle {
public: public:
#if defined(OS_WIN) #if defined(OS_WIN)
...@@ -101,6 +137,10 @@ class BASE_EXPORT PlatformThread { ...@@ -101,6 +137,10 @@ class BASE_EXPORT PlatformThread {
// Gets the current thread id, which may be useful for logging purposes. // Gets the current thread id, which may be useful for logging purposes.
static PlatformThreadId CurrentId(); static PlatformThreadId CurrentId();
// Gets the current thread reference, which can be used to check if
// we're on the right thread quickly.
static PlatformThreadRef CurrentRef();
// Get the current handle. // Get the current handle.
static PlatformThreadHandle CurrentHandle(); static PlatformThreadHandle CurrentHandle();
......
...@@ -164,7 +164,12 @@ PlatformThreadId PlatformThread::CurrentId() { ...@@ -164,7 +164,12 @@ PlatformThreadId PlatformThread::CurrentId() {
#endif #endif
} }
//static // static
PlatformThreadRef PlatformThread::CurrentRef() {
return PlatformThreadRef(pthread_self());
}
// static
PlatformThreadHandle PlatformThread::CurrentHandle() { PlatformThreadHandle PlatformThread::CurrentHandle() {
return PlatformThreadHandle(pthread_self(), CurrentId()); return PlatformThreadHandle(pthread_self(), CurrentId());
} }
......
...@@ -129,6 +129,11 @@ PlatformThreadId PlatformThread::CurrentId() { ...@@ -129,6 +129,11 @@ PlatformThreadId PlatformThread::CurrentId() {
return GetCurrentThreadId(); return GetCurrentThreadId();
} }
// static
PlatformThreadRef PlatformThread::CurrentRef() {
return PlatformThreadRef(GetCurrentThreadId());
}
// static // static
PlatformThreadHandle PlatformThread::CurrentHandle() { PlatformThreadHandle PlatformThread::CurrentHandle() {
NOTIMPLEMENTED(); // See OpenThread() NOTIMPLEMENTED(); // See OpenThread()
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
namespace base { namespace base {
ThreadCheckerImpl::ThreadCheckerImpl() ThreadCheckerImpl::ThreadCheckerImpl()
: valid_thread_id_(kInvalidThreadId) { : valid_thread_id_() {
EnsureThreadIdAssigned(); EnsureThreadIdAssigned();
} }
...@@ -16,19 +16,19 @@ ThreadCheckerImpl::~ThreadCheckerImpl() {} ...@@ -16,19 +16,19 @@ ThreadCheckerImpl::~ThreadCheckerImpl() {}
bool ThreadCheckerImpl::CalledOnValidThread() const { bool ThreadCheckerImpl::CalledOnValidThread() const {
EnsureThreadIdAssigned(); EnsureThreadIdAssigned();
AutoLock auto_lock(lock_); AutoLock auto_lock(lock_);
return valid_thread_id_ == PlatformThread::CurrentId(); return valid_thread_id_ == PlatformThread::CurrentRef();
} }
void ThreadCheckerImpl::DetachFromThread() { void ThreadCheckerImpl::DetachFromThread() {
AutoLock auto_lock(lock_); AutoLock auto_lock(lock_);
valid_thread_id_ = kInvalidThreadId; valid_thread_id_ = PlatformThreadRef();
} }
void ThreadCheckerImpl::EnsureThreadIdAssigned() const { void ThreadCheckerImpl::EnsureThreadIdAssigned() const {
AutoLock auto_lock(lock_); AutoLock auto_lock(lock_);
if (valid_thread_id_ != kInvalidThreadId) if (valid_thread_id_.is_null()) {
return; valid_thread_id_ = PlatformThread::CurrentRef();
valid_thread_id_ = PlatformThread::CurrentId(); }
} }
} // namespace base } // namespace base
...@@ -35,7 +35,7 @@ class BASE_EXPORT ThreadCheckerImpl { ...@@ -35,7 +35,7 @@ class BASE_EXPORT ThreadCheckerImpl {
mutable base::Lock lock_; mutable base::Lock lock_;
// This is mutable so that CalledOnValidThread can set it. // This is mutable so that CalledOnValidThread can set it.
// It's guarded by |lock_|. // It's guarded by |lock_|.
mutable PlatformThreadId valid_thread_id_; mutable PlatformThreadRef valid_thread_id_;
}; };
} // namespace base } // namespace base
......
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