Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16063 )
Change subject: [util] improved performance of BlockingQueue ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/16063/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16063/3//COMMIT_MSG@10 PS3, Line 10: Switched from std::list to std::deque for the underlying : queue. What's the reason for the switch? http://gerrit.cloudera.org:8080/#/c/16063/3/src/kudu/util/blocking_queue.h File src/kudu/util/blocking_queue.h: http://gerrit.cloudera.org:8080/#/c/16063/3/src/kudu/util/blocking_queue.h@192 PS3, Line 192: if (PREDICT_FALSE(deadline.Initialized() && MonoTime::Now() > deadline)) { : return Status::TimedOut(""); : } : MutexLock l(lock_); : while (true) { : if (PREDICT_FALSE(shutdown_)) { : return Status::Aborted(""); : } : if (size_ < max_size_) { : increment_size_unlocked(val); : queue_.emplace_back(val); : l.Unlock(); : not_empty_.Signal(); : return Status::OK(); : } : if (!deadline.Initialized()) { : not_full_.Wait(); : } else if (PREDICT_FALSE(!not_full_.WaitUntil(deadline))) { : return Status::TimedOut(""); : } : } > Isn't it safe to pass a const ref as an r-value? E.g. why not just wrap the My understanding is that before this change both rvalue and const ref would bind to this function and now the rvalue will bind to the rvalue specific function. So earlier a copy will be created for both rvalue and const ref and with this implementation no copy for rvalue. Having said that your concern appears more about how do we reduce code duplication across these functions. Though a rvalue will bind to a const ref, I don't think vice-versa is true. So invoking const ref function from rvalue function will work but lose the benefit of avoiding the copy(the entire point of adding rvalue variant function). Wrapping the other way round won't work. Perhaps using a macro will help avoid code duplication. -- To view, visit http://gerrit.cloudera.org:8080/16063 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie80620e5e86cd72c29320096dcdcc712eea1b0f2 Gerrit-Change-Number: 16063 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 11 Jun 2020 18:47:37 +0000 Gerrit-HasComments: Yes
