Index: lib/sanitizer_common/sanitizer_quarantine.h =================================================================== --- lib/sanitizer_common/sanitizer_quarantine.h +++ lib/sanitizer_common/sanitizer_quarantine.h @@ -65,10 +65,13 @@ } void NOINLINE Drain(Cache *c, Callback cb) { + QuarantineBatch *batch_to_deallocate = nullptr; { SpinMutexLock l(&cache_mutex_); - cache_.Transfer(c); + batch_to_deallocate = cache_.Transfer(c); } + if (batch_to_deallocate) + cb.Deallocate(batch_to_deallocate); if (cache_.Size() > GetSize() && recycle_mutex_.TryLock()) Recycle(cb); } @@ -143,10 +146,37 @@ SizeAdd(size); } - void Transfer(QuarantineCache *c) { - list_.append_back(&c->list_); - SizeAdd(c->Size()); - atomic_store(&c->size_, 0, memory_order_relaxed); + QuarantineBatch *Transfer(QuarantineCache *from_cache) { + QuarantineBatch *batch_to_deallocate = nullptr; + if (list_.empty() || from_cache->list_.empty() || + from_cache->list_.front()->next || + from_cache->list_.front()->count + list_.back()->count > + QuarantineBatch::kSize) { + // Besides the trivial case of empty lists, there are either more than one + // batch to transfer or there're too many elements in the first batch and + // even if this cache's last batch is not full yet, it's too costly to + // move elements one by one, so we just accept memory penalty (the wasted + // space in this cache's last batch array). + // TODO(alekseyshl): consider transferring full batches and merge + // partially filled tail ones. + list_.append_back(&from_cache->list_); + SizeAdd(from_cache->Size()); + } else { + // Two tail batches can be merged maintaining batch and cache invariants. + QuarantineBatch *from_batch = from_cache->list_.front(); + from_cache->list_.pop_front(); + QuarantineBatch *to_batch = list_.back(); + for (uptr i = 0; i < from_batch->count; ++i) + to_batch->batch[to_batch->count + i] = from_batch->batch[i]; + to_batch->count += from_batch->count; + CHECK_GT(from_batch->size, sizeof(QuarantineBatch)); + uptr transferred_nodes_size = from_batch->size - sizeof(QuarantineBatch); + to_batch->size += transferred_nodes_size; + SizeAdd(transferred_nodes_size); + batch_to_deallocate = from_batch; + } + atomic_store(&from_cache->size_, 0, memory_order_relaxed); + return batch_to_deallocate; } void EnqueueBatch(QuarantineBatch *b) { Index: test/asan/TestCases/Linux/thread_local_quarantine_size_kb.cc =================================================================== --- test/asan/TestCases/Linux/thread_local_quarantine_size_kb.cc +++ test/asan/TestCases/Linux/thread_local_quarantine_size_kb.cc @@ -6,7 +6,7 @@ // RUN: %env_asan_opts=thread_local_quarantine_size_kb=64:quarantine_size_mb=64 %run %t 2>&1 | \ // RUN: FileCheck %s --allow-empty --check-prefix=CHECK-SMALL-LOCAL-CACHE-SMALL-OVERHEAD // RUN: %env_asan_opts=thread_local_quarantine_size_kb=0:quarantine_size_mb=64 %run %t 2>&1 | \ -// RUN: FileCheck %s --check-prefix=CHECK-NO-LOCAL-CACHE-HUGE-OVERHEAD +// RUN: FileCheck %s --check-prefix=CHECK-NO-LOCAL-CACHE-SMALL-OVERHEAD #include #include @@ -17,6 +17,8 @@ // total, and verify that quarantine does not incur too much memory overhead. // There's always an overhead for red zones, shadow memory and such, but // quarantine accounting should not significantly contribute to that. +// The zero sized thread local cache is specifically tested since it used to +// generate a huge overhead of almost empty quarantine batches. static const int kNumAllocs = 20000; static const int kAllocSize = 256; static const size_t kHeapSizeLimit = 12 << 20; @@ -37,4 +39,4 @@ // CHECK-VALUE: thread_local_quarantine_size_kb=256K // CHECK-SMALL-LOCAL-CACHE-SMALL-OVERHEAD-NOT: Heap size limit exceeded -// CHECK-NO-LOCAL-CACHE-HUGE-OVERHEAD: Heap size limit exceeded +// CHECK-NO-LOCAL-CACHE-SMALL-OVERHEAD-NOT: Heap size limit exceeded