Adar Dembo has posted comments on this change.

Change subject: Remove miscellaneous boost dependencies
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3490/2/src/kudu/cfile/encoding-test.cc
File src/kudu/cfile/encoding-test.cc:

Line 76:     vector<string> to_insert;
I'm going to try to summarize what's going on here for anyone who finds this 
confusing.

The old implementation used boost::ptr_vector because, if it had used a 
pre-C++11 std::vector, resizing the vector (via push_back()) would create new 
strings and use copy constructors to copy the string data to them, invalidating 
the generated slices. Reference-counted strings would save the day, but that's 
an implementation detail that should not be relied on (and cannot be relied on 
when using the new gcc5 ABI or libc++ on OS X).

With C++11, resizing ought to move the strings, not copy them, so using a 
vector is safe.

Alternatively, we could use a regular array here, at which point it's very 
clear that there's never any resizing going on. I don't really care one way or 
the other though.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0fb6e90e765082cdfde3175f504cf8512fd2cc4f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

Reply via email to