[PATCH] D40862: [scudo] Correct performance regression in Secondary

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad created this revision.

This wasn't noticed: `RoundUpTo` doesn't produce a constant expression, so the
sizes were not constant either. Enforce them to be static const, replace
`RoundUpTo` by its expression. The compiler can now optimize the associated
computations accordingly.

Also looking at the produced assembly, `PageSize` was fetched multiple times
during `Allocate`, so keep a local value of it. As a result it's fetched once
and kept in a register.


https://reviews.llvm.org/D40862

Files:
  lib/scudo/scudo_allocator_secondary.h


Index: lib/scudo/scudo_allocator_secondary.h
===================================================================
--- lib/scudo/scudo_allocator_secondary.h
+++ lib/scudo/scudo_allocator_secondary.h
@@ -24,16 +24,17 @@
 class ScudoLargeMmapAllocator {
  public:
   void Init() {
-    PageSize = GetPageSizeCached();
+    PageSizeCached = GetPageSizeCached();
   }
 
   void *Allocate(AllocatorStats *Stats, uptr Size, uptr Alignment) {
-    uptr UserSize = Size - AlignedChunkHeaderSize;
+    const uptr UserSize = Size - AlignedChunkHeaderSize;
     // The Scudo frontend prevents us from allocating more than
     // MaxAllowedMallocSize, so integer overflow checks would be superfluous.
     uptr MapSize = Size + AlignedReservedAddressRangeSize;
     if (Alignment > MinAlignment)
       MapSize += Alignment;
+    const uptr PageSize = PageSizeCached;
     MapSize = RoundUpTo(MapSize, PageSize);
     // Account for 2 guard pages, one before and one after the chunk.
     MapSize += 2 * PageSize;
@@ -79,7 +80,7 @@
     // Actually mmap the memory, preserving the guard pages on either side
     CHECK_EQ(MapBeg + PageSize,
              AddressRange.Map(MapBeg + PageSize, MapSize - 2 * PageSize));
-    uptr Ptr = UserBeg - AlignedChunkHeaderSize;
+    const uptr Ptr = UserBeg - AlignedChunkHeaderSize;
     ReservedAddressRange *StoredRange = getReservedAddressRange(Ptr);
     *StoredRange = AddressRange;
 
@@ -98,6 +99,7 @@
   void Deallocate(AllocatorStats *Stats, void *Ptr) {
     // Since we're unmapping the entirety of where the ReservedAddressRange
     // actually is, copy onto the stack.
+    const uptr PageSize = PageSizeCached;
     ReservedAddressRange AddressRange = *getReservedAddressRange(Ptr);
     {
       SpinMutexLock l(&StatsMutex);
@@ -113,7 +115,7 @@
     // Deduct PageSize as ReservedAddressRange size includes the trailing guard
     // page.
     uptr MapEnd = reinterpret_cast<uptr>(StoredRange->base()) +
-        StoredRange->size() - PageSize;
+        StoredRange->size() - PageSizeCached;
     return MapEnd - reinterpret_cast<uptr>(Ptr);
   }
 
@@ -126,12 +128,12 @@
     return getReservedAddressRange(reinterpret_cast<uptr>(Ptr));
   }
 
-  const uptr AlignedReservedAddressRangeSize =
-      RoundUpTo(sizeof(ReservedAddressRange), MinAlignment);
-  const uptr HeadersSize =
+  static const uptr AlignedReservedAddressRangeSize =
+      (sizeof(ReservedAddressRange) + MinAlignment - 1) & ~(MinAlignment - 1);
+  static const uptr HeadersSize =
       AlignedReservedAddressRangeSize + AlignedChunkHeaderSize;
 
-  uptr PageSize;
+  uptr PageSizeCached;
   SpinMutex StatsMutex;
 };
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40862.125622.patch
Type: text/x-patch
Size: 2636 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171205/22083fc7/attachment-0001.bin>