andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Remove some unnecessary constructors

Browse Source 
Summary:
This is continuing the work done by 27b22f13a3

It's just cleaning up some unnecessary constructors. The most important change is removing Block::Block(const BlockContents& contents) constructor. It was only used from the unit test.

Test Plan: compiles

Reviewers: sdong, yhchiang, ljin

Reviewed By: ljin

Subscribers: leveldb

Differential Revision: https://reviews.facebook.net/D23547
This commit is contained in:
Igor Canadi 2014-09-17 16:45:58 -07:00
parent feadb9df53
commit ff76895614
6 changed files with 34 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
table/block.cc
View File

@ -297,8 +297,10 @@ uint32_t Block::NumRestarts() const {
return DecodeFixed32(data_ + size_ - sizeof(uint32_t)); return DecodeFixed32(data_ + size_ - sizeof(uint32_t));
} }
Block::Block(const BlockContents& contents) Block::Block(BlockContents&& contents)
: data_(contents.data.data()), size_(contents.data.size()) { : contents_(std::move(contents)),
data_(contents_.data.data()),
size_(contents_.data.size()) {
if (size_ < sizeof(uint32_t)) { if (size_ < sizeof(uint32_t)) {
size_ = 0; // Error marker size_ = 0; // Error marker
} else { } else {
@ -311,10 +313,6 @@ Block::Block(const BlockContents& contents)
} }
} }
Block::Block(BlockContents&& contents) : Block(contents) {
contents_ = std::move(contents);
}
Iterator* Block::NewIterator( Iterator* Block::NewIterator(
const Comparator* cmp, BlockIter* iter, bool total_order_seek) { const Comparator* cmp, BlockIter* iter, bool total_order_seek) {
if (size_ < 2*sizeof(uint32_t)) { if (size_ < 2*sizeof(uint32_t)) {

5
table/block.h
View File

@ -31,7 +31,6 @@ class Block {
public: public:
// Initialize the block with the specified contents. // Initialize the block with the specified contents.
explicit Block(BlockContents&& contents); explicit Block(BlockContents&& contents);
explicit Block(const BlockContents& contents);
~Block() = default; ~Block() = default;
@ -66,8 +65,8 @@ class Block {
private: private:
BlockContents contents_; BlockContents contents_;
const char* data_; const char* data_; // contents_.data.data()
size_t size_; size_t size_; // contents_.data.size()
uint32_t restart_offset_; // Offset in data_ of restart array uint32_t restart_offset_; // Offset in data_ of restart array
std::unique_ptr<BlockHashIndex> hash_index_; std::unique_ptr<BlockHashIndex> hash_index_;
std::unique_ptr<BlockPrefixIndex> prefix_index_; std::unique_ptr<BlockPrefixIndex> prefix_index_;

20
table/block_based_filter_block.cc
View File

@ -137,32 +137,26 @@ void BlockBasedFilterBlockBuilder::GenerateFilter() {
BlockBasedFilterBlockReader::BlockBasedFilterBlockReader( BlockBasedFilterBlockReader::BlockBasedFilterBlockReader(
const SliceTransform* prefix_extractor, const SliceTransform* prefix_extractor,
const BlockBasedTableOptions& table_opt, const Slice& contents) const BlockBasedTableOptions& table_opt, BlockContents&& contents)
: policy_(table_opt.filter_policy.get()), : policy_(table_opt.filter_policy.get()),
prefix_extractor_(prefix_extractor), prefix_extractor_(prefix_extractor),
whole_key_filtering_(table_opt.whole_key_filtering), whole_key_filtering_(table_opt.whole_key_filtering),
data_(nullptr), data_(nullptr),
offset_(nullptr), offset_(nullptr),
num_(0), num_(0),
base_lg_(0) { base_lg_(0),
contents_(std::move(contents)) {
assert(policy_); assert(policy_);
size_t n = contents.size(); size_t n = contents_.data.size();
if (n < 5) return; // 1 byte for base_lg_ and 4 for start of offset array if (n < 5) return; // 1 byte for base_lg_ and 4 for start of offset array
base_lg_ = contents[n - 1]; base_lg_ = contents_.data[n - 1];
uint32_t last_word = DecodeFixed32(contents.data() + n - 5); uint32_t last_word = DecodeFixed32(contents_.data.data() + n - 5);
if (last_word > n - 5) return; if (last_word > n - 5) return;
data_ = contents.data(); data_ = contents_.data.data();
offset_ = data_ + last_word; offset_ = data_ + last_word;
num_ = (n - 5 - last_word) / 4; num_ = (n - 5 - last_word) / 4;
} }
BlockBasedFilterBlockReader::BlockBasedFilterBlockReader(
const SliceTransform* prefix_extractor,
const BlockBasedTableOptions& table_opt, BlockContents&& contents)
: BlockBasedFilterBlockReader(prefix_extractor, table_opt, contents.data) {
contents_ = std::move(contents);
}
bool BlockBasedFilterBlockReader::KeyMayMatch(const Slice& key, bool BlockBasedFilterBlockReader::KeyMayMatch(const Slice& key,
uint64_t block_offset) { uint64_t block_offset) {
assert(block_offset != kNotValid); assert(block_offset != kNotValid);

3
table/block_based_filter_block.h
View File

@ -72,9 +72,6 @@ class BlockBasedFilterBlockBuilder : public FilterBlockBuilder {
class BlockBasedFilterBlockReader : public FilterBlockReader { class BlockBasedFilterBlockReader : public FilterBlockReader {
public: public:
// REQUIRES: "contents" and *policy must stay live while *this is live. // REQUIRES: "contents" and *policy must stay live while *this is live.
BlockBasedFilterBlockReader(const SliceTransform* prefix_extractor,
const BlockBasedTableOptions& table_opt,
const Slice& contents);
BlockBasedFilterBlockReader(const SliceTransform* prefix_extractor, BlockBasedFilterBlockReader(const SliceTransform* prefix_extractor,
const BlockBasedTableOptions& table_opt, const BlockBasedTableOptions& table_opt,
BlockContents&& contents); BlockContents&& contents);

28
table/block_based_filter_block_test.cc
View File

@ -55,9 +55,9 @@ class FilterBlockTest {
TEST(FilterBlockTest, EmptyBuilder) { TEST(FilterBlockTest, EmptyBuilder) {
BlockBasedFilterBlockBuilder builder(nullptr, table_options_); BlockBasedFilterBlockBuilder builder(nullptr, table_options_);
Slice block = builder.Finish(); BlockContents block(builder.Finish(), false, kNoCompression);
ASSERT_EQ("\\x00\\x00\\x00\\x00\\x0b", EscapeString(block)); ASSERT_EQ("\\x00\\x00\\x00\\x00\\x0b", EscapeString(block.data));
BlockBasedFilterBlockReader reader(nullptr, table_options_, block); BlockBasedFilterBlockReader reader(nullptr, table_options_, std::move(block));
ASSERT_TRUE(reader.KeyMayMatch("foo", 0)); ASSERT_TRUE(reader.KeyMayMatch("foo", 0));
ASSERT_TRUE(reader.KeyMayMatch("foo", 100000)); ASSERT_TRUE(reader.KeyMayMatch("foo", 100000));
} }
@ -72,8 +72,8 @@ TEST(FilterBlockTest, SingleChunk) {
builder.Add("box"); builder.Add("box");
builder.StartBlock(300); builder.StartBlock(300);
builder.Add("hello"); builder.Add("hello");
Slice block = builder.Finish(); BlockContents block(builder.Finish(), false, kNoCompression);
BlockBasedFilterBlockReader reader(nullptr, table_options_, block); BlockBasedFilterBlockReader reader(nullptr, table_options_, std::move(block));
ASSERT_TRUE(reader.KeyMayMatch("foo", 100)); ASSERT_TRUE(reader.KeyMayMatch("foo", 100));
ASSERT_TRUE(reader.KeyMayMatch("bar", 100)); ASSERT_TRUE(reader.KeyMayMatch("bar", 100));
ASSERT_TRUE(reader.KeyMayMatch("box", 100)); ASSERT_TRUE(reader.KeyMayMatch("box", 100));
@ -103,8 +103,8 @@ TEST(FilterBlockTest, MultiChunk) {
builder.Add("box"); builder.Add("box");
builder.Add("hello"); builder.Add("hello");
Slice block = builder.Finish(); BlockContents block(builder.Finish(), false, kNoCompression);
BlockBasedFilterBlockReader reader(nullptr, table_options_, block); BlockBasedFilterBlockReader reader(nullptr, table_options_, std::move(block));
// Check first filter // Check first filter
ASSERT_TRUE(reader.KeyMayMatch("foo", 0)); ASSERT_TRUE(reader.KeyMayMatch("foo", 0));
@ -147,10 +147,10 @@ class BlockBasedFilterBlockTest {
TEST(BlockBasedFilterBlockTest, BlockBasedEmptyBuilder) { TEST(BlockBasedFilterBlockTest, BlockBasedEmptyBuilder) {
FilterBlockBuilder* builder = new BlockBasedFilterBlockBuilder( FilterBlockBuilder* builder = new BlockBasedFilterBlockBuilder(
nullptr, table_options_); nullptr, table_options_);
Slice block = builder->Finish(); BlockContents block(builder->Finish(), false, kNoCompression);
ASSERT_EQ("\\x00\\x00\\x00\\x00\\x0b", EscapeString(block)); ASSERT_EQ("\\x00\\x00\\x00\\x00\\x0b", EscapeString(block.data));
FilterBlockReader* reader = new BlockBasedFilterBlockReader( FilterBlockReader* reader = new BlockBasedFilterBlockReader(
nullptr, table_options_, block); nullptr, table_options_, std::move(block));
ASSERT_TRUE(reader->KeyMayMatch("foo", 0)); ASSERT_TRUE(reader->KeyMayMatch("foo", 0));
ASSERT_TRUE(reader->KeyMayMatch("foo", 100000)); ASSERT_TRUE(reader->KeyMayMatch("foo", 100000));
@ -169,9 +169,9 @@ TEST(BlockBasedFilterBlockTest, BlockBasedSingleChunk) {
builder->Add("box"); builder->Add("box");
builder->StartBlock(300); builder->StartBlock(300);
builder->Add("hello"); builder->Add("hello");
Slice block = builder->Finish(); BlockContents block(builder->Finish(), false, kNoCompression);
FilterBlockReader* reader = new BlockBasedFilterBlockReader( FilterBlockReader* reader = new BlockBasedFilterBlockReader(
nullptr, table_options_, block); nullptr, table_options_, std::move(block));
ASSERT_TRUE(reader->KeyMayMatch("foo", 100)); ASSERT_TRUE(reader->KeyMayMatch("foo", 100));
ASSERT_TRUE(reader->KeyMayMatch("bar", 100)); ASSERT_TRUE(reader->KeyMayMatch("bar", 100));
ASSERT_TRUE(reader->KeyMayMatch("box", 100)); ASSERT_TRUE(reader->KeyMayMatch("box", 100));
@ -205,9 +205,9 @@ TEST(BlockBasedFilterBlockTest, BlockBasedMultiChunk) {
builder->Add("box"); builder->Add("box");
builder->Add("hello"); builder->Add("hello");
Slice block = builder->Finish(); BlockContents block(builder->Finish(), false, kNoCompression);
FilterBlockReader* reader = new BlockBasedFilterBlockReader( FilterBlockReader* reader = new BlockBasedFilterBlockReader(
nullptr, table_options_, block); nullptr, table_options_, std::move(block));
// Check first filter // Check first filter
ASSERT_TRUE(reader->KeyMayMatch("foo", 0)); ASSERT_TRUE(reader->KeyMayMatch("foo", 0));

12
table/block_test.cc
View File

@ -146,13 +146,15 @@ BlockContents GetBlockContents(std::unique_ptr<BlockBuilder> *builder,
return contents; return contents;
} }
void CheckBlockContents(const BlockContents &contents, const int max_key, void CheckBlockContents(BlockContents contents, const int max_key,
const std::vector<std::string> &keys, const std::vector<std::string> &keys,
const std::vector<std::string> &values) { const std::vector<std::string> &values) {
const size_t prefix_size = 6; const size_t prefix_size = 6;
// create block reader // create block reader
Block reader1(contents); BlockContents contents_ref(contents.data, contents.cachable,
Block reader2(contents); contents.compression_type);
Block reader1(std::move(contents));
Block reader2(std::move(contents_ref));
std::unique_ptr<const SliceTransform> prefix_extractor( std::unique_ptr<const SliceTransform> prefix_extractor(
NewFixedPrefixTransform(prefix_size)); NewFixedPrefixTransform(prefix_size));
@ -210,7 +212,7 @@ TEST(BlockTest, SimpleIndexHash) {
std::unique_ptr<BlockBuilder> builder; std::unique_ptr<BlockBuilder> builder;
auto contents = GetBlockContents(&builder, keys, values); auto contents = GetBlockContents(&builder, keys, values);
CheckBlockContents(contents, kMaxKey, keys, values); CheckBlockContents(std::move(contents), kMaxKey, keys, values);
} }
TEST(BlockTest, IndexHashWithSharedPrefix) { TEST(BlockTest, IndexHashWithSharedPrefix) {
@ -229,7 +231,7 @@ TEST(BlockTest, IndexHashWithSharedPrefix) {
std::unique_ptr<BlockBuilder> builder; std::unique_ptr<BlockBuilder> builder;
auto contents = GetBlockContents(&builder, keys, values, kPrefixGroup); auto contents = GetBlockContents(&builder, keys, values, kPrefixGroup);
CheckBlockContents(contents, kMaxKey, keys, values); CheckBlockContents(std::move(contents), kMaxKey, keys, values);
} }
} // namespace rocksdb } // namespace rocksdb