[cros] Add RecursiveLock for CrOS implementation. (issue 11361057)

28 views
Skip to first unread message

iva...@chromium.org

unread,
Nov 2, 2012, 5:09:45 AM11/2/12
to dvy...@chromium.org, chromium...@chromium.org
Reviewers: dvyukov,

Description:
[cros] Add RecursiveLock for CrOS implementation.


BUG=157348


Please review this at https://codereview.chromium.org/11361057/

SVN Base: svn://svn.chromium.org/chrome/trunk/src

Affected files:
A rlz/lib/recursive_lock.h
A rlz/lib/recursive_lock.cc
A + rlz/lib/recursive_lock_unittest.cc
M rlz/rlz.gyp


dvy...@google.com

unread,
Nov 2, 2012, 5:52:20 AM11/2/12
to iva...@chromium.org, dvy...@chromium.org, chromium...@chromium.org
You usually implement recursive mutex as follows. It's much cheaper and
faster.

class RecursiveLock {
public:
RecursiveLock()
: owner_()
, recursion_() {
}

~RecursiveLock() {
}

void Acquire() {
Atomic32 me = PlatformThread::CurrentId();
if (me != NoBarrier_Load(&owner_) {
mtx_.Lock();
assert(recursion_ == 0);
assert(owner_ == 0);
NoBarrier_Store(&owner_, me);
}
recursion_++;
}

void Release() {
assert(&owner_ == PlatformThread::CurrentId());
assert(recursion_ > 0);
if (!--recursion_) {
NoBarrier_Store(&owner_, 0);
mtx_.Unlock();
}
}

private:
Lock mtx_;
Atomic32 owner_;
int recursion_;
};


You code looks correct as well. You do not need volatile for lock_count_,
it's
confusing.


https://codereview.chromium.org/11361057/

iva...@chromium.org

unread,
Nov 2, 2012, 7:00:50 AM11/2/12
to dvy...@chromium.org, dvy...@google.com, rog...@chromium.org, chromium...@chromium.org
Thanks! I like your implementation more, updated.

Adding Roger for OWNERS approval.


https://codereview.chromium.org/11361057/

dvy...@google.com

unread,
Nov 2, 2012, 7:51:12 AM11/2/12
to iva...@chromium.org, dvy...@chromium.org, rog...@chromium.org, chromium...@chromium.org

rog...@chromium.org

unread,
Nov 2, 2012, 4:21:22 PM11/2/12
to iva...@chromium.org, dvy...@chromium.org, dvy...@google.com, chromium...@chromium.org

commi...@chromium.org

unread,
Nov 3, 2012, 9:21:05 AM11/3/12
to iva...@chromium.org, dvy...@chromium.org, dvy...@google.com, rog...@chromium.org, chromium...@chromium.org

commi...@chromium.org

unread,
Nov 3, 2012, 12:41:17 PM11/3/12
to iva...@chromium.org, dvy...@chromium.org, dvy...@google.com, rog...@chromium.org, chromium...@chromium.org
Retried try job too often for step(s) content_browsertests

https://chromiumcodereview.appspot.com/11361057/

commi...@chromium.org

unread,
Nov 6, 2012, 3:29:57 AM11/6/12
to iva...@chromium.org, dvy...@chromium.org, dvy...@google.com, rog...@chromium.org, chromium...@chromium.org

commi...@chromium.org

unread,
Nov 6, 2012, 5:39:03 AM11/6/12
to iva...@chromium.org, dvy...@chromium.org, dvy...@google.com, rog...@chromium.org, chromium...@chromium.org
Reply all
Reply to author
Forward
0 new messages