i#6938 sched migrate: Add lock contention stats

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

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 <mutex>
+#include <thread>
+#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

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

clients/drcachesim/scheduler/scheduler.cpp

@@ -687,6 +687,12 @@ scheduler_tmpl_t<RecordType, ReaderType>::~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 <typename RecordType, typename ReaderType>

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<app_pc>(base_addr), nullptr, true);
     assert(pc != nullptr);