Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11302 )

Change subject: util: add once class based on std::call_once
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11302/1/src/kudu/util/once.h
File src/kudu/util/once.h:

http://gerrit.cloudera.org:8080/#/c/11302/1/src/kudu/util/once.h@132
PS1, Line 132:   size_t memory_footprint_excluding_this() const;
> It could, but it would be unused as of this patch. Would you prefer I add i
I guess I thought that if this was a drop-in replacement for KuduOnceDynamic, 
you'd have to provide it too. But maybe we're not heap allocating 
KuduOnceDynamic in the places you switched over.

In any case, after thinking about it some more, you should add it now. I'm 
worried that someone who looks at KuduOnceLambda but doesn't look at 
KuduOnceDynamic in detail will see the one function, not understand the nuanced 
difference, and use it even on a heap-allocated KuduOnceLambda.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide56057a800bf07923031df5a2a76a42f0b15358
Gerrit-Change-Number: 11302
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 24 Aug 2018 21:14:07 +0000
Gerrit-HasComments: Yes

Reply via email to