impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] Allow BlockingQueue and ThreadPool to accept rvalue args
Date Tue, 04 Apr 2017 19:59:21 GMT
Henry Robinson has posted comments on this change.

Change subject: Allow BlockingQueue and ThreadPool to accept rvalue args
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6442/1//COMMIT_MSG
Commit Message:

Line 11: queue. Very often we create a thin wrapper for each work item we submit
> This CR uses it in only one place where it doesn't seem to make much sense 
The shared_ptr example is illustrative, but does also have some small effect: the shared_ptr
is no longer copied so the ref count changes and synchronization that would have to happen
with a copy are elided. 

I do have a change that uses this for a non-copyable object. But I also think that this is
a natural improvement for the API in that copy c'tors won't be silently called where it's
avoidable.

Note that some callsites of this API will have their behavior changed with no code changes.
For example, Coordinator::UpdateFilter() uses ThreadPool::Offer() with a boost::function that
is an rvalue. With this patch the function is moved into the queue and not copied. That could
be significant when the function has large argument objects.


http://gerrit.cloudera.org:8080/#/c/6442/1/be/src/util/blocking-queue.h
File be/src/util/blocking-queue.h:

Line 198:   /// Overloads for inserting an item into the list, depending whether it should
be moved
> depending on
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1791870576cb269e86495034f92555de48f92f10
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message