[sanitizer] Atomic access to StackDepot stats

[eugenis]

compiler-rt/lib/sanitizer_common/sanitizer_stackdepotbase.h

130

Missing ;

136

This is a more expensive operation than the one used before. Probably does not matter, but please check that you are not slowing things down.

[vitalybuka]

rebase

[eugenis]

compiler-rt/lib/sanitizer_common/sanitizer_stackdepotbase.h

136

Did you look at perf with msan? Stackdepot is much busier with msan. This is an atomic r/m/w of a global location...

[eugenis]

compiler-rt/lib/sanitizer_common/sanitizer_stackdepotbase.h

136

In fact. Why can't we add up seq[i] numbers for each part when we read the stats, instead of maintaining an extra global counter?

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_stackdepotbase.h

136

no, but I run large test on Asan where Put() costs solid 6% and see no difference.
Note that this is a slow path. Only fraction of Most ::Put calls endup here, and we already acquired a lock.
We need try hard to slowdown this code if we change anything after line 116.

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_stackdepotbase.h

136

In fact. Why can't we add up seq[i] numbers for each part when we read the stats, instead of maintaining an extra global counter?

We can, but this is unrelated change.
This code is wrong as-is and this patch fixes it for free.
Later when I figure out what to do with our main issue - memory, I'll consider this micro optimizations.

compiler-rt/lib/asan/asan_stats.cpp

 }
 
 static void PrintAccumulatedStats() {
   AsanStats stats;
   GetAccumulatedStats(&stats);
   // Use lock to keep reports from mixing up.
   Lock lock(&print_lock);
   stats.Print();
-  StackDepotStats *stack_depot_stats = StackDepotGetStats();
+  StackDepotStats stack_depot_stats = StackDepotGetStats();
   Printf("Stats: StackDepot: %zd ids; %zdM allocated\n",
-         stack_depot_stats->n_uniq_ids, stack_depot_stats->allocated >> 20);
+         stack_depot_stats.n_uniq_ids, stack_depot_stats.allocated >> 20);
   PrintInternalAllocatorStats();
 }
 
 }  // namespace __asan
 
 // ---------------------- Interface ---------------- {{{1
 using namespace __asan;
 

compiler-rt/lib/hwasan/hwasan.cpp

 static void CheckUnwind() {
   GET_FATAL_STACK_TRACE_PC_BP(StackTrace::GetCurrentPc(), GET_CURRENT_FRAME());
   stack.Print();
 }
 
 static void HwasanFormatMemoryUsage(InternalScopedString &s) {
   HwasanThreadList &thread_list = hwasanThreadList();
   auto thread_stats = thread_list.GetThreadStats();
-  auto *sds = StackDepotGetStats();
+  auto sds = StackDepotGetStats();
   AllocatorStatCounters asc;
   GetAllocatorStats(asc);
   s.append(
       "HWASAN pid: %d rss: %zd threads: %zd stacks: %zd"
       " thr_aux: %zd stack_depot: %zd uniq_stacks: %zd"
       " heap: %zd",
       internal_getpid(), GetRSS(), thread_stats.n_live_threads,
       thread_stats.total_stack_size,
       thread_stats.n_live_threads * thread_list.MemoryUsedPerThread(),
-      sds->allocated, sds->n_uniq_ids, asc[AllocatorStatMapped]);
+      sds.allocated, sds.n_uniq_ids, asc[AllocatorStatMapped]);
 }
 
 #if SANITIZER_ANDROID
 static constexpr uptr kMemoryUsageBufferSize = 4096;
 
 static char *memory_usage_buffer = nullptr;
 
 static void InitMemoryUsage() {

compiler-rt/lib/memprof/memprof_stats.cpp

 }
 
 static void PrintAccumulatedStats() {
   MemprofStats stats;
   GetAccumulatedStats(&stats);
   // Use lock to keep reports from mixing up.
   Lock lock(&print_lock);
   stats.Print();
-  StackDepotStats *stack_depot_stats = StackDepotGetStats();
+  StackDepotStats stack_depot_stats = StackDepotGetStats();
   Printf("Stats: StackDepot: %zd ids; %zdM allocated\n",
-         stack_depot_stats->n_uniq_ids, stack_depot_stats->allocated >> 20);
+         stack_depot_stats.n_uniq_ids, stack_depot_stats.allocated >> 20);
   PrintInternalAllocatorStats();
 }
 
 } // namespace __memprof
 
 // ---------------------- Interface ---------------- {{{1
 using namespace __memprof;
 

