diff --git a/db/db_bloom_filter_test.cc b/db/db_bloom_filter_test.cc index ee0582bf7b027cf03849d061e91430859459e900..84d7facd28d2e3fc35980f7427d4a94335208572 100644 --- a/db/db_bloom_filter_test.cc +++ b/db/db_bloom_filter_test.cc @@ -172,28 +172,38 @@ TEST_F(DBBloomFilterTest, GetFilterByPrefixBloomCustomPrefixExtractor) { ASSERT_EQ("foo", Get("barbarbar")); ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 0); - // TODO (Zhongyi): uncomment the asserts involving level_to_perf_context when per - // level perf context is enabled in block based table reader - // ASSERT_EQ(0, (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); + ASSERT_EQ( + 0, + (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); ASSERT_EQ("foo2", Get("barbarbar2")); ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 0); - // ASSERT_EQ(0, (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); + ASSERT_EQ( + 0, + (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); ASSERT_EQ("NOT_FOUND", Get("barbarbar3")); ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 0); - // ASSERT_EQ(0, (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); + ASSERT_EQ( + 0, + (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); ASSERT_EQ("NOT_FOUND", Get("barfoofoo")); ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 1); - // ASSERT_EQ(1, (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); + ASSERT_EQ( + 1, + (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); ASSERT_EQ("NOT_FOUND", Get("foobarbar")); ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 2); - // ASSERT_EQ(2, (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); + ASSERT_EQ( + 2, + (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); ro.total_order_seek = true; ASSERT_TRUE(db_->Get(ro, "foobarbar", &value).IsNotFound()); ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 2); - // ASSERT_EQ(2, (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); + ASSERT_EQ( + 2, + (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); get_perf_context()->Reset(); } } @@ -242,7 +252,9 @@ TEST_F(DBBloomFilterTest, GetFilterByPrefixBloom) { ro.total_order_seek = true; ASSERT_TRUE(db_->Get(ro, "foobarbar", &value).IsNotFound()); ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 2); - // ASSERT_EQ(2, (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); + ASSERT_EQ( + 2, + (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); get_perf_context()->Reset(); } } @@ -400,7 +412,13 @@ TEST_F(DBBloomFilterTest, WholeKeyFilterProp) { ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 12); ASSERT_EQ("bar", Get("barfoo")); ASSERT_EQ(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 12); - // ASSERT_EQ(12, (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful); + uint64_t bloom_filter_useful_all_levels = 0; + for (auto& kv : (*(get_perf_context()->level_to_perf_context))) { + if (kv.second.bloom_filter_useful > 0) { + bloom_filter_useful_all_levels += kv.second.bloom_filter_useful; + } + } + ASSERT_EQ(12, bloom_filter_useful_all_levels); get_perf_context()->Reset(); } } @@ -528,7 +546,9 @@ TEST_F(DBBloomFilterTest, BloomFilterRate) { ASSERT_EQ("NOT_FOUND", Get(1, Key(i + 33333))); } ASSERT_GE(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), maxKey * 0.98); - // ASSERT_GE((*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful, maxKey*0.98); + ASSERT_GE( + (*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful, + maxKey * 0.98); get_perf_context()->Reset(); } } @@ -1098,8 +1118,14 @@ TEST_F(DBBloomFilterTest, OptimizeFiltersForHits) { // no bloom filter. Most keys be checked bloom filters twice. ASSERT_GT(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 65000 * 2); ASSERT_LT(TestGetTickerCount(options, BLOOM_FILTER_USEFUL), 120000 * 2); - // ASSERT_GT((*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful, 65000*2); - // ASSERT_LT((*(get_perf_context()->level_to_perf_context))[0].bloom_filter_useful, 120000*2); + uint64_t bloom_filter_useful_all_levels = 0; + for (auto& kv : (*(get_perf_context()->level_to_perf_context))) { + if (kv.second.bloom_filter_useful > 0) { + bloom_filter_useful_all_levels += kv.second.bloom_filter_useful; + } + } + ASSERT_GT(bloom_filter_useful_all_levels, 65000 * 2); + ASSERT_LT(bloom_filter_useful_all_levels, 120000 * 2); for (int i = 0; i < numkeys; i += 2) { ASSERT_EQ(Get(1, Key(i)), "val"); diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc index faf5826f2a2aab21ab345bbd2c9b741154d78542..b116c7220bd4dc020d8ab754ffaacc2c233dc9e6 100644 --- a/table/block_based_table_reader.cc +++ b/table/block_based_table_reader.cc @@ -815,7 +815,7 @@ Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions, // raw pointer will be used to create HashIndexReader, whose reset may // access a dangling pointer. Rep* rep = new BlockBasedTable::Rep(ioptions, env_options, table_options, - internal_comparator, skip_filters, + internal_comparator, skip_filters, level, immortal_table); rep->file = std::move(file); rep->footer = footer; @@ -2315,8 +2315,7 @@ bool BlockBasedTable::FullFilterKeyMayMatch( } if (may_match) { RecordTick(rep_->ioptions.statistics, BLOOM_FILTER_FULL_POSITIVE); - // TODO(Zhongyi): use the correct level here - // PERF_COUNTER_BY_LEVEL_ADD(bloom_filter_full_positive, 1, /*level*/); + PERF_COUNTER_BY_LEVEL_ADD(bloom_filter_full_positive, 1, rep_->level); } return may_match; } @@ -2341,8 +2340,7 @@ Status BlockBasedTable::Get(const ReadOptions& read_options, const Slice& key, if (!FullFilterKeyMayMatch(read_options, filter, key, no_io, prefix_extractor)) { RecordTick(rep_->ioptions.statistics, BLOOM_FILTER_USEFUL); - // TODO(Zhongyi): use the correct level here - // PERF_COUNTER_BY_LEVEL_ADD(bloom_filter_useful, 1, /*level*/); + PERF_COUNTER_BY_LEVEL_ADD(bloom_filter_useful, 1, rep_->level); } else { IndexBlockIter iiter_on_stack; // if prefix_extractor found in block differs from options, disable @@ -2375,8 +2373,7 @@ Status BlockBasedTable::Get(const ReadOptions& read_options, const Slice& key, // TODO: think about interaction with Merge. If a user key cannot // cross one data block, we should be fine. RecordTick(rep_->ioptions.statistics, BLOOM_FILTER_USEFUL); - // TODO(Zhongyi): use the correct level here - // PERF_COUNTER_BY_LEVEL_ADD(bloom_filter_useful, 1, /*level*/); + PERF_COUNTER_BY_LEVEL_ADD(bloom_filter_useful, 1, rep_->level); break; } else { DataBlockIter biter; @@ -2429,8 +2426,8 @@ Status BlockBasedTable::Get(const ReadOptions& read_options, const Slice& key, } if (matched && filter != nullptr && !filter->IsBlockBased()) { RecordTick(rep_->ioptions.statistics, BLOOM_FILTER_FULL_TRUE_POSITIVE); - // TODO(Zhongyi): use the correct level here - // PERF_COUNTER_BY_LEVEL_ADD(bloom_filter_full_true_positive, 1, /*level*/); + PERF_COUNTER_BY_LEVEL_ADD(bloom_filter_full_true_positive, 1, + rep_->level); } if (s.ok()) { s = iiter->status(); diff --git a/table/block_based_table_reader.h b/table/block_based_table_reader.h index 59a0f36b56bc792d4e9a6bffa3b08aaf12c54bce..24a7ef24baadea29858cc2d0d16548a788a71f62 100644 --- a/table/block_based_table_reader.h +++ b/table/block_based_table_reader.h @@ -432,7 +432,7 @@ struct BlockBasedTable::Rep { Rep(const ImmutableCFOptions& _ioptions, const EnvOptions& _env_options, const BlockBasedTableOptions& _table_opt, const InternalKeyComparator& _internal_comparator, bool skip_filters, - const bool _immortal_table) + int _level, const bool _immortal_table) : ioptions(_ioptions), env_options(_env_options), table_options(_table_opt), @@ -445,6 +445,7 @@ struct BlockBasedTable::Rep { prefix_filtering(true), range_del_handle(BlockHandle::NullBlockHandle()), global_seqno(kDisableGlobalSequenceNumber), + level(_level), immortal_table(_immortal_table) {} const ImmutableCFOptions& ioptions; @@ -518,6 +519,10 @@ struct BlockBasedTable::Rep { // and every key have it's own seqno. SequenceNumber global_seqno; + // the level when the table is opened, could potentially change when trivial + // move is involved + int level; + // If false, blocks in this file are definitely all uncompressed. Knowing this // before reading individual blocks enables certain optimizations. bool blocks_maybe_compressed = true; diff --git a/table/partitioned_filter_block_test.cc b/table/partitioned_filter_block_test.cc index 0b11c0df2a785747c49e655e65d7a8c01a20cf92..2ee773befbfde28748969ad6130e7f8a73c63f58 100644 --- a/table/partitioned_filter_block_test.cc +++ b/table/partitioned_filter_block_test.cc @@ -147,7 +147,7 @@ class PartitionedFilterBlockTest const bool kImmortal = true; table.reset(new MockedBlockBasedTable( new BlockBasedTable::Rep(ioptions, env_options, table_options_, icomp, - !kSkipFilters, !kImmortal))); + !kSkipFilters, 0, !kImmortal))); auto reader = new PartitionedFilterBlockReader( prefix_extractor, true, BlockContents(slice, false, kNoCompression), nullptr, nullptr, icomp, table.get(), pib->seperator_is_key_plus_seq(),