From af7838de36fd54dd7bb02969a2e4b47215e9cb05 Mon Sep 17 00:00:00 2001 From: Mike Lin Date: Sat, 25 Jan 2014 14:12:24 -0800 Subject: [PATCH] address code review comments on 5e3aeb5f8e - reduce string copying in Compaction::Summary - simplify file number checking in UniversalCompactionStopStyleSimilarSize unit test --- db/compaction.cc | 30 +++++++++++++++++++----------- db/db_test.cc | 11 +---------- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/db/compaction.cc b/db/compaction.cc index 8a843b650..ce3cd8627 100644 --- a/db/compaction.cc +++ b/db/compaction.cc @@ -194,7 +194,7 @@ static void FileSizeSummary(unsigned long long sz, char* output, int len) { } } -static void InputSummary(std::vector& files, char* output, +static int InputSummary(std::vector& files, char* output, int len) { int write = 0; for (unsigned int i = 0; i < files.size(); i++) { @@ -209,29 +209,37 @@ static void InputSummary(std::vector& files, char* output, break; write += ret; } + return write; } void Compaction::Summary(char* output, int 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(), level_, seek_compaction_); - if (write < 0 || write > len) { + if (write < 0 || write >= len) { + return; + } + + write += InputSummary(inputs_[0], output+write, len-write); + if (write < 0 || write >= len) { + return; + } + + write += snprintf(output+write, len-write, "],["); + if (write < 0 || write >= len) { return; } - char level_low_summary[1024]; - InputSummary(inputs_[0], level_low_summary, sizeof(level_low_summary)); - char level_up_summary[1024]; if (inputs_[1].size()) { - InputSummary(inputs_[1], level_up_summary, sizeof(level_up_summary)); - } else { - level_up_summary[0] = '\0'; + write += InputSummary(inputs_[1], output+write, len-write); + } + if (write < 0 || write >= len) { + return; } - snprintf(output + write, len - write, "[%s],[%s]", - level_low_summary, level_up_summary); + snprintf(output+write, len-write, "]"); } } // namespace rocksdb diff --git a/db/db_test.cc b/db/db_test.cc index 9953cc2f7..3a46282e2 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -2051,6 +2051,7 @@ TEST(DBTest, UniversalCompactionStopStyleSimilarSize) { options.level0_file_num_compaction_trigger = 4; options.compaction_options_universal.size_ratio = 10; options.compaction_options_universal.stop_style = kCompactionStopStyleSimilarSize; + options.num_levels=1; Reopen(&options); Random rnd(301); @@ -2082,9 +2083,6 @@ TEST(DBTest, UniversalCompactionStopStyleSimilarSize) { // (level0_file_num_compaction_trigger+1)=4 files and should have a big // file of size 4. ASSERT_EQ(NumTableFilesAtLevel(0), 1); - for (int i = 1; i < options.num_levels ; i++) { - ASSERT_EQ(NumTableFilesAtLevel(i), 0); - } // Stage 2: // 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. // After compaction, we should have 3 files, with size 4, 0.4, 2. ASSERT_EQ(NumTableFilesAtLevel(0), 3); - for (int i = 1; i < options.num_levels ; i++) { - ASSERT_EQ(NumTableFilesAtLevel(i), 0); - } - // Stage 3: // 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. @@ -2130,9 +2124,6 @@ TEST(DBTest, UniversalCompactionStopStyleSimilarSize) { dbfull()->TEST_WaitForCompact(); // Level-0 compaction is triggered, but no file will be picked up. 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)