diff --git a/clients/drcachesim/CMakeLists.txt b/clients/drcachesim/CMakeLists.txt index 801da27c67d..b61af20ec1d 100644 --- a/clients/drcachesim/CMakeLists.txt +++ b/clients/drcachesim/CMakeLists.txt @@ -168,6 +168,7 @@ add_exported_library(drmemtrace_func_view STATIC tools/func_view.cpp) add_exported_library(drmemtrace_invariant_checker STATIC tools/invariant_checker.cpp) add_exported_library(drmemtrace_schedule_stats STATIC tools/schedule_stats.cpp) add_exported_library(drmemtrace_schedule_file STATIC common/schedule_file.cpp) +add_exported_library(drmemtrace_mutex_dbg_owned STATIC common/mutex_dbg_owned.cpp) target_link_libraries(drmemtrace_invariant_checker drdecode drmemtrace_schedule_file) @@ -284,7 +285,7 @@ target_link_libraries(drmemtrace_launcher drmemtrace_simulator drmemtrace_reuse_ drmemtrace_histogram drmemtrace_reuse_time drmemtrace_basic_counts drmemtrace_opcode_mix drmemtrace_syscall_mix drmemtrace_view drmemtrace_func_view drmemtrace_raw2trace directory_iterator drmemtrace_invariant_checker - drmemtrace_schedule_stats drmemtrace_record_filter) + drmemtrace_schedule_stats drmemtrace_record_filter drmemtrace_mutex_dbg_owned) if (UNIX) target_link_libraries(drmemtrace_launcher dl) endif () @@ -331,7 +332,7 @@ add_exported_library(drmemtrace_analyzer STATIC ${snappy_reader} ${lz4_reader} ) -target_link_libraries(drmemtrace_analyzer directory_iterator) +target_link_libraries(drmemtrace_analyzer directory_iterator drmemtrace_mutex_dbg_owned) if (libsnappy) target_link_libraries(drmemtrace_analyzer snappy) endif () @@ -610,6 +611,12 @@ macro(add_win32_flags target) if (DEBUG AND NOT DEFINED ${target}_uses_configure) append_property_list(TARGET ${target} COMPILE_DEFINITIONS "DEBUG") endif () + if (NOT DEBUG) + get_property(cur TARGET ${target} PROPERTY COMPILE_DEFINITIONS) + if (NOT "${cur}" MATCHES "NDEBUG") + append_property_list(TARGET ${target} COMPILE_DEFINITIONS "NDEBUG") + endif () + endif () if (WIN32) if (DEBUG) get_property(cur TARGET ${target} PROPERTY COMPILE_FLAGS) @@ -666,6 +673,7 @@ add_win32_flags(drmemtrace_schedule_stats) add_win32_flags(drmemtrace_schedule_file) add_win32_flags(directory_iterator) add_win32_flags(test_helpers) +add_win32_flags(drmemtrace_mutex_dbg_owned) if (WIN32 AND DEBUG) get_target_property(sim_srcs drmemtrace_launcher SOURCES) get_target_property(raw2trace_srcs drraw2trace SOURCES) @@ -821,13 +829,13 @@ if (BUILD_TESTS) add_executable(tool.scheduler.unit_tests tests/scheduler_unit_tests.cpp) target_link_libraries(tool.scheduler.unit_tests drmemtrace_analyzer test_helpers) - add_win32_flags(tool.scheduler.unit_tests) if (WIN32) # We have a dup symbol from linking in DR. Linking libc first doesn't help. append_property_string(TARGET tool.scheduler.unit_tests LINK_FLAGS "/force:multiple") endif () configure_DynamoRIO_standalone(tool.scheduler.unit_tests) + add_win32_flags(tool.scheduler.unit_tests) add_test(NAME tool.scheduler.unit_tests COMMAND tool.scheduler.unit_tests ${PROJECT_SOURCE_DIR}/clients/drcachesim/tests) @@ -881,6 +889,7 @@ if (BUILD_TESTS) add_executable(tool.drcacheoff.record_filter_unit_tests tests/record_filter_unit_tests.cpp) configure_DynamoRIO_standalone(tool.drcacheoff.record_filter_unit_tests) + add_win32_flags(tool.drcacheoff.record_filter_unit_tests) target_link_libraries(tool.drcacheoff.record_filter_unit_tests drmemtrace_analyzer drmemtrace_basic_counts drmemtrace_record_filter test_helpers) add_test(NAME tool.drcacheoff.record_filter_unit_tests diff --git a/clients/drcachesim/common/mutex_dbg_owned.cpp b/clients/drcachesim/common/mutex_dbg_owned.cpp new file mode 100644 index 00000000000..8289ca10d1f --- /dev/null +++ b/clients/drcachesim/common/mutex_dbg_owned.cpp @@ -0,0 +1,104 @@ +/* ********************************************************** + * Copyright (c) 2024 Google, Inc. All rights reserved. + * **********************************************************/ + +/* + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * * Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * * Neither the name of Google, Inc. nor the names of its contributors may be + * used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL VMWARE, INC. OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH + * DAMAGE. + */ + +#include +#include +#include "mutex_dbg_owned.h" + +namespace dynamorio { +namespace drmemtrace { + +void +mutex_dbg_owned::lock() +{ +#ifdef NDEBUG + lock_.lock(); +#else + bool contended = true; + if (lock_.try_lock()) + contended = false; + else + lock_.lock(); + owner_ = std::this_thread::get_id(); + ++count_acquired_; + if (contended) + ++count_contended_; +#endif +} + +bool +mutex_dbg_owned::try_lock() +{ +#ifdef NDEBUG + return lock_.try_lock(); +#else + if (lock_.try_lock()) { + owner_ = std::this_thread::get_id(); + return true; + } + return false; +#endif +} + +void +mutex_dbg_owned::unlock() +{ +#ifndef NDEBUG + owner_ = std::thread::id(); // id() creates a no-thread sentinel value. +#endif + lock_.unlock(); +} + +// This query should only be called when the lock is required to be held +// as it is racy when the lock is not held. +bool +mutex_dbg_owned::owned_by_cur_thread() +{ + return owner_ == std::this_thread::get_id(); +} + +// These statistics only count lock(): they do *not* count try_lock() +// (we could count try_lock with std::atomic on count_contended). +int64_t +mutex_dbg_owned::get_count_acquired() +{ + return count_acquired_; +} + +int64_t +mutex_dbg_owned::get_count_contended() +{ + return count_contended_; +} + +} // namespace drmemtrace +} // namespace dynamorio diff --git a/clients/drcachesim/common/mutex_dbg_owned.h b/clients/drcachesim/common/mutex_dbg_owned.h index f41f0e512fc..a17b86f3369 100644 --- a/clients/drcachesim/common/mutex_dbg_owned.h +++ b/clients/drcachesim/common/mutex_dbg_owned.h @@ -30,7 +30,7 @@ * DAMAGE. */ -/* mutex_dbg_owned.h: std::mutex plus an assertable owner in debug builds. */ +/* mutex_dbg_owned.h: std::mutex plus an assertable owner and stats in debug builds. */ #ifndef _MUTEX_DBG_OWNED_H_ #define _MUTEX_DBG_OWNED_H_ 1 @@ -43,50 +43,36 @@ namespace drmemtrace { // A wrapper around std::mutex which adds an owner field for asserts on ownership // when a lock is required to be held by the caller (where -// std::unique_lock::owns_lock() cannot easily be used). The owner field is only -// maintained when NDEBUG is not defined: i.e., it is targeted for asserts. +// std::unique_lock::owns_lock() cannot easily be used). +// It also adds contention statistics. These additional fields are only maintained +// when NDEBUG is not defined: i.e., they are targeted for asserts and diagnostics. class mutex_dbg_owned { public: void - lock() - { - lock_.lock(); -#ifndef NDEBUG - owner_ = std::this_thread::get_id(); -#endif - } + lock(); bool - try_lock() - { -#ifdef NDEBUG - return lock_.try_lock(); -#else - if (lock_.try_lock()) { - owner_ = std::this_thread::get_id(); - return true; - } - return false; -#endif - } + try_lock(); void - unlock() - { -#ifndef NDEBUG - owner_ = std::thread::id(); // id() creates a no-thread sentinel value. -#endif - lock_.unlock(); - } + unlock(); // This query should only be called when the lock is required to be held // as it is racy when the lock is not held. bool - owned_by_cur_thread() - { - return owner_ == std::this_thread::get_id(); - } + owned_by_cur_thread(); + // These statistics only count lock(): they do *not* count try_lock() + // (we could count try_lock with std::atomic on count_contended). + int64_t + get_count_acquired(); + int64_t + get_count_contended(); private: std::mutex lock_; + // We do not place these under ifndef NDEBUG as it is too easy to get two + // compilation units with different values for NDEBUG conflicting. + // We thus pay the space cost in NDEBUG build. std::thread::id owner_; + int64_t count_acquired_ = 0; + int64_t count_contended_ = 0; }; } // namespace drmemtrace diff --git a/clients/drcachesim/scheduler/scheduler.cpp b/clients/drcachesim/scheduler/scheduler.cpp index 793f4a149ec..f5107f2e2f9 100644 --- a/clients/drcachesim/scheduler/scheduler.cpp +++ b/clients/drcachesim/scheduler/scheduler.cpp @@ -687,6 +687,12 @@ scheduler_tmpl_t::~scheduler_tmpl_t() VPRINT(this, 1, " %-25s: %9" PRId64 "\n", "Migrations", outputs_[i].stats[memtrace_stream_t::SCHED_STAT_MIGRATIONS]); } +#ifndef NDEBUG + VPRINT(this, 1, "%-27s: %9" PRId64 "\n", "Schedule lock acquired", + sched_lock_.get_count_acquired()); + VPRINT(this, 1, "%-27s: %9" PRId64 "\n", "Schedule lock contended", + sched_lock_.get_count_contended()); +#endif } template diff --git a/clients/drcachesim/tests/memref_gen.h b/clients/drcachesim/tests/memref_gen.h index b752994ec03..785b49e2b45 100644 --- a/clients/drcachesim/tests/memref_gen.h +++ b/clients/drcachesim/tests/memref_gen.h @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2021-2023 Google, LLC All rights reserved. + * Copyright (c) 2021-2024 Google, LLC All rights reserved. * **********************************************************/ /* @@ -190,7 +190,9 @@ add_encodings_to_memrefs(instrlist_t *ilist, { static const int MAX_DECODE_SIZE = 2048; byte decode_buf[MAX_DECODE_SIZE]; +#ifndef NDEBUG byte *pc = +#endif instrlist_encode_to_copy(GLOBAL_DCONTEXT, ilist, decode_buf, reinterpret_cast(base_addr), nullptr, true); assert(pc != nullptr);