address code review comments on 5e3aeb5f8e

- reduce string copying in Compaction::Summary
- simplify file number checking in UniversalCompactionStopStyleSimilarSize unit test
This commit is contained in:
Mike Lin 2014-01-25 14:12:24 -08:00
parent 5e3aeb5f8e
commit af7838de36
2 changed files with 21 additions and 22 deletions

View File

@ -194,7 +194,7 @@ static void FileSizeSummary(unsigned long long sz, char* output, int len) {
} }
} }
static void InputSummary(std::vector<FileMetaData*>& files, char* output, static int InputSummary(std::vector<FileMetaData*>& files, char* output,
int len) { int len) {
int write = 0; int write = 0;
for (unsigned int i = 0; i < files.size(); i++) { for (unsigned int i = 0; i < files.size(); i++) {
@ -209,29 +209,37 @@ static void InputSummary(std::vector<FileMetaData*>& files, char* output,
break; break;
write += ret; write += ret;
} }
return write;
} }
void Compaction::Summary(char* output, int len) { void Compaction::Summary(char* output, int len) {
int write = snprintf(output, len, int write = snprintf(output, len,
"Base version %lu Base level %d, seek compaction:%d, inputs:", "Base version %lu Base level %d, seek compaction:%d, inputs: [",
(unsigned long)input_version_->GetVersionNumber(), (unsigned long)input_version_->GetVersionNumber(),
level_, level_,
seek_compaction_); seek_compaction_);
if (write < 0 || write > len) { if (write < 0 || write >= len) {
return; return;
} }
char level_low_summary[1024]; write += InputSummary(inputs_[0], output+write, len-write);
InputSummary(inputs_[0], level_low_summary, sizeof(level_low_summary)); if (write < 0 || write >= len) {
char level_up_summary[1024]; return;
if (inputs_[1].size()) {
InputSummary(inputs_[1], level_up_summary, sizeof(level_up_summary));
} else {
level_up_summary[0] = '\0';
} }
snprintf(output + write, len - write, "[%s],[%s]", write += snprintf(output+write, len-write, "],[");
level_low_summary, level_up_summary); if (write < 0 || write >= len) {
return;
}
if (inputs_[1].size()) {
write += InputSummary(inputs_[1], output+write, len-write);
}
if (write < 0 || write >= len) {
return;
}
snprintf(output+write, len-write, "]");
} }
} // namespace rocksdb } // namespace rocksdb

View File

@ -2051,6 +2051,7 @@ TEST(DBTest, UniversalCompactionStopStyleSimilarSize) {
options.level0_file_num_compaction_trigger = 4; options.level0_file_num_compaction_trigger = 4;
options.compaction_options_universal.size_ratio = 10; options.compaction_options_universal.size_ratio = 10;
options.compaction_options_universal.stop_style = kCompactionStopStyleSimilarSize; options.compaction_options_universal.stop_style = kCompactionStopStyleSimilarSize;
options.num_levels=1;
Reopen(&options); Reopen(&options);
Random rnd(301); Random rnd(301);
@ -2082,9 +2083,6 @@ TEST(DBTest, UniversalCompactionStopStyleSimilarSize) {
// (level0_file_num_compaction_trigger+1)=4 files and should have a big // (level0_file_num_compaction_trigger+1)=4 files and should have a big
// file of size 4. // file of size 4.
ASSERT_EQ(NumTableFilesAtLevel(0), 1); ASSERT_EQ(NumTableFilesAtLevel(0), 1);
for (int i = 1; i < options.num_levels ; i++) {
ASSERT_EQ(NumTableFilesAtLevel(i), 0);
}
// Stage 2: // Stage 2:
// Now we have one file at level 0, with size 4. We also have some data in // Now we have one file at level 0, with size 4. We also have some data in
@ -2116,10 +2114,6 @@ TEST(DBTest, UniversalCompactionStopStyleSimilarSize) {
// Before compaction, we have 4 files at level 0, with size 4, 0.4, 1, 1. // Before compaction, we have 4 files at level 0, with size 4, 0.4, 1, 1.
// After compaction, we should have 3 files, with size 4, 0.4, 2. // After compaction, we should have 3 files, with size 4, 0.4, 2.
ASSERT_EQ(NumTableFilesAtLevel(0), 3); ASSERT_EQ(NumTableFilesAtLevel(0), 3);
for (int i = 1; i < options.num_levels ; i++) {
ASSERT_EQ(NumTableFilesAtLevel(i), 0);
}
// Stage 3: // Stage 3:
// Now we have 3 files at level 0, with size 4, 0.4, 2. Generate one // Now we have 3 files at level 0, with size 4, 0.4, 2. Generate one
// more file at level-0, which should trigger level-0 compaction. // more file at level-0, which should trigger level-0 compaction.
@ -2130,9 +2124,6 @@ TEST(DBTest, UniversalCompactionStopStyleSimilarSize) {
dbfull()->TEST_WaitForCompact(); dbfull()->TEST_WaitForCompact();
// Level-0 compaction is triggered, but no file will be picked up. // Level-0 compaction is triggered, but no file will be picked up.
ASSERT_EQ(NumTableFilesAtLevel(0), 4); ASSERT_EQ(NumTableFilesAtLevel(0), 4);
for (int i = 1; i < options.num_levels ; i++) {
ASSERT_EQ(NumTableFilesAtLevel(i), 0);
}
} }
#if defined(SNAPPY) && defined(ZLIB) && defined(BZIP2) #if defined(SNAPPY) && defined(ZLIB) && defined(BZIP2)