Adar Dembo has posted comments on this change. Change subject: Reduce arguments in file Read API ......................................................................
Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/6770/3/src/kudu/util/env.h File src/kudu/util/env.h: PS3, Line 355: virtual Status Read(Slice* result) > Hmm... I am not sure I understand the purpose of changing the function sign I think the primary motivation for this refactor was the new ReadV() API that Grant is working on. ReadV() operates on multiple caller-provided buffers, which means a list of (scratch, length, result slice) tuples. It's tough to reason about ReadV()'s IN and OUT parameter semantics since you have to consider both these tuples and the list of tuples itself. On the other hand, I think an IN/OUT list of IN/OUT slices is a little simpler. Outside of ReadV(), doing away with short reads means there is only one case where a Slice member is updated by Read(), and that's here in SequentialFile (since a need to deal with EOF). In all other cases, the Slice _buffer_ is written to, but the Slice _members_ aren't changed at all. So Slice acts more like an IN parameter than a true IN/OUT one. FWIW, memory-mapped file reading was removed Dec 2015 (commit a26e074), so resetting a Slice for zero-copy is no longer a concern. -- To view, visit http://gerrit.cloudera.org:8080/6770 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3e4df425a7387c28eb428e851a9f8c25167755d4 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
