[kudu-CR] replace boost::detail::yield with std::this thread::yield

2016-06-24 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: replace boost::detail::yield with std::this_thread::yield
..


Patch Set 1:

> I dont think this is a good idea - we use boost yield in some cases
 > where we might spin a pretty long time and spinning on
 > sched_yield() would suck up lots of kernel CPU (it reschedules the
 > same thread if there's nothing else to run)

It doesn't seem like it'd be too much work to implement our own adaptive yield. 
The boost implementation is small.

-- 
To view, visit http://gerrit.cloudera.org:8080/3492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1c94f8377982028e608c0b0d0b56a48f8df4dfe2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] replace boost::detail::yield with std::this thread::yield

2016-06-24 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: replace boost::detail::yield with std::this_thread::yield
..


Patch Set 1: Code-Review-1

I dont think this is a good idea - we use boost yield in some cases where we 
might spin a pretty long time and spinning on sched_yield() would suck up lots 
of kernel CPU (it reschedules the same thread if there's nothing else to run)

-- 
To view, visit http://gerrit.cloudera.org:8080/3492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1c94f8377982028e608c0b0d0b56a48f8df4dfe2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] replace boost::detail::yield with std::this thread::yield

2016-06-24 Thread Dan Burkert (Code Review)
Dan Burkert has posted comments on this change.

Change subject: replace boost::detail::yield with std::this_thread::yield
..


Patch Set 1:

This isn't entirely equivalent since I removed the spin counters.  We could put 
those back in, but it's more effort.  Not sure exactly how boost uses them.

-- 
To view, visit http://gerrit.cloudera.org:8080/3492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1c94f8377982028e608c0b0d0b56a48f8df4dfe2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] replace boost::detail::yield with std::this thread::yield

2016-06-24 Thread Dan Burkert (Code Review)
Hello Adar Dembo,

I'd like you to do a code review.  Please visit

http://gerrit.cloudera.org:8080/3492

to review the following change.

Change subject: replace boost::detail::yield with std::this_thread::yield
..

replace boost::detail::yield with std::this_thread::yield

source of boost::detail::yield:
https://github.com/boostorg/smart_ptr/blob/boost-1.59.0/include/boost/smart_ptr/detail/yield_k.hpp#L116-L144

source of std::this_thread::yield:
https://github.com/gcc-mirror/gcc/blob/gcc_5_3_0_release/libstdc%2B%2B-v3/include/std/thread#L265-L272
(boils down to a call to sched_yield(2)).

Change-Id: I1c94f8377982028e608c0b0d0b56a48f8df4dfe2
---
M src/kudu/tablet/concurrent_btree.h
M src/kudu/util/rw_semaphore.h
M src/kudu/util/thread.cc
M src/kudu/util/threadpool-test.cc
4 files changed, 14 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/3492/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1c94f8377982028e608c0b0d0b56a48f8df4dfe2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 


[kudu-CR] replace boost::detail::yield with std::this thread::yield

2016-06-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: replace boost::detail::yield with std::this_thread::yield
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/1986/

-- 
To view, visit http://gerrit.cloudera.org:8080/3492
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1c94f8377982028e608c0b0d0b56a48f8df4dfe2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No