impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] Allow BlockingQueue and ThreadPool to accept rvalue args
Date Tue, 04 Apr 2017 05:00:33 GMT
Alex Behm 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 to me, but perhaps
I'm missing why moving a shared_ptr is significant. The danger is that if someone stumbles
upon this interesting rvalue reference code with only basic c++11 knowledge (== myself), then
the single use is really confusing, and possibly misleading.

I'm not opposed to this change, but the code does seem more obscure than before. Are you doing
this because you have a future use of the feature in mind?


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


-- 
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