This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 24797ffd3eab3345696383820f6e8ad8a093e76c Author: Alexey Serbin <[email protected]> AuthorDate: Fri Jun 9 17:48:23 2023 -0700 [cfile] replace BlockPointer::Equals() with operator==() The motivation for this change is a realisation that methods like Equals() look a bit funny in C++ code since the language supports much more expressive and readable notation for comparing objects. I also did a minor cleanup to bring the code in block_pointer.h to the style guide and removed unused instances of the BlockPointer class elsewhere in the code. This patch doesn't contain any functional changes. Change-Id: I3b2c05931ce4b678ef184d0c812cf0fb51b650c7 Reviewed-on: http://gerrit.cloudera.org:8080/20031 Tested-by: Alexey Serbin <[email protected]> Reviewed-by: Abhishek Chennaka <[email protected]> --- src/kudu/cfile/block_pointer.h | 46 ++++++++++++++++++++++-------------------- src/kudu/cfile/bloomfile.cc | 2 +- src/kudu/cfile/cfile-test.cc | 4 +--- src/kudu/cfile/index_btree.cc | 1 - 4 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/kudu/cfile/block_pointer.h b/src/kudu/cfile/block_pointer.h index 885a41aa4..d9989bf8c 100644 --- a/src/kudu/cfile/block_pointer.h +++ b/src/kudu/cfile/block_pointer.h @@ -14,10 +14,10 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. -#ifndef KUDU_CFILE_BLOCK_POINTER_H -#define KUDU_CFILE_BLOCK_POINTER_H -#include <stdio.h> +#pragma once + +#include <cstdio> #include <string> #include "kudu/cfile/cfile.pb.h" @@ -31,31 +31,32 @@ namespace cfile { class BlockPointer { public: - BlockPointer() {} - BlockPointer(const BlockPointer &from) : - offset_(from.offset_), - size_(from.size_) {} - - explicit BlockPointer(const BlockPointerPB &from) : - offset_(from.offset()), - size_(from.size()) { + BlockPointer() + : offset_(0), + size_(0) { + } + + constexpr BlockPointer(uint64_t offset, uint64_t size) + : offset_(offset), + size_(size) { } - BlockPointer(uint64_t offset, uint64_t size) : - offset_(offset), - size_(size) {} + explicit BlockPointer(const BlockPointerPB& from) + : offset_(from.offset()), + size_(from.size()) { + } std::string ToString() const { return strings::Substitute("offset=$0 size=$1", offset_, size_); } template<class StrType> - void EncodeTo(StrType *s) const { + void EncodeTo(StrType* s) const { PutVarint64(s, offset_); InlinePutVarint32(s, size_); } - Status DecodeFrom(const uint8_t *data, const uint8_t *limit) { + Status DecodeFrom(const uint8_t* data, const uint8_t* limit) { data = GetVarint64Ptr(data, limit, &offset_); if (!data) { return Status::Corruption("bad block pointer"); @@ -69,29 +70,30 @@ class BlockPointer { return Status::OK(); } - void CopyToPB(BlockPointerPB *pb) const { + void CopyToPB(BlockPointerPB* pb) const { pb->set_offset(offset_); pb->set_size(size_); } - uint64_t offset() const { + constexpr uint64_t offset() const { return offset_; } - uint32_t size() const { + constexpr uint32_t size() const { return size_; } - bool Equals(const BlockPointer& other) const { + constexpr bool operator==(const BlockPointer& other) const { return offset_ == other.offset_ && size_ == other.size_; } + constexpr bool operator!=(const BlockPointer& other) const { + return !(*this == other); + } private: uint64_t offset_; uint32_t size_; }; - } // namespace cfile } // namespace kudu -#endif diff --git a/src/kudu/cfile/bloomfile.cc b/src/kudu/cfile/bloomfile.cc index a1cffcd46..3bcdd32b9 100644 --- a/src/kudu/cfile/bloomfile.cc +++ b/src/kudu/cfile/bloomfile.cc @@ -323,7 +323,7 @@ Status BloomFileReader::CheckKeyPresent(const BloomKeyProbe &probe, // If the previous lookup from this bloom on this thread seeked to a different // block in the BloomFile, we need to read the correct block and re-hydrate the // BloomFilter instance. - if (!bci->cur_block_pointer.Equals(bblk_ptr)) { + if (bblk_ptr != bci->cur_block_pointer) { scoped_refptr<BlockHandle> dblk_data; RETURN_NOT_OK(reader_->ReadBlock(io_context, bblk_ptr, CFileReader::CACHE_BLOCK, &dblk_data)); diff --git a/src/kudu/cfile/cfile-test.cc b/src/kudu/cfile/cfile-test.cc index bd79ba833..cfb19f367 100644 --- a/src/kudu/cfile/cfile-test.cc +++ b/src/kudu/cfile/cfile-test.cc @@ -25,6 +25,7 @@ #include <memory> #include <sstream> #include <string> +#include <type_traits> #include <vector> #include <gflags/gflags.h> @@ -122,7 +123,6 @@ class TestCFile : public CFileTestBase { unique_ptr<CFileReader> reader; ASSERT_OK(CFileReader::Open(std::move(block), ReaderOptions(), &reader)); - BlockPointer ptr; unique_ptr<CFileIterator> iter; ASSERT_OK(reader->NewIterator(&iter, CFileReader::CACHE_BLOCK, nullptr)); @@ -634,8 +634,6 @@ void TestCFile::TestReadWriteStrings(EncodingType encoding, ASSERT_OK(reader->CountRows(&reader_nrows)); ASSERT_EQ(nrows, reader_nrows); - BlockPointer ptr; - unique_ptr<CFileIterator> iter; ASSERT_OK(reader->NewIterator(&iter, CFileReader::CACHE_BLOCK, nullptr)); diff --git a/src/kudu/cfile/index_btree.cc b/src/kudu/cfile/index_btree.cc index 63b5ce602..06963b529 100644 --- a/src/kudu/cfile/index_btree.cc +++ b/src/kudu/cfile/index_btree.cc @@ -83,7 +83,6 @@ Status IndexTreeBuilder::Append( size_t est_size = idx_block->EstimateEncodedSize(); if (est_size > options_->index_block_size && idx_block->count() > 1) { - BlockPointer index_block_ptr; RETURN_NOT_OK(FinishBlockAndPropagate(level)); }
