Unfriending classes

Summary:
In this diff I made some effort to reduce usage of friending. To do that, I had to expose Compaction::inputs_ through a method inputs(). Not sure if this is a good idea, there is a trade-off. I think it's less confusing than having lots of friends.

I also thought about other friendship relationships, but they are too much tangled at this point. Once you friend two classes, it's very hard to unfriend them :)

Test Plan: make check

Reviewers: haobo, kailiu, sdong, dhruba

Reviewed By: kailiu

CC: leveldb

Differential Revision: https://reviews.facebook.net/D15267
This commit is contained in:
Igor Canadi 2014-01-22 10:55:16 -08:00
parent 6fe9b57748
commit fb01755aa4
3 changed files with 16 additions and 15 deletions

View File

@ -26,7 +26,7 @@ Compaction::Compaction(Version* input_version, int level, int out_level,
: level_(level), : level_(level),
out_level_(out_level), out_level_(out_level),
max_output_file_size_(target_file_size), max_output_file_size_(target_file_size),
maxGrandParentOverlapBytes_(max_grandparent_overlap_bytes), max_grandparent_overlap_bytes_(max_grandparent_overlap_bytes),
input_version_(input_version), input_version_(input_version),
number_levels_(input_version_->NumberLevels()), number_levels_(input_version_->NumberLevels()),
seek_compaction_(seek_compaction), seek_compaction_(seek_compaction),
@ -64,7 +64,7 @@ bool Compaction::IsTrivialMove() const {
return (level_ != out_level_ && return (level_ != out_level_ &&
num_input_files(0) == 1 && num_input_files(0) == 1 &&
num_input_files(1) == 0 && num_input_files(1) == 0 &&
TotalFileSize(grandparents_) <= maxGrandParentOverlapBytes_); TotalFileSize(grandparents_) <= max_grandparent_overlap_bytes_);
} }
void Compaction::AddInputDeletions(VersionEdit* edit) { void Compaction::AddInputDeletions(VersionEdit* edit) {
@ -117,7 +117,7 @@ bool Compaction::ShouldStopBefore(const Slice& internal_key) {
} }
seen_key_ = true; seen_key_ = true;
if (overlapped_bytes_ > maxGrandParentOverlapBytes_) { if (overlapped_bytes_ > max_grandparent_overlap_bytes_) {
// Too much overlap for current output; start new output // Too much overlap for current output; start new output
overlapped_bytes_ = 0; overlapped_bytes_ = 0;
return true; return true;

View File

@ -33,9 +33,14 @@ class Compaction {
// "which" must be either 0 or 1 // "which" must be either 0 or 1
int num_input_files(int which) const { return inputs_[which].size(); } int num_input_files(int which) const { return inputs_[which].size(); }
// Returns input version of the compaction
Version* input_version() const { return input_version_; }
// Return the ith input file at "level()+which" ("which" must be 0 or 1). // Return the ith input file at "level()+which" ("which" must be 0 or 1).
FileMetaData* input(int which, int i) const { return inputs_[which][i]; } FileMetaData* input(int which, int i) const { return inputs_[which][i]; }
std::vector<FileMetaData*>* inputs(int which) { return &inputs_[which]; }
// Maximum size of files to build during this compaction. // Maximum size of files to build during this compaction.
uint64_t MaxOutputFileSize() const { return max_output_file_size_; } uint64_t MaxOutputFileSize() const { return max_output_file_size_; }
@ -74,8 +79,6 @@ class Compaction {
bool IsFullCompaction() { return is_full_compaction_; } bool IsFullCompaction() { return is_full_compaction_; }
private: private:
friend class Version;
friend class VersionSet;
friend class CompactionPicker; friend class CompactionPicker;
friend class UniversalCompactionPicker; friend class UniversalCompactionPicker;
friend class LevelCompactionPicker; friend class LevelCompactionPicker;
@ -87,7 +90,7 @@ class Compaction {
int level_; int level_;
int out_level_; // levels to which output files are stored int out_level_; // levels to which output files are stored
uint64_t max_output_file_size_; uint64_t max_output_file_size_;
uint64_t maxGrandParentOverlapBytes_; uint64_t max_grandparent_overlap_bytes_;
Version* input_version_; Version* input_version_;
VersionEdit* edit_; VersionEdit* edit_;
int number_levels_; int number_levels_;

View File

@ -1994,23 +1994,21 @@ Iterator* VersionSet::MakeInputIterator(Compaction* c) {
// Level-0 files have to be merged together. For other levels, // Level-0 files have to be merged together. For other levels,
// we will make a concatenating iterator per level. // we will make a concatenating iterator per level.
// TODO(opt): use concatenating iterator for level-0 if there is no overlap // TODO(opt): use concatenating iterator for level-0 if there is no overlap
const int space = (c->level() == 0 ? c->inputs_[0].size() + 1 : 2); const int space = (c->level() == 0 ? c->inputs(0)->size() + 1 : 2);
Iterator** list = new Iterator*[space]; Iterator** list = new Iterator*[space];
int num = 0; int num = 0;
for (int which = 0; which < 2; which++) { for (int which = 0; which < 2; which++) {
if (!c->inputs_[which].empty()) { if (!c->inputs(which)->empty()) {
if (c->level() + which == 0) { if (c->level() + which == 0) {
const std::vector<FileMetaData*>& files = c->inputs_[which]; for (const auto& file : *c->inputs(which)) {
for (size_t i = 0; i < files.size(); i++) {
list[num++] = table_cache_->NewIterator( list[num++] = table_cache_->NewIterator(
options, storage_options_compactions_, options, storage_options_compactions_, file->number,
files[i]->number, files[i]->file_size, nullptr, file->file_size, nullptr, true /* for compaction */);
true /* for compaction */);
} }
} else { } else {
// Create concatenating iterator for the files from this level // Create concatenating iterator for the files from this level
list[num++] = NewTwoLevelIterator( list[num++] = NewTwoLevelIterator(
new Version::LevelFileNumIterator(icmp_, &c->inputs_[which]), new Version::LevelFileNumIterator(icmp_, c->inputs(which)),
&GetFileIterator, table_cache_, options, storage_options_, &GetFileIterator, table_cache_, options, storage_options_,
true /* for compaction */); true /* for compaction */);
} }
@ -2034,7 +2032,7 @@ uint64_t VersionSet::MaxFileSizeForLevel(int level) {
// in the current version // in the current version
bool VersionSet::VerifyCompactionFileConsistency(Compaction* c) { bool VersionSet::VerifyCompactionFileConsistency(Compaction* c) {
#ifndef NDEBUG #ifndef NDEBUG
if (c->input_version_ != current_) { if (c->input_version() != current_) {
Log(options_->info_log, "VerifyCompactionFileConsistency version mismatch"); Log(options_->info_log, "VerifyCompactionFileConsistency version mismatch");
} }