compiler-rt/lib/msan/msan_chained_origin_depot.h

 #ifndef MSAN_CHAINED_ORIGIN_DEPOT_H
 #define MSAN_CHAINED_ORIGIN_DEPOT_H
 
 #include "sanitizer_common/sanitizer_common.h"
 
 namespace __msan {
 
 // Gets the statistic of the origin chain storage.
-StackDepotStats *ChainedOriginDepotGetStats();
+StackDepotStats ChainedOriginDepotGetStats();
 
 // Stores a chain with StackDepot ID here_id and previous chain ID prev_id.
 // If successful, returns true and the new chain id new_id.
 // If the same element already exists, returns false and sets new_id to the
 // existing ID.
 bool ChainedOriginDepotPut(u32 here_id, u32 prev_id, u32 *new_id);
 
 // Retrieves the stored StackDepot ID for the given origin ID.
 u32 ChainedOriginDepotGet(u32 id, u32 *other);
 
 void ChainedOriginDepotLockAll();
 void ChainedOriginDepotUnlockAll();
 
 }  // namespace __msan
 
 #endif  // MSAN_CHAINED_ORIGIN_DEPOT_H

compiler-rt/lib/msan/msan_report.cpp

void ReportExpectedUMRNotFound(StackTrace *stack) {
   Printf("WARNING: Expected use of uninitialized value not found\n");
   stack->Print();
 }
 
 void ReportStats() {
   ScopedErrorReportLock l;
 
   if (__msan_get_track_origins() > 0) {
-    StackDepotStats *stack_depot_stats = StackDepotGetStats();
+    StackDepotStats stack_depot_stats = StackDepotGetStats();
     // FIXME: we want this at normal exit, too!
     // FIXME: but only with verbosity=1 or something
-    Printf("Unique heap origins: %zu\n", stack_depot_stats->n_uniq_ids);
-    Printf("Stack depot allocated bytes: %zu\n", stack_depot_stats->allocated);
+    Printf("Unique heap origins: %zu\n", stack_depot_stats.n_uniq_ids);
+    Printf("Stack depot allocated bytes: %zu\n", stack_depot_stats.allocated);
 
-    StackDepotStats *chained_origin_depot_stats = ChainedOriginDepotGetStats();
+    StackDepotStats chained_origin_depot_stats = ChainedOriginDepotGetStats();
     Printf("Unique origin histories: %zu\n",
-           chained_origin_depot_stats->n_uniq_ids);
+           chained_origin_depot_stats.n_uniq_ids);
     Printf("History depot allocated bytes: %zu\n",
-           chained_origin_depot_stats->allocated);
+           chained_origin_depot_stats.allocated);
   }
 }
 
 void ReportAtExitStatistics() {
   ScopedErrorReportLock l;
 
   if (msan_report_count > 0) {
     Decorator d;

compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cpp

while (true) {
     const uptr current_rss_mb = GetRSS() >> 20;
     if (Verbosity()) {
       // If RSS has grown 10% since last time, print some information.
       if (prev_reported_rss * 11 / 10 < current_rss_mb) {
         Printf("%s: RSS: %zdMb\n", SanitizerToolName, current_rss_mb);
         prev_reported_rss = current_rss_mb;
       }
       // If stack depot has grown 10% since last time, print it too.
-      StackDepotStats *stack_depot_stats = StackDepotGetStats();
-      if (stack_depot_stats) {
-        if (prev_reported_stack_depot_size * 11 / 10 <
-            stack_depot_stats->allocated) {
-          Printf("%s: StackDepot: %zd ids; %zdM allocated\n",
-                 SanitizerToolName,
-                 stack_depot_stats->n_uniq_ids,
-                 stack_depot_stats->allocated >> 20);
-          prev_reported_stack_depot_size = stack_depot_stats->allocated;
-        }
+      StackDepotStats stack_depot_stats = StackDepotGetStats();
+          stack_depot_stats.allocated) {
+        Printf("%s: StackDepot: %zd ids; %zdM allocated\n", SanitizerToolName,
+               stack_depot_stats.n_uniq_ids, stack_depot_stats.allocated >> 20);
+        prev_reported_stack_depot_size = stack_depot_stats.allocated;
       }
     }
     // Check RSS against the limit.
     if (hard_rss_limit_mb && hard_rss_limit_mb < current_rss_mb) {
       Report("%s: hard rss limit exhausted (%zdMb vs %zdMb)\n",
              SanitizerToolName, hard_rss_limit_mb, current_rss_mb);
       DumpProcessMap();
       Die();

compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.h

struct StackDepotHandle {
   bool valid() { return node_; }
   u32 id();
   int use_count();
   void inc_use_count_unsafe();
 };
 
 const int kStackDepotMaxUseCount = 1U << (SANITIZER_ANDROID ? 16 : 20);
 
-StackDepotStats *StackDepotGetStats();
+StackDepotStats StackDepotGetStats();
 u32 StackDepotPut(StackTrace stack);
 StackDepotHandle StackDepotPut_WithHandle(StackTrace stack);
 // Retrieves a stored stack trace by the id.
 StackTrace StackDepotGet(u32 id);
 
 void StackDepotLockAll();
 void StackDepotUnlockAll();
 void StackDepotPrintAll();

compiler-rt/lib/sanitizer_common/sanitizer_stackdepot.cpp

void StackDepotHandle::inc_use_count_unsafe() {
   CHECK_LT(prev + 1, StackDepotNode::kMaxUseCount);
 }
 
 // FIXME(dvyukov): this single reserved bit is used in TSan.
 typedef StackDepotBase<StackDepotNode, 1, StackDepotNode::kTabSizeLog>
     StackDepot;
 static StackDepot theDepot;
 
-StackDepotStats *StackDepotGetStats() {
-  return theDepot.GetStats();
-}
+StackDepotStats StackDepotGetStats() { return theDepot.GetStats(); }
 
 u32 StackDepotPut(StackTrace stack) {
   StackDepotHandle h = theDepot.Put(stack);
   return h.valid() ? h.id() : 0;
 }
 
 StackDepotHandle StackDepotPut_WithHandle(StackTrace stack) {
   return theDepot.Put(stack);

 
 bool StackDepotReverseMap::IdDescPair::IdComparator(
     const StackDepotReverseMap::IdDescPair &a,
     const StackDepotReverseMap::IdDescPair &b) {
   return a.id < b.id;
 }
 
 StackDepotReverseMap::StackDepotReverseMap() {
-  map_.reserve(StackDepotGetStats()->n_uniq_ids + 100);
+  map_.reserve(StackDepotGetStats().n_uniq_ids + 100);
   for (int idx = 0; idx < StackDepot::kTabSize; idx++) {
     atomic_uintptr_t *p = &theDepot.tab[idx];
     uptr v = atomic_load(p, memory_order_consume);
     StackDepotNode *s = (StackDepotNode*)(v & ~1);
     for (; s; s = s->link) {
       IdDescPair pair = {s->id, s};
       map_.push_back(pair);
     }

compiler-rt/lib/sanitizer_common/sanitizer_stackdepotbase.h

 //===----------------------------------------------------------------------===//
 
 #ifndef SANITIZER_STACKDEPOTBASE_H
 #define SANITIZER_STACKDEPOTBASE_H
 
 #include <stdio.h>
 
 #include "sanitizer_atomic.h"
+#include "sanitizer_common.h"
 #include "sanitizer_internal_defs.h"
 #include "sanitizer_mutex.h"
 #include "sanitizer_persistent_allocator.h"
+#include "sanitizer_stackdepot.h"
 
 namespace __sanitizer {
 
 template <class Node, int kReservedBits, int kTabSizeLog>
 class StackDepotBase {
  public:
   typedef typename Node::args_type args_type;
   typedef typename Node::handle_type handle_type;
   // Maps stack trace to an unique id.
   handle_type Put(args_type args, bool *inserted = nullptr);
   // Retrieves a stored stack trace by the id.
   args_type Get(u32 id);
 
-  StackDepotStats *GetStats() { return &stats; }
+  StackDepotStats GetStats() const {
+    return {
+        atomic_load_relaxed(&n_uniq_ids),
+        atomic_load_relaxed(&allocated),
+    };
+  }
 
   void LockAll();
   void UnlockAll();
   void PrintAll();
 
  private:
   static Node *find(Node *s, args_type args, u32 hash);
   static Node *lock(atomic_uintptr_t *p);
   static void unlock(atomic_uintptr_t *p, Node *s);
 
   static const int kTabSize = 1 << kTabSizeLog;  // Hash table size.
   static const int kPartBits = 8;
   static const int kPartShift = sizeof(u32) * 8 - kPartBits - kReservedBits;
   static const int kPartCount =
       1 << kPartBits;  // Number of subparts in the table.
   static const int kPartSize = kTabSize / kPartCount;
   static const int kMaxId = 1 << kPartShift;
 
   atomic_uintptr_t tab[kTabSize];   // Hash table of Node's.
   atomic_uint32_t seq[kPartCount];  // Unique id generators.
 
-  StackDepotStats stats;
+  atomic_uintptr_t n_uniq_ids;
+  atomic_uintptr_t allocated;
 
   friend class StackDepotReverseMap;
 };
 
 template <class Node, int kReservedBits, int kTabSizeLog>
 Node *StackDepotBase<Node, kReservedBits, kTabSizeLog>::find(Node *s,
                                                              args_type args,
                                                              u32 hash) {
if (s2 != s) {
     node = find(s2, args, h);
     if (node) {
       unlock(p, s2);
       return node->get_handle();
     }
   }
   uptr part = (h % kTabSize) / kPartSize;
   u32 id = atomic_fetch_add(&seq[part], 1, memory_order_relaxed) + 1;
-  stats.n_uniq_ids++;
-  CHECK_LT(id, kMaxId);
+  atomic_fetch_add(&n_uniq_ids, 1, memory_order_relaxed) CHECK_LT(id, kMaxId);
   id |= part << kPartShift;
   CHECK_NE(id, 0);
   CHECK_EQ(id & (((u32)-1) >> kReservedBits), id);
   uptr memsz = Node::storage_size(args);
   s = (Node *)PersistentAlloc(memsz);
-  stats.allocated += memsz;
+  atomic_fetch_add(&allocated, memsz, memory_order_relaxed);
   s->id = id;
   s->store(args, h);
   s->link = s2;
   unlock(p, s);
   if (inserted) *inserted = true;
   return s->get_handle();
 }
 

compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp

else
     mem[MemOther] += rss;
 }
 
 void WriteMemoryProfile(char *buf, uptr buf_size, u64 uptime_ns) {
   uptr mem[MemCount];
   internal_memset(mem, 0, sizeof(mem));
   GetMemoryProfile(FillProfileCallback, mem, MemCount);
   auto meta = ctx->metamap.GetMemoryStats();
-  StackDepotStats *stacks = StackDepotGetStats();
+  StackDepotStats stacks = StackDepotGetStats();
   uptr nthread, nlive;
   ctx->thread_registry.GetNumberOfThreads(&nthread, &nlive);
   uptr internal_stats[AllocatorStatCount];
   internal_allocator()->GetStats(internal_stats);
   // All these are allocated from the common mmap region.
-  mem[MemMmap] -= meta.mem_block + meta.sync_obj + stacks->allocated +
+  mem[MemMmap] -= meta.mem_block + meta.sync_obj + stacks.allocated +
                   internal_stats[AllocatorStatMapped];
   if (s64(mem[MemMmap]) < 0)
     mem[MemMmap] = 0;
   internal_snprintf(
       buf, buf_size,
       "%llus: RSS %zd MB: shadow:%zd meta:%zd file:%zd mmap:%zd"
       " trace:%zd heap:%zd other:%zd intalloc:%zd memblocks:%zd syncobj:%zu"
       " stacks=%zd[%zd] nthr=%zd/%zd\n",
       uptime_ns / (1000 * 1000 * 1000), mem[MemTotal] >> 20,
       mem[MemShadow] >> 20, mem[MemMeta] >> 20, mem[MemFile] >> 20,
       mem[MemMmap] >> 20, mem[MemTrace] >> 20, mem[MemHeap] >> 20,
       mem[MemOther] >> 20, internal_stats[AllocatorStatMapped] >> 20,
-      meta.mem_block >> 20, meta.sync_obj >> 20, stacks->allocated >> 20,
-      stacks->n_uniq_ids, nlive, nthread);
+      meta.mem_block >> 20, meta.sync_obj >> 20, stacks.allocated >> 20,
+      stacks.n_uniq_ids, nlive, nthread);
 }
 
 #  if SANITIZER_LINUX
 void FlushShadowMemoryCallback(
     const SuspendedThreadsList &suspended_threads_list,
     void *argument) {
   ReleaseMemoryPagesToOS(ShadowBeg(), ShadowEnd());
 }

compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp

#if !SANITIZER_GO
   RegionMemUsage(LoAppMemBeg(), LoAppMemEnd(), &low_res, &low_dirty);
   RegionMemUsage(HiAppMemBeg(), HiAppMemEnd(), &high_res, &high_dirty);
   RegionMemUsage(HeapMemBeg(), HeapMemEnd(), &heap_res, &heap_dirty);
 #else  // !SANITIZER_GO
   uptr app_res, app_dirty;
   RegionMemUsage(LoAppMemBeg(), LoAppMemEnd(), &app_res, &app_dirty);
 #endif
 
-  StackDepotStats *stacks = StackDepotGetStats();
+  StackDepotStats stacks = StackDepotGetStats();
   uptr nthread, nlive;
   ctx->thread_registry.GetNumberOfThreads(&nthread, &nlive);
-  internal_snprintf(buf, buf_size,
+  internal_snprintf(
+      buf, buf_size,
       "shadow   (0x%016zx-0x%016zx): resident %zd kB, dirty %zd kB\n"
       "meta     (0x%016zx-0x%016zx): resident %zd kB, dirty %zd kB\n"
       "traces   (0x%016zx-0x%016zx): resident %zd kB, dirty %zd kB\n"
-#if !SANITIZER_GO
+#  if !SANITIZER_GO
       "low app  (0x%016zx-0x%016zx): resident %zd kB, dirty %zd kB\n"
       "high app (0x%016zx-0x%016zx): resident %zd kB, dirty %zd kB\n"
       "heap     (0x%016zx-0x%016zx): resident %zd kB, dirty %zd kB\n"
-#else  // !SANITIZER_GO
+#  else  // !SANITIZER_GO
       "app      (0x%016zx-0x%016zx): resident %zd kB, dirty %zd kB\n"
-#endif
+#  endif
       "stacks: %zd unique IDs, %zd kB allocated\n"
       "threads: %zd total, %zd live\n"
       "------------------------------\n",
       ShadowBeg(), ShadowEnd(), shadow_res / 1024, shadow_dirty / 1024,
       MetaShadowBeg(), MetaShadowEnd(), meta_res / 1024, meta_dirty / 1024,
       TraceMemBeg(), TraceMemEnd(), trace_res / 1024, trace_dirty / 1024,
-#if !SANITIZER_GO
+#  if !SANITIZER_GO
       LoAppMemBeg(), LoAppMemEnd(), low_res / 1024, low_dirty / 1024,
       HiAppMemBeg(), HiAppMemEnd(), high_res / 1024, high_dirty / 1024,
       HeapMemBeg(), HeapMemEnd(), heap_res / 1024, heap_dirty / 1024,
-#else  // !SANITIZER_GO
+#  else  // !SANITIZER_GO
       LoAppMemBeg(), LoAppMemEnd(), app_res / 1024, app_dirty / 1024,
-#endif
-    stacks->n_uniq_ids, stacks->allocated / 1024,
-    nthread, nlive);
+#  endif
+      stacks.n_uniq_ids, stacks.allocated / 1024, nthread, nlive);
 }
 
 #  if !SANITIZER_GO
 void InitializeShadowMemoryPlatform() { }
 
 // On OS X, GCD worker threads are created without a call to pthread_create. We
 // need to properly register these threads with ThreadCreate and ThreadStart.
 // These threads don't have a parent thread, as they are created "spuriously".