From f955144e9ddb3d0e48819fcd7570837f4fb56dd8 Mon Sep 17 00:00:00 2001 From: lemonhx Date: Fri, 2 Jul 2021 13:00:34 +0800 Subject: [PATCH 1/8] `PerfFlag` implementation aside the `PerfLevel` design spec in on: https://docs.google.com/document/d/1JYmWMIZwYV0AZW6rNv_oFVZLBCAkxLLYslt39eEZstM/edit?usp=sharing Signed-off-by: lemonhx --- CMakeLists.txt | 1 + db/c.cc | 16 +++++ include/rocksdb/c.h | 4 ++ include/rocksdb/perf_flag.h | 15 +++++ include/rocksdb/perf_flag_defs.h | 107 +++++++++++++++++++++++++++++++ monitoring/iostats_context_imp.h | 4 +- monitoring/perf_context_imp.h | 39 ++++++----- monitoring/perf_flag_imp.h | 10 +++ monitoring/perf_step_timer.h | 5 +- 9 files changed, 177 insertions(+), 24 deletions(-) create mode 100644 include/rocksdb/perf_flag.h create mode 100644 include/rocksdb/perf_flag_defs.h create mode 100644 monitoring/perf_flag_imp.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 441173a29a1..b66f0dfaee9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -585,6 +585,7 @@ set(SOURCES monitoring/iostats_context.cc monitoring/perf_context.cc monitoring/perf_level.cc + monitoring/perf_flag.cc monitoring/persistent_stats_history.cc monitoring/statistics.cc monitoring/thread_status_impl.cc diff --git a/db/c.cc b/db/c.cc index 4fe556c3063..7275f2f0a1b 100644 --- a/db/c.cc +++ b/db/c.cc @@ -24,6 +24,7 @@ #include "rocksdb/memtablerep.h" #include "rocksdb/merge_operator.h" #include "rocksdb/options.h" +#include "rocksdb/perf_flag.h" #include "rocksdb/perf_context.h" #include "rocksdb/rate_limiter.h" #include "rocksdb/slice_transform.h" @@ -114,6 +115,9 @@ using rocksdb::Checkpoint; using rocksdb::TransactionLogIterator; using rocksdb::BatchResult; using rocksdb::PerfLevel; +using rocksdb::EnablePerfFlag; +using rocksdb::DisablePerfFlag; +using rocksdb::CheckPerfFlag; using rocksdb::PerfContext; using rocksdb::MemoryUtil; @@ -2749,6 +2753,18 @@ void rocksdb_set_perf_level(int v) { SetPerfLevel(level); } +void rocksdb_enable_perf_flag(uint64_t flag){ + EnablePerfFlag(flag); +} + +void rocksdb_disable_perf_flag(uint64_t flag){ + DisablePerfFlag(flag); +} + +int rocksdb_check_perf_flag(uint64_t flag){ + return (int)CheckPerfFlag(flag); +} + rocksdb_perfcontext_t* rocksdb_perfcontext_create() { rocksdb_perfcontext_t* context = new rocksdb_perfcontext_t; context->rep = rocksdb::get_perf_context(); diff --git a/include/rocksdb/c.h b/include/rocksdb/c.h index f21dbcf7d4a..e01e6711039 100644 --- a/include/rocksdb/c.h +++ b/include/rocksdb/c.h @@ -1119,7 +1119,11 @@ enum { rocksdb_total_metric_count = 68 }; +#include "perf_flag_defs.h" extern ROCKSDB_LIBRARY_API void rocksdb_set_perf_level(int); +extern ROCKSDB_LIBRARY_API void rocksdb_enable_perf_flag(uint64_t); +extern ROCKSDB_LIBRARY_API void rocksdb_disable_perf_flag(uint64_t); +extern ROCKSDB_LIBRARY_API int rocksdb_check_perf_flag(uint64_t); extern ROCKSDB_LIBRARY_API rocksdb_perfcontext_t* rocksdb_perfcontext_create(); extern ROCKSDB_LIBRARY_API void rocksdb_perfcontext_reset( rocksdb_perfcontext_t* context); diff --git a/include/rocksdb/perf_flag.h b/include/rocksdb/perf_flag.h new file mode 100644 index 00000000000..e99e877491d --- /dev/null +++ b/include/rocksdb/perf_flag.h @@ -0,0 +1,15 @@ +// complements to perf_level +#include +#pragma once +#define FLAGS_LEN \ + (((uint64_t)FLAG_END & (uint64_t)0b111) == 0 \ + ? ((uint64_t)FLAG_END >> (uint64_t)0b11) \ + : ((uint64_t)FLAG_END >> (uint64_t)0b11) + (uint64_t)1) +namespace rocksdb{ + void EnablePerfFlag(uint64_t flag); + void DisablePerfFlag(uint64_t flag); + bool CheckPerfFlag(uint64_t flag); +} + +#define GET_FLAG(flag) perf_flags[(uint64_t)(flag) >> (uint64_t)0b11] +#include "perf_flag_defs.h" diff --git a/include/rocksdb/perf_flag_defs.h b/include/rocksdb/perf_flag_defs.h new file mode 100644 index 00000000000..6125f3fc9e7 --- /dev/null +++ b/include/rocksdb/perf_flag_defs.h @@ -0,0 +1,107 @@ +#pragma once +#include + +enum { + flag_user_key_comparison_count = 0, + flag_block_cache_hit_count, + flag_block_read_count, + flag_block_read_byte, + flag_block_read_time, + flag_block_cache_index_hit_count, + flag_index_block_read_count, + flag_block_cache_filter_hit_count, + flag_filter_block_read_count, + flag_compression_dict_block_read_count, + flag_block_checksum_time, + flag_block_decompress_time, + flag_get_read_bytes, + flag_multiget_read_bytes, + flag_iter_read_bytes, + flag_internal_key_skipped_count, + flag_internal_delete_skipped_count, + flag_internal_recent_skipped_count, + flag_internal_merge_count, + flag_get_snapshot_time, + flag_get_from_memtable_time, + flag_get_from_memtable_count, + flag_get_post_process_time, + flag_get_from_output_files_time, + flag_seek_on_memtable_time, + flag_seek_on_memtable_count, + flag_next_on_memtable_count, + flag_prev_on_memtable_count, + flag_seek_child_seek_time, + flag_seek_child_seek_count, + flag_seek_min_heap_time, + flag_seek_max_heap_time, + flag_seek_internal_seek_time, + flag_find_next_user_entry_time, + flag_write_wal_time, + flag_write_memtable_time, + flag_write_delay_time, + flag_write_scheduling_flushes_compactions_time, + flag_write_pre_and_post_process_time, + flag_write_thread_wait_nanos, + flag_db_mutex_lock_nanos, + flag_db_condition_wait_nanos, + flag_merge_operator_time_nanos, + flag_read_index_block_nanos, + flag_read_filter_block_nanos, + flag_new_table_block_iter_nanos, + flag_new_table_iterator_nanos, + flag_block_seek_nanos, + flag_find_table_nanos, + flag_bloom_memtable_hit_count, + flag_bloom_memtable_miss_count, + flag_bloom_sst_hit_count, + flag_bloom_sst_miss_count, + flag_key_lock_wait_time, + flag_key_lock_wait_count, + flag_env_new_sequential_file_nanos, + flag_env_new_random_access_file_nanos, + flag_env_new_writable_file_nanos, + flag_env_reuse_writable_file_nanos, + flag_env_new_random_rw_file_nanos, + flag_env_new_directory_nanos, + flag_env_file_exists_nanos, + flag_env_get_children_nanos, + flag_env_get_children_file_attributes_nanos, + flag_env_delete_file_nanos, + flag_env_create_dir_nanos, + flag_env_create_dir_if_missing_nanos, + flag_env_delete_dir_nanos, + flag_env_get_file_size_nanos, + flag_env_get_file_modification_time_nanos, + flag_env_rename_file_nanos, + flag_env_link_file_nanos, + flag_env_lock_file_nanos, + flag_env_unlock_file_nanos, + flag_env_new_logger_nanos, + flag_get_cpu_nanos, + flag_iter_next_cpu_nanos, + flag_iter_prev_cpu_nanos, + flag_iter_seek_cpu_nanos, + flag_encrypt_data_nanos, + flag_decrypt_data_nanos, + + flag_get_from_table_nanos, + flag_user_key_return_count, + flag_block_cache_miss_count, + flag_bloom_filter_full_positive, + flag_bloom_filter_useful, + flag_bloom_filter_full_true_positive, + + flag_bytes_read, + flag_bytes_written, + flag_open_nanos, + flag_allocate_nanos, + flag_write_nanos, + flag_read_nanos, + flag_range_sync_nanos, + flag_prepare_write_nanos, + flag_fsync_nanos, + flag_logger_nanos, + flag_cpu_read_nanos, + flag_cpu_write_nanos, + FLAG_END +}; diff --git a/monitoring/iostats_context_imp.h b/monitoring/iostats_context_imp.h index 19e34209b1b..4dcd8c161e2 100644 --- a/monitoring/iostats_context_imp.h +++ b/monitoring/iostats_context_imp.h @@ -34,13 +34,13 @@ extern __thread IOStatsContext iostats_context; // Declare and set start time of the timer #define IOSTATS_TIMER_GUARD(metric) \ - PerfStepTimer iostats_step_timer_##metric(&(iostats_context.metric)); \ + PerfStepTimer iostats_step_timer_##metric(&(iostats_context.metric), CheckPerfFlag(flag_##metric)); \ iostats_step_timer_##metric.Start(); // Declare and set start time of the timer #define IOSTATS_CPU_TIMER_GUARD(metric, env) \ PerfStepTimer iostats_step_timer_##metric( \ - &(iostats_context.metric), env, true, \ + &(iostats_context.metric), CheckPerfFlag(flag_##metric), env, true, \ PerfLevel::kEnableTimeAndCPUTimeExceptForMutex); \ iostats_step_timer_##metric.Start(); diff --git a/monitoring/perf_context_imp.h b/monitoring/perf_context_imp.h index e0ff8afc58e..49338fdd962 100644 --- a/monitoring/perf_context_imp.h +++ b/monitoring/perf_context_imp.h @@ -38,24 +38,24 @@ extern thread_local PerfContext perf_context; // Declare and set start time of the timer #define PERF_TIMER_GUARD(metric) \ - PerfStepTimer perf_step_timer_##metric(&(perf_context.metric)); \ + PerfStepTimer perf_step_timer_##metric(&(perf_context.metric),CheckPerfFlag(flag_##metric)); \ perf_step_timer_##metric.Start(); // Declare and set start time of the timer #define PERF_TIMER_GUARD_WITH_ENV(metric, env) \ - PerfStepTimer perf_step_timer_##metric(&(perf_context.metric), env); \ + PerfStepTimer perf_step_timer_##metric(&(perf_context.metric),CheckPerfFlag(flag_##metric),env); \ perf_step_timer_##metric.Start(); // Declare and set start time of the timer #define PERF_CPU_TIMER_GUARD(metric, env) \ PerfStepTimer perf_step_timer_##metric( \ - &(perf_context.metric), env, true, \ + &(perf_context.metric),CheckPerfFlag(flag_##metric), env, true, \ PerfLevel::kEnableTimeAndCPUTimeExceptForMutex); \ perf_step_timer_##metric.Start(); #define PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(metric, condition, stats, \ ticker_type) \ - PerfStepTimer perf_step_timer_##metric(&(perf_context.metric), nullptr, \ + PerfStepTimer perf_step_timer_##metric(&(perf_context.metric),CheckPerfFlag(flag_##metric), nullptr, \ false, PerfLevel::kEnableTime, stats, \ ticker_type); \ if (condition) { \ @@ -68,26 +68,25 @@ extern thread_local PerfContext perf_context; // Increase metric value #define PERF_COUNTER_ADD(metric, value) \ - if (perf_level >= PerfLevel::kEnableCount) { \ + if (perf_level >= PerfLevel::kEnableCount || CheckPerfFlag(flag_##metric)) { \ perf_context.metric += value; \ } // Increase metric value -#define PERF_COUNTER_BY_LEVEL_ADD(metric, value, level) \ - if (perf_level >= PerfLevel::kEnableCount && \ - perf_context.per_level_perf_context_enabled && \ - perf_context.level_to_perf_context) { \ - if ((*(perf_context.level_to_perf_context)).find(level) != \ - (*(perf_context.level_to_perf_context)).end()) { \ - (*(perf_context.level_to_perf_context))[level].metric += value; \ - } \ - else { \ - PerfContextByLevel empty_context; \ - (*(perf_context.level_to_perf_context))[level] = empty_context; \ - (*(perf_context.level_to_perf_context))[level].metric += value; \ - } \ - } \ +#define PERF_COUNTER_BY_LEVEL_ADD(metric, value, level) \ + if ((perf_level >= PerfLevel::kEnableCount || CheckPerfFlag(flag_##metric)) && \ + perf_context.per_level_perf_context_enabled && \ + perf_context.level_to_perf_context) { \ + if ((*(perf_context.level_to_perf_context)).find(level) != \ + (*(perf_context.level_to_perf_context)).end()) { \ + (*(perf_context.level_to_perf_context))[level].metric += value; \ + } else { \ + PerfContextByLevel empty_context; \ + (*(perf_context.level_to_perf_context))[level] = empty_context; \ + (*(perf_context.level_to_perf_context))[level].metric += value; \ + } \ + } #endif -} +} // namespace rocksdb diff --git a/monitoring/perf_flag_imp.h b/monitoring/perf_flag_imp.h new file mode 100644 index 00000000000..453c5e03db8 --- /dev/null +++ b/monitoring/perf_flag_imp.h @@ -0,0 +1,10 @@ +#include +#include "rocksdb/perf_flag.h" + +namespace rocksdb { +#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL +extern __thread uint8_t perf_flags[FLAGS_LEN]; +#else +extern uint8_t perf_flags[FLAGS_LEN]; +#endif +} diff --git a/monitoring/perf_step_timer.h b/monitoring/perf_step_timer.h index 6501bd54aba..5aa89c9049c 100644 --- a/monitoring/perf_step_timer.h +++ b/monitoring/perf_step_timer.h @@ -5,6 +5,7 @@ // #pragma once #include "monitoring/perf_level_imp.h" +#include "monitoring/perf_flag_imp.h" #include "rocksdb/env.h" #include "util/stop_watch.h" @@ -13,10 +14,10 @@ namespace rocksdb { class PerfStepTimer { public: explicit PerfStepTimer( - uint64_t* metric, Env* env = nullptr, bool use_cpu_time = false, + uint64_t* metric, bool enable_flag = false, Env* env = nullptr, bool use_cpu_time = false, PerfLevel enable_level = PerfLevel::kEnableTimeExceptForMutex, Statistics* statistics = nullptr, uint32_t ticker_type = 0) - : perf_counter_enabled_(perf_level >= enable_level), + : perf_counter_enabled_(perf_level >= enable_level || enable_flag), use_cpu_time_(use_cpu_time), env_((perf_counter_enabled_ || statistics != nullptr) ? ((env != nullptr) ? env : Env::Default()) From f113d65d520cb5abc0f263023710127438f1ab3d Mon Sep 17 00:00:00 2001 From: lemonhx Date: Tue, 6 Jul 2021 18:26:11 +0800 Subject: [PATCH 2/8] Adding some test and fixing git errors fix indentation error in `c.cc` adding comment on `FLAG_END` explain what does `& 0b111` does fix a huge mistake in `CheckPerfFlag` and writing the edge case tests for `perf_flag` Signed-off-by: lemonhx --- CMakeLists.txt | 3 +- db/c.cc | 2 +- include/rocksdb/perf_flag_defs.h | 1 + monitoring/perf_flag.cc | 33 +++++++++++++ monitoring/perf_flag_test.cc | 82 ++++++++++++++++++++++++++++++++ 5 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 monitoring/perf_flag.cc create mode 100644 monitoring/perf_flag_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index b66f0dfaee9..015da49f4c3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -975,7 +975,8 @@ if(WITH_TESTS) memtable/skiplist_test.cc memtable/write_buffer_manager_test.cc monitoring/histogram_test.cc - monitoring/iostats_context_test.cc + monitoring/perf_flag_test.cc + monitoring/iostats_context_test.cc monitoring/statistics_test.cc monitoring/stats_history_test.cc options/options_settable_test.cc diff --git a/db/c.cc b/db/c.cc index 7275f2f0a1b..4610329c29d 100644 --- a/db/c.cc +++ b/db/c.cc @@ -2758,7 +2758,7 @@ void rocksdb_enable_perf_flag(uint64_t flag){ } void rocksdb_disable_perf_flag(uint64_t flag){ - DisablePerfFlag(flag); + DisablePerfFlag(flag); } int rocksdb_check_perf_flag(uint64_t flag){ diff --git a/include/rocksdb/perf_flag_defs.h b/include/rocksdb/perf_flag_defs.h index 6125f3fc9e7..d2cddf98e25 100644 --- a/include/rocksdb/perf_flag_defs.h +++ b/include/rocksdb/perf_flag_defs.h @@ -103,5 +103,6 @@ enum { flag_logger_nanos, flag_cpu_read_nanos, flag_cpu_write_nanos, + // Should always be the last FLAG_END }; diff --git a/monitoring/perf_flag.cc b/monitoring/perf_flag.cc new file mode 100644 index 00000000000..e62d638f0fb --- /dev/null +++ b/monitoring/perf_flag.cc @@ -0,0 +1,33 @@ +#include "rocksdb/perf_flag.h" + +namespace rocksdb { +#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL +__thread uint8_t perf_flags[FLAGS_LEN] = {0}; +#else +uint8_t perf_flags[FLAGS_LEN] = {0}; +#endif + +void EnablePerfFlag(uint64_t flag) { + if (CheckPerfFlag(flag)) { + } else { + // & 0b111 means find the flag location is a alternative way to do mod operation + GET_FLAG(flag) ^= (uint64_t)0b1 << ((uint64_t)flag & (uint64_t)0b111); + } +} + +void DisablePerfFlag(uint64_t flag) { + if (CheckPerfFlag(flag)) { + GET_FLAG(flag) ^= (uint64_t)0b1 << ((uint64_t)flag & (uint64_t)0b111); + } else { + } +} + +bool CheckPerfFlag(uint64_t flag) { + auto _1 = (uint64_t)0b1 << (flag & (uint64_t)0b111); + auto _2 = GET_FLAG(flag); + auto _3 = _2 & _1; + return ((uint64_t)GET_FLAG(flag) & (uint64_t)0b1 + << (flag & (uint64_t)0b111)) != 0; +} + +} // namespace rocksdb \ No newline at end of file diff --git a/monitoring/perf_flag_test.cc b/monitoring/perf_flag_test.cc new file mode 100644 index 00000000000..35c25de881b --- /dev/null +++ b/monitoring/perf_flag_test.cc @@ -0,0 +1,82 @@ +#include "rocksdb/perf_flag.h" + +#include +#include + +#include + +#include "rocksdb/db.h" +#include "rocksdb/env.h" +#include "rocksdb/options.h" +#include "rocksdb/perf_context.h" +#include "rocksdb/slice.h" + +namespace rocksdb { +class PerfFlagTest : testing::Test {}; +TEST(PerfFlagTest, TestEnableFlag) { + for (int i = 0; i < 10; ++i) { + EnablePerfFlag(i); + ASSERT_EQ(CheckPerfFlag(i), true); + } +} +TEST(PerfFlagTest, TestDisableFlag) { + for (int i = 0; i < 10; ++i) { + EnablePerfFlag(i); + } + for (int i = 0; i < 10; ++i) { + DisablePerfFlag(i); + ASSERT_EQ(CheckPerfFlag(i), false); + } +} + +#define DB_TEST_HELPER(ENV, VALIDATION) \ + { \ + std::unique_ptr env{NewMemEnv(Env::Default())}; \ + Options options; \ + options.create_if_missing = true; \ + options.env = env.get(); \ + DB* db; \ + ENV const Slice keys[] = {Slice("aaa"), Slice("bbb"), Slice("ccc")}; \ + const Slice vals[] = {Slice("foo"), Slice("bar"), Slice("baz")}; \ + ASSERT_OK(DB::Open(options, "/dir/db", &db)); \ + for (size_t i = 0; i < 3; ++i) { \ + ASSERT_OK(db->Put(WriteOptions(), keys[i], vals[i])); \ + } \ + VALIDATION \ + std::cout << "current_perf_context:\n\t" \ + << get_perf_context()->ToString(true) << std::endl; \ + } + +TEST(PerfFlagTest, TestEnableFlagStandAlone) { + DB_TEST_HELPER( + { + SetPerfLevel(PerfLevel::kDisable); + EnablePerfFlag(flag_user_key_comparison_count); + }, + { ASSERT_GT(get_perf_context()->user_key_comparison_count, 0); }); +} + +TEST(PerfFlagTest, TestPerfLevelNonoverlappingPerfFlag) { + DB_TEST_HELPER( + { + SetPerfLevel(PerfLevel::kEnableCount); + EnablePerfFlag(flag_write_wal_time); + }, + { ASSERT_GT(get_perf_context()->write_wal_time, 0); }); +} + +TEST(PerfFlagTest, TestPerfLevelOverLappingPerfFlag) { + DB_TEST_HELPER( + { + SetPerfLevel(PerfLevel::kEnableTime); + EnablePerfFlag(flag_write_wal_time); + }, + { ASSERT_GT(get_perf_context()->write_wal_time, 0); }); +} + +} // namespace rocksdb + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} \ No newline at end of file From 4ff539b871cd65b8823bef62fddc4745d29452dd Mon Sep 17 00:00:00 2001 From: lemonhx Date: Mon, 2 Aug 2021 08:48:04 +0800 Subject: [PATCH 3/8] add source to `src.mk` add test into `Makefile` and removed unused variable. fix indentation error in `CMakeLists` Signed-off-by: lemonhx --- CMakeLists.txt | 40 ++++++++++++++++++++-------------------- Makefile | 4 ++++ monitoring/perf_flag.cc | 3 --- src.mk | 2 ++ 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 015da49f4c3..c2c25f1474c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -965,28 +965,28 @@ if(WITH_TESTS) encryption/encryption_test.cc env/env_basic_test.cc env/env_test.cc - env/mock_env_test.cc - file/delete_scheduler_test.cc - logging/auto_roll_logger_test.cc - logging/env_logger_test.cc - logging/event_logger_test.cc - memory/arena_test.cc - memtable/inlineskiplist_test.cc - memtable/skiplist_test.cc - memtable/write_buffer_manager_test.cc - monitoring/histogram_test.cc + env/mock_env_test.cc + file/delete_scheduler_test.cc + logging/auto_roll_logger_test.cc + logging/env_logger_test.cc + logging/event_logger_test.cc + memory/arena_test.cc + memtable/inlineskiplist_test.cc + memtable/skiplist_test.cc + memtable/write_buffer_manager_test.cc + monitoring/histogram_test.cc monitoring/perf_flag_test.cc monitoring/iostats_context_test.cc - monitoring/statistics_test.cc - monitoring/stats_history_test.cc - options/options_settable_test.cc - options/options_test.cc - table/block_based/block_based_filter_block_test.cc - table/block_based/block_test.cc - table/block_based/data_block_hash_index_test.cc - table/block_based/full_filter_block_test.cc - table/block_based/partitioned_filter_block_test.cc - table/cleanable_test.cc + monitoring/statistics_test.cc + monitoring/stats_history_test.cc + options/options_settable_test.cc + options/options_test.cc + table/block_based/block_based_filter_block_test.cc + table/block_based/block_test.cc + table/block_based/data_block_hash_index_test.cc + table/block_based/full_filter_block_test.cc + table/block_based/partitioned_filter_block_test.cc + table/cleanable_test.cc table/cuckoo/cuckoo_table_builder_test.cc table/cuckoo/cuckoo_table_reader_test.cc table/merger_test.cc diff --git a/Makefile b/Makefile index e935bc14be6..0dd3f8931eb 100644 --- a/Makefile +++ b/Makefile @@ -418,6 +418,7 @@ ANALYZETOOLOBJECTS = $(ANALYZER_LIB_SOURCES:.cc=.o) EXPOBJECTS = $(LIBOBJECTS) $(TESTUTIL) TESTS = \ + perf_flag_test \ db_basic_test \ db_encryption_test \ db_test2 \ @@ -1579,6 +1580,9 @@ ldb: tools/ldb.o $(LIBOBJECTS) iostats_context_test: monitoring/iostats_context_test.o $(LIBOBJECTS) $(TESTHARNESS) $(AM_V_CCLD)$(CXX) $^ $(EXEC_LDFLAGS) -o $@ $(LDFLAGS) +perf_flag_test: monitoring/perf_flag_test.o $(LIBOBJECTS) $(TESTHARNESS) + $(AM_LINK) + persistent_cache_test: utilities/persistent_cache/persistent_cache_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) $(AM_LINK) diff --git a/monitoring/perf_flag.cc b/monitoring/perf_flag.cc index e62d638f0fb..813a57638d0 100644 --- a/monitoring/perf_flag.cc +++ b/monitoring/perf_flag.cc @@ -23,9 +23,6 @@ void DisablePerfFlag(uint64_t flag) { } bool CheckPerfFlag(uint64_t flag) { - auto _1 = (uint64_t)0b1 << (flag & (uint64_t)0b111); - auto _2 = GET_FLAG(flag); - auto _3 = _2 & _1; return ((uint64_t)GET_FLAG(flag) & (uint64_t)0b1 << (flag & (uint64_t)0b111)) != 0; } diff --git a/src.mk b/src.mk index 12bb2e0601a..7e2daa164cd 100644 --- a/src.mk +++ b/src.mk @@ -94,6 +94,7 @@ LIB_SOURCES = \ monitoring/iostats_context.cc \ monitoring/perf_context.cc \ monitoring/perf_level.cc \ + monitoring/perf_flag.cc \ monitoring/persistent_stats_history.cc \ monitoring/statistics.cc \ monitoring/thread_status_impl.cc \ @@ -364,6 +365,7 @@ MAIN_SOURCES = \ monitoring/iostats_context_test.cc \ monitoring/statistics_test.cc \ monitoring/stats_history_test.cc \ + monitoring/perf_flag_test.cc \ options/options_test.cc \ table/block_based/block_based_filter_block_test.cc \ table/block_based/block_test.cc \ From c05a1121e42e17a2461bc97012c7fb0f3879cc92 Mon Sep 17 00:00:00 2001 From: LemonHX Date: Mon, 9 Aug 2021 07:49:49 +0800 Subject: [PATCH 4/8] fix formatting and styling issues. Co-authored-by: Xinye Tao Signed-off-by: lemonhx --- CMakeLists.txt | 44 +++++++++++++++++------------------ db/c.cc | 8 +++---- include/rocksdb/perf_flag.h | 4 ++-- monitoring/perf_context_imp.h | 2 +- monitoring/perf_flag.cc | 6 ++--- monitoring/perf_flag_test.cc | 4 +++- 6 files changed, 35 insertions(+), 33 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c2c25f1474c..4ebbce8b8a8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -965,28 +965,28 @@ if(WITH_TESTS) encryption/encryption_test.cc env/env_basic_test.cc env/env_test.cc - env/mock_env_test.cc - file/delete_scheduler_test.cc - logging/auto_roll_logger_test.cc - logging/env_logger_test.cc - logging/event_logger_test.cc - memory/arena_test.cc - memtable/inlineskiplist_test.cc - memtable/skiplist_test.cc - memtable/write_buffer_manager_test.cc - monitoring/histogram_test.cc - monitoring/perf_flag_test.cc - monitoring/iostats_context_test.cc - monitoring/statistics_test.cc - monitoring/stats_history_test.cc - options/options_settable_test.cc - options/options_test.cc - table/block_based/block_based_filter_block_test.cc - table/block_based/block_test.cc - table/block_based/data_block_hash_index_test.cc - table/block_based/full_filter_block_test.cc - table/block_based/partitioned_filter_block_test.cc - table/cleanable_test.cc + env/mock_env_test.cc + file/delete_scheduler_test.cc + logging/auto_roll_logger_test.cc + logging/env_logger_test.cc + logging/event_logger_test.cc + memory/arena_test.cc + memtable/inlineskiplist_test.cc + memtable/skiplist_test.cc + memtable/write_buffer_manager_test.cc + monitoring/histogram_test.cc + monitoring/perf_flag_test.cc + monitoring/iostats_context_test.cc + monitoring/statistics_test.cc + monitoring/stats_history_test.cc + options/options_settable_test.cc + options/options_test.cc + table/block_based/block_based_filter_block_test.cc + table/block_based/block_test.cc + table/block_based/data_block_hash_index_test.cc + table/block_based/full_filter_block_test.cc + table/block_based/partitioned_filter_block_test.cc + table/cleanable_test.cc table/cuckoo/cuckoo_table_builder_test.cc table/cuckoo/cuckoo_table_reader_test.cc table/merger_test.cc diff --git a/db/c.cc b/db/c.cc index 4610329c29d..12bf3f764c1 100644 --- a/db/c.cc +++ b/db/c.cc @@ -2753,16 +2753,16 @@ void rocksdb_set_perf_level(int v) { SetPerfLevel(level); } -void rocksdb_enable_perf_flag(uint64_t flag){ +void rocksdb_enable_perf_flag(uint64_t flag) { EnablePerfFlag(flag); } -void rocksdb_disable_perf_flag(uint64_t flag){ +void rocksdb_disable_perf_flag(uint64_t flag) { DisablePerfFlag(flag); } -int rocksdb_check_perf_flag(uint64_t flag){ - return (int)CheckPerfFlag(flag); +int rocksdb_check_perf_flag(uint64_t flag) { + return static_cast(CheckPerfFlag(flag)); } rocksdb_perfcontext_t* rocksdb_perfcontext_create() { diff --git a/include/rocksdb/perf_flag.h b/include/rocksdb/perf_flag.h index e99e877491d..bb67f5e2e69 100644 --- a/include/rocksdb/perf_flag.h +++ b/include/rocksdb/perf_flag.h @@ -5,11 +5,11 @@ (((uint64_t)FLAG_END & (uint64_t)0b111) == 0 \ ? ((uint64_t)FLAG_END >> (uint64_t)0b11) \ : ((uint64_t)FLAG_END >> (uint64_t)0b11) + (uint64_t)1) -namespace rocksdb{ +namespace rocksdb { void EnablePerfFlag(uint64_t flag); void DisablePerfFlag(uint64_t flag); bool CheckPerfFlag(uint64_t flag); -} +} // namespace rocksdb #define GET_FLAG(flag) perf_flags[(uint64_t)(flag) >> (uint64_t)0b11] #include "perf_flag_defs.h" diff --git a/monitoring/perf_context_imp.h b/monitoring/perf_context_imp.h index 49338fdd962..04b5b84722b 100644 --- a/monitoring/perf_context_imp.h +++ b/monitoring/perf_context_imp.h @@ -38,7 +38,7 @@ extern thread_local PerfContext perf_context; // Declare and set start time of the timer #define PERF_TIMER_GUARD(metric) \ - PerfStepTimer perf_step_timer_##metric(&(perf_context.metric),CheckPerfFlag(flag_##metric)); \ + PerfStepTimer perf_step_timer_##metric(&(perf_context.metric), CheckPerfFlag(flag_##metric)); \ perf_step_timer_##metric.Start(); // Declare and set start time of the timer diff --git a/monitoring/perf_flag.cc b/monitoring/perf_flag.cc index 813a57638d0..3526eea0fc1 100644 --- a/monitoring/perf_flag.cc +++ b/monitoring/perf_flag.cc @@ -10,7 +10,8 @@ uint8_t perf_flags[FLAGS_LEN] = {0}; void EnablePerfFlag(uint64_t flag) { if (CheckPerfFlag(flag)) { } else { - // & 0b111 means find the flag location is a alternative way to do mod operation + // & 0b111 means find the flag location is a alternative way to do mod + // operation GET_FLAG(flag) ^= (uint64_t)0b1 << ((uint64_t)flag & (uint64_t)0b111); } } @@ -18,7 +19,6 @@ void EnablePerfFlag(uint64_t flag) { void DisablePerfFlag(uint64_t flag) { if (CheckPerfFlag(flag)) { GET_FLAG(flag) ^= (uint64_t)0b1 << ((uint64_t)flag & (uint64_t)0b111); - } else { } } @@ -27,4 +27,4 @@ bool CheckPerfFlag(uint64_t flag) { << (flag & (uint64_t)0b111)) != 0; } -} // namespace rocksdb \ No newline at end of file +} // namespace rocksdb diff --git a/monitoring/perf_flag_test.cc b/monitoring/perf_flag_test.cc index 35c25de881b..31a5dc95f45 100644 --- a/monitoring/perf_flag_test.cc +++ b/monitoring/perf_flag_test.cc @@ -13,12 +13,14 @@ namespace rocksdb { class PerfFlagTest : testing::Test {}; + TEST(PerfFlagTest, TestEnableFlag) { for (int i = 0; i < 10; ++i) { EnablePerfFlag(i); ASSERT_EQ(CheckPerfFlag(i), true); } } + TEST(PerfFlagTest, TestDisableFlag) { for (int i = 0; i < 10; ++i) { EnablePerfFlag(i); @@ -79,4 +81,4 @@ TEST(PerfFlagTest, TestPerfLevelOverLappingPerfFlag) { int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); -} \ No newline at end of file +} From 5a54760eb273676082dc504de188dcc05884816f Mon Sep 17 00:00:00 2001 From: tabokie Date: Thu, 12 Aug 2021 12:15:23 +0800 Subject: [PATCH 5/8] address comments Signed-off-by: tabokie --- CMakeLists.txt | 1 - Makefile | 5 +- db/builder.cc | 3 +- db/c.cc | 10 +--- db/compaction/compaction_iterator.cc | 2 +- db/perf_context_test.cc | 20 +++++++ include/rocksdb/c.h | 1 - include/rocksdb/perf_flag.h | 12 ++-- monitoring/iostats_context_imp.h | 13 +++-- monitoring/perf_context_imp.h | 39 +++++++------ monitoring/perf_flag.cc | 7 +-- monitoring/perf_flag_test.cc | 84 ---------------------------- monitoring/perf_step_timer.h | 5 +- src.mk | 1 - 14 files changed, 68 insertions(+), 135 deletions(-) delete mode 100644 monitoring/perf_flag_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 4ebbce8b8a8..b66f0dfaee9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -975,7 +975,6 @@ if(WITH_TESTS) memtable/skiplist_test.cc memtable/write_buffer_manager_test.cc monitoring/histogram_test.cc - monitoring/perf_flag_test.cc monitoring/iostats_context_test.cc monitoring/statistics_test.cc monitoring/stats_history_test.cc diff --git a/Makefile b/Makefile index 0dd3f8931eb..e120bb92c93 100644 --- a/Makefile +++ b/Makefile @@ -418,7 +418,6 @@ ANALYZETOOLOBJECTS = $(ANALYZER_LIB_SOURCES:.cc=.o) EXPOBJECTS = $(LIBOBJECTS) $(TESTUTIL) TESTS = \ - perf_flag_test \ db_basic_test \ db_encryption_test \ db_test2 \ @@ -439,6 +438,7 @@ TESTS = \ thread_local_test \ rate_limiter_test \ perf_context_test \ + \ iostats_context_test \ db_wal_test \ db_block_cache_test \ @@ -1580,9 +1580,6 @@ ldb: tools/ldb.o $(LIBOBJECTS) iostats_context_test: monitoring/iostats_context_test.o $(LIBOBJECTS) $(TESTHARNESS) $(AM_V_CCLD)$(CXX) $^ $(EXEC_LDFLAGS) -o $@ $(LDFLAGS) -perf_flag_test: monitoring/perf_flag_test.o $(LIBOBJECTS) $(TESTHARNESS) - $(AM_LINK) - persistent_cache_test: utilities/persistent_cache/persistent_cache_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) $(AM_LINK) diff --git a/db/builder.cc b/db/builder.cc index eac1b5fe2e1..d9ca20bce59 100644 --- a/db/builder.cc +++ b/db/builder.cc @@ -192,7 +192,8 @@ Status BuildTable( meta->fd.file_size = file_size; meta->marked_for_compaction = builder->NeedCompact(); assert(meta->fd.GetFileSize() > 0); - tp = builder->GetTableProperties(); // refresh now that builder is finished + tp = builder + ->GetTableProperties(); // refresh now that builder is finished if (table_properties) { *table_properties = tp; } diff --git a/db/c.cc b/db/c.cc index 12bf3f764c1..5acca963011 100644 --- a/db/c.cc +++ b/db/c.cc @@ -24,8 +24,8 @@ #include "rocksdb/memtablerep.h" #include "rocksdb/merge_operator.h" #include "rocksdb/options.h" -#include "rocksdb/perf_flag.h" #include "rocksdb/perf_context.h" +#include "rocksdb/perf_flag.h" #include "rocksdb/rate_limiter.h" #include "rocksdb/slice_transform.h" #include "rocksdb/statistics.h" @@ -2753,13 +2753,9 @@ void rocksdb_set_perf_level(int v) { SetPerfLevel(level); } -void rocksdb_enable_perf_flag(uint64_t flag) { - EnablePerfFlag(flag); -} +void rocksdb_enable_perf_flag(uint64_t flag) { EnablePerfFlag(flag); } -void rocksdb_disable_perf_flag(uint64_t flag) { - DisablePerfFlag(flag); -} +void rocksdb_disable_perf_flag(uint64_t flag) { DisablePerfFlag(flag); } int rocksdb_check_perf_flag(uint64_t flag) { return static_cast(CheckPerfFlag(flag)); diff --git a/db/compaction/compaction_iterator.cc b/db/compaction/compaction_iterator.cc index 1d3406b4082..48281af1255 100644 --- a/db/compaction/compaction_iterator.cc +++ b/db/compaction/compaction_iterator.cc @@ -121,7 +121,7 @@ void CompactionIterator::Next() { key_ = merge_out_iter_.key(); value_ = merge_out_iter_.value(); bool valid_key __attribute__((__unused__)); - valid_key = ParseInternalKey(key_, &ikey_); + valid_key = ParseInternalKey(key_, &ikey_); // MergeUntil stops when it encounters a corrupt key and does not // include them in the result, so we expect the keys here to be valid. assert(valid_key); diff --git a/db/perf_context_test.cc b/db/perf_context_test.cc index 94eabff7ff5..4f0d74e52f8 100644 --- a/db/perf_context_test.cc +++ b/db/perf_context_test.cc @@ -938,6 +938,26 @@ TEST_F(PerfContextTest, CPUTimer) { ASSERT_EQ(count, get_perf_context()->iter_seek_cpu_nanos); } } + +TEST_F(PerfContextTest, BitMapControl) { + DestroyDB(kDbName, Options()); + auto db = OpenDb(); + WriteOptions write_options; + SetPerfLevel(PerfLevel::kDisable); + EnablePerfFlag(flag_user_key_comparison_count); + EnablePerfFlag(flag_write_wal_time); + + for (int i = 0; i < FLAGS_total_keys; ++i) { + std::string i_str = ToString(i); + std::string key = "k" + i_str; + std::string value = "v" + i_str; + + db->Put(write_options, key, value); + } + ASSERT_GT(get_perf_context()->user_key_comparison_count, 0); + ASSERT_GT(get_perf_context()->write_wal_time, 0); +} + } // namespace rocksdb int main(int argc, char** argv) { diff --git a/include/rocksdb/c.h b/include/rocksdb/c.h index e01e6711039..1db70697df1 100644 --- a/include/rocksdb/c.h +++ b/include/rocksdb/c.h @@ -1119,7 +1119,6 @@ enum { rocksdb_total_metric_count = 68 }; -#include "perf_flag_defs.h" extern ROCKSDB_LIBRARY_API void rocksdb_set_perf_level(int); extern ROCKSDB_LIBRARY_API void rocksdb_enable_perf_flag(uint64_t); extern ROCKSDB_LIBRARY_API void rocksdb_disable_perf_flag(uint64_t); diff --git a/include/rocksdb/perf_flag.h b/include/rocksdb/perf_flag.h index bb67f5e2e69..020a121f427 100644 --- a/include/rocksdb/perf_flag.h +++ b/include/rocksdb/perf_flag.h @@ -1,15 +1,17 @@ // complements to perf_level -#include #pragma once + +#include + #define FLAGS_LEN \ (((uint64_t)FLAG_END & (uint64_t)0b111) == 0 \ ? ((uint64_t)FLAG_END >> (uint64_t)0b11) \ : ((uint64_t)FLAG_END >> (uint64_t)0b11) + (uint64_t)1) namespace rocksdb { - void EnablePerfFlag(uint64_t flag); - void DisablePerfFlag(uint64_t flag); - bool CheckPerfFlag(uint64_t flag); -} // namespace rocksdb +void EnablePerfFlag(uint64_t flag); +void DisablePerfFlag(uint64_t flag); +bool CheckPerfFlag(uint64_t flag); +} // namespace rocksdb #define GET_FLAG(flag) perf_flags[(uint64_t)(flag) >> (uint64_t)0b11] #include "perf_flag_defs.h" diff --git a/monitoring/iostats_context_imp.h b/monitoring/iostats_context_imp.h index 4dcd8c161e2..41b432543ba 100644 --- a/monitoring/iostats_context_imp.h +++ b/monitoring/iostats_context_imp.h @@ -33,15 +33,16 @@ extern __thread IOStatsContext iostats_context; #define IOSTATS(metric) (iostats_context.metric) // Declare and set start time of the timer -#define IOSTATS_TIMER_GUARD(metric) \ - PerfStepTimer iostats_step_timer_##metric(&(iostats_context.metric), CheckPerfFlag(flag_##metric)); \ +#define IOSTATS_TIMER_GUARD(metric) \ + PerfStepTimer iostats_step_timer_##metric(&(iostats_context.metric), \ + CheckPerfFlag(flag_##metric)); \ iostats_step_timer_##metric.Start(); // Declare and set start time of the timer -#define IOSTATS_CPU_TIMER_GUARD(metric, env) \ - PerfStepTimer iostats_step_timer_##metric( \ - &(iostats_context.metric), CheckPerfFlag(flag_##metric), env, true, \ - PerfLevel::kEnableTimeAndCPUTimeExceptForMutex); \ +#define IOSTATS_CPU_TIMER_GUARD(metric, env) \ + PerfStepTimer iostats_step_timer_##metric( \ + &(iostats_context.metric), CheckPerfFlag(flag_##metric), env, true, \ + PerfLevel::kEnableTimeAndCPUTimeExceptForMutex); \ iostats_step_timer_##metric.Start(); #else // ROCKSDB_SUPPORT_THREAD_LOCAL diff --git a/monitoring/perf_context_imp.h b/monitoring/perf_context_imp.h index 04b5b84722b..ed61e6ec26f 100644 --- a/monitoring/perf_context_imp.h +++ b/monitoring/perf_context_imp.h @@ -37,29 +37,31 @@ extern thread_local PerfContext perf_context; #define PERF_TIMER_START(metric) perf_step_timer_##metric.Start(); // Declare and set start time of the timer -#define PERF_TIMER_GUARD(metric) \ - PerfStepTimer perf_step_timer_##metric(&(perf_context.metric), CheckPerfFlag(flag_##metric)); \ +#define PERF_TIMER_GUARD(metric) \ + PerfStepTimer perf_step_timer_##metric(&(perf_context.metric), \ + CheckPerfFlag(flag_##metric)); \ perf_step_timer_##metric.Start(); // Declare and set start time of the timer -#define PERF_TIMER_GUARD_WITH_ENV(metric, env) \ - PerfStepTimer perf_step_timer_##metric(&(perf_context.metric),CheckPerfFlag(flag_##metric),env); \ +#define PERF_TIMER_GUARD_WITH_ENV(metric, env) \ + PerfStepTimer perf_step_timer_##metric(&(perf_context.metric), \ + CheckPerfFlag(flag_##metric), env); \ perf_step_timer_##metric.Start(); // Declare and set start time of the timer -#define PERF_CPU_TIMER_GUARD(metric, env) \ - PerfStepTimer perf_step_timer_##metric( \ - &(perf_context.metric),CheckPerfFlag(flag_##metric), env, true, \ - PerfLevel::kEnableTimeAndCPUTimeExceptForMutex); \ +#define PERF_CPU_TIMER_GUARD(metric, env) \ + PerfStepTimer perf_step_timer_##metric( \ + &(perf_context.metric), CheckPerfFlag(flag_##metric), env, true, \ + PerfLevel::kEnableTimeAndCPUTimeExceptForMutex); \ perf_step_timer_##metric.Start(); -#define PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(metric, condition, stats, \ - ticker_type) \ - PerfStepTimer perf_step_timer_##metric(&(perf_context.metric),CheckPerfFlag(flag_##metric), nullptr, \ - false, PerfLevel::kEnableTime, stats, \ - ticker_type); \ - if (condition) { \ - perf_step_timer_##metric.Start(); \ +#define PERF_CONDITIONAL_TIMER_FOR_MUTEX_GUARD(metric, condition, stats, \ + ticker_type) \ + PerfStepTimer perf_step_timer_##metric( \ + &(perf_context.metric), CheckPerfFlag(flag_##metric), nullptr, false, \ + PerfLevel::kEnableTime, stats, ticker_type); \ + if (condition) { \ + perf_step_timer_##metric.Start(); \ } // Update metric with time elapsed since last START. start time is reset @@ -67,14 +69,15 @@ extern thread_local PerfContext perf_context; #define PERF_TIMER_MEASURE(metric) perf_step_timer_##metric.Measure(); // Increase metric value -#define PERF_COUNTER_ADD(metric, value) \ +#define PERF_COUNTER_ADD(metric, value) \ if (perf_level >= PerfLevel::kEnableCount || CheckPerfFlag(flag_##metric)) { \ - perf_context.metric += value; \ + perf_context.metric += value; \ } // Increase metric value #define PERF_COUNTER_BY_LEVEL_ADD(metric, value, level) \ - if ((perf_level >= PerfLevel::kEnableCount || CheckPerfFlag(flag_##metric)) && \ + if ((perf_level >= PerfLevel::kEnableCount || \ + CheckPerfFlag(flag_##metric)) && \ perf_context.per_level_perf_context_enabled && \ perf_context.level_to_perf_context) { \ if ((*(perf_context.level_to_perf_context)).find(level) != \ diff --git a/monitoring/perf_flag.cc b/monitoring/perf_flag.cc index 3526eea0fc1..a3bdbda353e 100644 --- a/monitoring/perf_flag.cc +++ b/monitoring/perf_flag.cc @@ -8,8 +8,7 @@ uint8_t perf_flags[FLAGS_LEN] = {0}; #endif void EnablePerfFlag(uint64_t flag) { - if (CheckPerfFlag(flag)) { - } else { + if (!CheckPerfFlag(flag)) { // & 0b111 means find the flag location is a alternative way to do mod // operation GET_FLAG(flag) ^= (uint64_t)0b1 << ((uint64_t)flag & (uint64_t)0b111); @@ -23,8 +22,8 @@ void DisablePerfFlag(uint64_t flag) { } bool CheckPerfFlag(uint64_t flag) { - return ((uint64_t)GET_FLAG(flag) & (uint64_t)0b1 - << (flag & (uint64_t)0b111)) != 0; + return ((uint64_t)GET_FLAG(flag) & + (uint64_t)0b1 << (flag & (uint64_t)0b111)) != 0; } } // namespace rocksdb diff --git a/monitoring/perf_flag_test.cc b/monitoring/perf_flag_test.cc deleted file mode 100644 index 31a5dc95f45..00000000000 --- a/monitoring/perf_flag_test.cc +++ /dev/null @@ -1,84 +0,0 @@ -#include "rocksdb/perf_flag.h" - -#include -#include - -#include - -#include "rocksdb/db.h" -#include "rocksdb/env.h" -#include "rocksdb/options.h" -#include "rocksdb/perf_context.h" -#include "rocksdb/slice.h" - -namespace rocksdb { -class PerfFlagTest : testing::Test {}; - -TEST(PerfFlagTest, TestEnableFlag) { - for (int i = 0; i < 10; ++i) { - EnablePerfFlag(i); - ASSERT_EQ(CheckPerfFlag(i), true); - } -} - -TEST(PerfFlagTest, TestDisableFlag) { - for (int i = 0; i < 10; ++i) { - EnablePerfFlag(i); - } - for (int i = 0; i < 10; ++i) { - DisablePerfFlag(i); - ASSERT_EQ(CheckPerfFlag(i), false); - } -} - -#define DB_TEST_HELPER(ENV, VALIDATION) \ - { \ - std::unique_ptr env{NewMemEnv(Env::Default())}; \ - Options options; \ - options.create_if_missing = true; \ - options.env = env.get(); \ - DB* db; \ - ENV const Slice keys[] = {Slice("aaa"), Slice("bbb"), Slice("ccc")}; \ - const Slice vals[] = {Slice("foo"), Slice("bar"), Slice("baz")}; \ - ASSERT_OK(DB::Open(options, "/dir/db", &db)); \ - for (size_t i = 0; i < 3; ++i) { \ - ASSERT_OK(db->Put(WriteOptions(), keys[i], vals[i])); \ - } \ - VALIDATION \ - std::cout << "current_perf_context:\n\t" \ - << get_perf_context()->ToString(true) << std::endl; \ - } - -TEST(PerfFlagTest, TestEnableFlagStandAlone) { - DB_TEST_HELPER( - { - SetPerfLevel(PerfLevel::kDisable); - EnablePerfFlag(flag_user_key_comparison_count); - }, - { ASSERT_GT(get_perf_context()->user_key_comparison_count, 0); }); -} - -TEST(PerfFlagTest, TestPerfLevelNonoverlappingPerfFlag) { - DB_TEST_HELPER( - { - SetPerfLevel(PerfLevel::kEnableCount); - EnablePerfFlag(flag_write_wal_time); - }, - { ASSERT_GT(get_perf_context()->write_wal_time, 0); }); -} - -TEST(PerfFlagTest, TestPerfLevelOverLappingPerfFlag) { - DB_TEST_HELPER( - { - SetPerfLevel(PerfLevel::kEnableTime); - EnablePerfFlag(flag_write_wal_time); - }, - { ASSERT_GT(get_perf_context()->write_wal_time, 0); }); -} - -} // namespace rocksdb - -int main(int argc, char** argv) { - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); -} diff --git a/monitoring/perf_step_timer.h b/monitoring/perf_step_timer.h index 5aa89c9049c..85fb2b13cbd 100644 --- a/monitoring/perf_step_timer.h +++ b/monitoring/perf_step_timer.h @@ -4,8 +4,8 @@ // (found in the LICENSE.Apache file in the root directory). // #pragma once -#include "monitoring/perf_level_imp.h" #include "monitoring/perf_flag_imp.h" +#include "monitoring/perf_level_imp.h" #include "rocksdb/env.h" #include "util/stop_watch.h" @@ -14,7 +14,8 @@ namespace rocksdb { class PerfStepTimer { public: explicit PerfStepTimer( - uint64_t* metric, bool enable_flag = false, Env* env = nullptr, bool use_cpu_time = false, + uint64_t* metric, bool enable_flag = false, Env* env = nullptr, + bool use_cpu_time = false, PerfLevel enable_level = PerfLevel::kEnableTimeExceptForMutex, Statistics* statistics = nullptr, uint32_t ticker_type = 0) : perf_counter_enabled_(perf_level >= enable_level || enable_flag), diff --git a/src.mk b/src.mk index 7e2daa164cd..6980d2f0b66 100644 --- a/src.mk +++ b/src.mk @@ -365,7 +365,6 @@ MAIN_SOURCES = \ monitoring/iostats_context_test.cc \ monitoring/statistics_test.cc \ monitoring/stats_history_test.cc \ - monitoring/perf_flag_test.cc \ options/options_test.cc \ table/block_based/block_based_filter_block_test.cc \ table/block_based/block_test.cc \ From 7aa7f274aeae289377fdc7d86cf634fea417bbd5 Mon Sep 17 00:00:00 2001 From: tabokie Date: Thu, 12 Aug 2021 12:24:23 +0800 Subject: [PATCH 6/8] remove redundant line Signed-off-by: tabokie --- Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Makefile b/Makefile index e120bb92c93..e935bc14be6 100644 --- a/Makefile +++ b/Makefile @@ -438,7 +438,6 @@ TESTS = \ thread_local_test \ rate_limiter_test \ perf_context_test \ - \ iostats_context_test \ db_wal_test \ db_block_cache_test \ From 8dbd097fcd412a682afd5a6d648bec9c8660c25d Mon Sep 17 00:00:00 2001 From: tabokie Date: Thu, 12 Aug 2021 14:15:17 +0800 Subject: [PATCH 7/8] trigger rebuild Signed-off-by: tabokie --- include/rocksdb/perf_flag.h | 8 +++++--- src.mk | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/include/rocksdb/perf_flag.h b/include/rocksdb/perf_flag.h index 020a121f427..9e8bb38f615 100644 --- a/include/rocksdb/perf_flag.h +++ b/include/rocksdb/perf_flag.h @@ -3,15 +3,17 @@ #include +#include "perf_flag_defs.h" + +#define GET_FLAG(flag) perf_flags[(uint64_t)(flag) >> (uint64_t)0b11] + #define FLAGS_LEN \ (((uint64_t)FLAG_END & (uint64_t)0b111) == 0 \ ? ((uint64_t)FLAG_END >> (uint64_t)0b11) \ : ((uint64_t)FLAG_END >> (uint64_t)0b11) + (uint64_t)1) + namespace rocksdb { void EnablePerfFlag(uint64_t flag); void DisablePerfFlag(uint64_t flag); bool CheckPerfFlag(uint64_t flag); } // namespace rocksdb - -#define GET_FLAG(flag) perf_flags[(uint64_t)(flag) >> (uint64_t)0b11] -#include "perf_flag_defs.h" diff --git a/src.mk b/src.mk index 6980d2f0b66..13923612f6d 100644 --- a/src.mk +++ b/src.mk @@ -94,7 +94,7 @@ LIB_SOURCES = \ monitoring/iostats_context.cc \ monitoring/perf_context.cc \ monitoring/perf_level.cc \ - monitoring/perf_flag.cc \ + monitoring/perf_flag.cc \ monitoring/persistent_stats_history.cc \ monitoring/statistics.cc \ monitoring/thread_status_impl.cc \ From 9e34b86789af1bc5def40a3ea70eaa162a069de8 Mon Sep 17 00:00:00 2001 From: tabokie Date: Thu, 12 Aug 2021 18:01:47 +0800 Subject: [PATCH 8/8] address comments Signed-off-by: tabokie --- include/rocksdb/perf_flag.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/include/rocksdb/perf_flag.h b/include/rocksdb/perf_flag.h index 9e8bb38f615..57536b9a7c5 100644 --- a/include/rocksdb/perf_flag.h +++ b/include/rocksdb/perf_flag.h @@ -5,12 +5,13 @@ #include "perf_flag_defs.h" -#define GET_FLAG(flag) perf_flags[(uint64_t)(flag) >> (uint64_t)0b11] +#define GET_FLAG(flag) perf_flags[(uint64_t)(flag) >> 3] -#define FLAGS_LEN \ - (((uint64_t)FLAG_END & (uint64_t)0b111) == 0 \ - ? ((uint64_t)FLAG_END >> (uint64_t)0b11) \ - : ((uint64_t)FLAG_END >> (uint64_t)0b11) + (uint64_t)1) +// FLAGS_LEN = ceiling(FLAG_END / bits(uint8_t)) +#define FLAGS_LEN \ + (((uint64_t)FLAG_END & (uint64_t)0b111) == 0 \ + ? ((uint64_t)FLAG_END >> 3) \ + : ((uint64_t)FLAG_END >> 3) + 1) namespace rocksdb { void EnablePerfFlag(uint64_t flag);