diff --git a/db/db_impl/spdb_db_gs_impl.cc b/db/db_impl/spdb_db_gs_impl.cc index cd394783f..863f715a9 100644 --- a/db/db_impl/spdb_db_gs_impl.cc +++ b/db/db_impl/spdb_db_gs_impl.cc @@ -268,12 +268,13 @@ struct GlobalContext { std::string* csk = nullptr; const Comparator* comparator = nullptr; std::shared_ptr logger; + + std::unique_ptr del_list_iter; }; struct LevelContext { std::unique_ptr values_iter; std::unique_ptr range_del_iter; - std::unique_ptr del_list_iter; // std::string prev_del_key; @@ -310,23 +311,23 @@ void ProcessCurrRangeTsVsDelList(GlobalContext& gc, LevelContext& lc) { // values iter is Invalid, DR Iter is valid => Add to global del-list and // complete level processing - if (lc.del_list_iter->Valid()) { - auto& del_elem = lc.del_list_iter->key(); + if (gc.del_list_iter->Valid()) { + auto& del_elem = gc.del_list_iter->key(); RelativePos overlap_start_rel_pos{RelativePos::NONE}; RelativePos overlap_end_rel_pos{RelativePos::NONE}; auto del_list_vs_range_ts = CompareDelElemToRangeTs( - lc.del_list_iter->key(), range_ts, gc.comparator, + gc.del_list_iter->key(), range_ts, gc.comparator, &overlap_start_rel_pos, &overlap_end_rel_pos); switch (del_list_vs_range_ts) { case RelativePos::BEFORE: - lc.del_list_iter->SeekForward(range_ts.start_key_); + gc.del_list_iter->SeekForward(range_ts.start_key_); break; case RelativePos::AFTER: gc.del_list->InsertBefore( - *lc.del_list_iter, + *gc.del_list_iter, DelElement(range_ts.start_key_, range_ts.end_key_)); lc.range_del_iter->Next(); break; @@ -341,9 +342,9 @@ void ProcessCurrRangeTsVsDelList(GlobalContext& gc, LevelContext& lc) { if (del_elem_starts_at_or_before_range_ts) { if (del_elem_ends_before_range_ts) { gc.del_list->ReplaceWith( - *lc.del_list_iter, + *gc.del_list_iter, DelElement(del_elem.user_start_key, range_ts.end_key_)); - lc.del_list_iter->SeekForward(range_ts.end_key_); + gc.del_list_iter->SeekForward(range_ts.end_key_); } else { // Del-elem contains range-ts => Nothing to do } @@ -351,13 +352,13 @@ void ProcessCurrRangeTsVsDelList(GlobalContext& gc, LevelContext& lc) { if (del_elem_ends_before_range_ts) { // Range-ts contains del-elem gc.del_list->ReplaceWith( - *lc.del_list_iter, + *gc.del_list_iter, DelElement(range_ts.start_key_, range_ts.end_key_)); - lc.del_list_iter->SeekForward(range_ts.end_key_); + gc.del_list_iter->SeekForward(range_ts.end_key_); } else { // del-elem start after range ts but ends after gc.del_list->ReplaceWith( - *lc.del_list_iter, + *gc.del_list_iter, DelElement(range_ts.start_key_, del_elem.user_end_key)); lc.range_del_iter->Seek(del_elem.user_end_key); } @@ -371,7 +372,7 @@ void ProcessCurrRangeTsVsDelList(GlobalContext& gc, LevelContext& lc) { } else { // del list exhausted gc.del_list->InsertBefore( - *lc.del_list_iter, DelElement(range_ts.start_key_, range_ts.end_key_)); + *gc.del_list_iter, DelElement(range_ts.start_key_, range_ts.end_key_)); lc.range_del_iter->Next(); } } @@ -379,15 +380,15 @@ void ProcessCurrRangeTsVsDelList(GlobalContext& gc, LevelContext& lc) { bool ProcessCurrValuesIterVsDelList(GlobalContext& gc, LevelContext& lc) { auto del_list_vs_values_iter_key = RelativePos::AFTER; - if (lc.del_list_iter->Valid()) { + if (gc.del_list_iter->Valid()) { del_list_vs_values_iter_key = CompareDelElemToUserKey( - lc.del_list_iter->key(), lc.values_parsed_ikey.user_key, gc.comparator, + gc.del_list_iter->key(), lc.values_parsed_ikey.user_key, gc.comparator, nullptr /* overlap_start_rel_pos */, nullptr /* overlap_end_rel_pos */); } switch (del_list_vs_values_iter_key) { case RelativePos::BEFORE: - lc.del_list_iter->SeekForward(lc.values_parsed_ikey.user_key); + gc.del_list_iter->SeekForward(lc.values_parsed_ikey.user_key); break; case RelativePos::AFTER: @@ -396,7 +397,7 @@ bool ProcessCurrValuesIterVsDelList(GlobalContext& gc, LevelContext& lc) { UpdateCSK(gc, lc); } else if (lc.value_category == ValueCategory::DEL_KEY) { gc.del_list->InsertBeforeAndSetIterOnInserted( - *lc.del_list_iter, DelElement(lc.values_parsed_ikey.user_key)); + *gc.del_list_iter, DelElement(lc.values_parsed_ikey.user_key)); lc.values_iter->Next(); } break; @@ -404,8 +405,8 @@ bool ProcessCurrValuesIterVsDelList(GlobalContext& gc, LevelContext& lc) { case RelativePos::OVERLAP: // The key is covered by the Del-Elem => it is irrelevant (as all of the // range covered) - if (lc.del_list_iter->key().IsRange()) { - lc.values_iter->Seek(lc.del_list_iter->key().user_end_key); + if (gc.del_list_iter->key().IsRange()) { + lc.values_iter->Seek(gc.del_list_iter->key().user_end_key); } else { lc.values_iter->Next(); } @@ -421,10 +422,7 @@ bool ProcessCurrValuesIterVsDelList(GlobalContext& gc, LevelContext& lc) { } Status ProcessLogLevel(GlobalContext& gc, LevelContext& lc) { - // TODO - Create this once for all levels rather than recreate per level - lc.del_list_iter.reset(gc.del_list->NewIterator().release()); - - lc.del_list_iter->SeekToFirst(); + gc.del_list_iter->SeekToFirst(); lc.values_iter->SeekToFirst(); lc.range_del_iter->SeekToFirst(); @@ -628,6 +626,9 @@ Status DBImpl::GetSmallest(const ReadOptions& read_options, gc.comparator = cfd->user_comparator(); gc.logger = immutable_db_options_.info_log; + // TODO - Create this once for all levels rather than recreate per level + gc.del_list_iter.reset(gc.del_list->NewIterator().release()); + // TODO - User the CSK to set the upper bound of applicable iterators // TODO - Figure out what to do about the Arena diff --git a/db/db_test.cc b/db/db_test.cc index fff2dd5c6..678841bc0 100644 --- a/db/db_test.cc +++ b/db/db_test.cc @@ -8234,15 +8234,35 @@ TEST_F(DBGsTest, GS_ValueImmediatelyDeletedInMutable) { ASSERT_OK(dbfull()->Put(WriteOptions(), "x", "b1")); ASSERT_OK(dbfull()->Delete(WriteOptions(), dflt_cfh, "x")); - gs_debug_prints = true; CALL_WRAPPER(GetSmallestAndValidate("")); } // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX #endif +namespace { +class CountingComparator : public Comparator { + public: + virtual const char* Name() const override { return "UdiComparator"; } + virtual int Compare(const Slice& a, const Slice& b) const override { + ++num_comparisons; + return BytewiseComparator()->Compare(a, b); + } + virtual void FindShortestSeparator(std::string* s, + const Slice& l) const override { + BytewiseComparator()->FindShortestSeparator(s, l); + } + virtual void FindShortSuccessor(std::string* key) const override { + BytewiseComparator()->FindShortSuccessor(key); + } + + public: + mutable size_t num_comparisons = 0U; +}; + +} // namespace class DBGsStressTest : public DBGsTest { public: - void CreateDB() { + void CreateDB(const Comparator* comparator = nullptr) { Options options; options.write_buffer_size = 64 << 20; options.max_write_buffer_number = 1000; @@ -8250,6 +8270,11 @@ class DBGsStressTest : public DBGsTest { options.level0_slowdown_writes_trigger = 2000; options.level0_stop_writes_trigger = 3000; options.level0_file_num_compaction_trigger = -1; + + if (comparator != nullptr) { + options.comparator = comparator; + } + ReopenNewDb(&options); } @@ -8345,7 +8370,8 @@ class DBGsStressTest : public DBGsTest { // #if 0 TEST_F(DBGsStressTest, GS_Stress) { - CreateDB(); + auto comparator = std::make_unique(); + CreateDB(comparator.get()); constexpr int num_flush_iters = 20; constexpr int num_keys_per_iter = 20000U; @@ -8361,6 +8387,7 @@ TEST_F(DBGsStressTest, GS_Stress) { bool delete_during_run = false; std::cout << "\nSeek Start\n"; + comparator->num_comparisons = 0U; auto start = std::chrono::high_resolution_clock::now(); for (auto i = 0; i < num_calls; ++i) { seek_results[i] = GetSmallestUsingSeekToFirst(); @@ -8372,12 +8399,14 @@ TEST_F(DBGsStressTest, GS_Stress) { } auto end = std::chrono::high_resolution_clock::now(); nano total_seek_time = end - start; + auto seek_num_comparisons = comparator->num_comparisons; // Find the keys that are expected to be found by Get-Smallest std::vector expected_smallest_keys = GetSmallestKeysUsingSeek(num_calls); std::cout << "GetSmallest Start\n"; + comparator->num_comparisons = 0U; start = std::chrono::high_resolution_clock::now(); for (auto i = 0; i < num_calls; ++i) { get_smallest_results[i] = GetSmallestUsingGetSmallest(); @@ -8390,6 +8419,7 @@ TEST_F(DBGsStressTest, GS_Stress) { } end = std::chrono::high_resolution_clock::now(); nano total_get_smallest_time = end - start; + auto get_smallest_num_comparisons = comparator->num_comparisons; // Validate that the results of both methods are the same for (auto i = 0; i < num_calls; ++i) { @@ -8406,6 +8436,10 @@ TEST_F(DBGsStressTest, GS_Stress) { std::cout << "GetSmallest Time:" << std::fixed << std::setprecision(precision) << (total_get_smallest_time.count() * 1e-9) << std::endl; + std::cout << "Seek #Comparisons:" << seek_num_comparisons + << ", GetSmallest #Comparisons:" << get_smallest_num_comparisons + << '\n'; + double ratio = static_cast(total_get_smallest_time.count()) / static_cast(total_seek_time.count()); std::cout << "GetSmallest / Seek ratio:" << std::fixed