Bug 1414168 - Base run offset calculations on the fixed header size, excluding regs_mask. r?njn draft
authorMike Hommey <mh+mozilla@glandium.org>
Wed, 08 Nov 2017 10:08:37 +0900
changeset 695336 bc593e927913071247505fc8b79d91887b673af6
parent 695335 59e637a91b0763cabd3ae10b23a8d98f5dd76218
child 695337 ae77d83b071ed824b6e5ae6dc2b6bee6bc3478b2
push id88395
push userbmo:mh+mozilla@glandium.org
push dateThu, 09 Nov 2017 03:01:34 +0000
reviewersnjn
bugs1414168
milestone58.0a1
Bug 1414168 - Base run offset calculations on the fixed header size, excluding regs_mask. r?njn On 64-bit platforms, sizeof(arena_run_t) includes a padding at the end of the struct to align to 64-bit, since the last field, regs_mask, is 32-bit, and its offset can be a multiple of 64-bit depending on the configuration. But we're doing size calculations for a dynamically-sized regs_mask based on sizeof(arena_run_t), completely ignoring that padding. Instead, we use the offset of regs_mask as a base for the calculation. Practically speaking, this doesn't change much with the current set of values, but could affect the overheads when we squeeze run sizes more.
memory/build/mozjemalloc.cpp
--- a/memory/build/mozjemalloc.cpp
+++ b/memory/build/mozjemalloc.cpp
@@ -2959,59 +2959,61 @@ arena_t::MallocBinHard(arena_bin_t* aBin
 // bin->mRunNumRegions, bin->mRunNumRegionsMask, and bin->mRunFirstRegionOffset are
 // also calculated here, since these settings are all interdependent.
 static size_t
 arena_bin_run_size_calc(arena_bin_t* bin, size_t min_run_size)
 {
   size_t try_run_size, good_run_size;
   unsigned good_nregs, good_mask_nelms, good_reg0_offset;
   unsigned try_nregs, try_mask_nelms, try_reg0_offset;
+  // Size of the run header, excluding regs_mask.
+  static const size_t kFixedHeaderSize = offsetof(arena_run_t, regs_mask);
 
   MOZ_ASSERT(min_run_size >= gPageSize);
   MOZ_ASSERT(min_run_size <= gMaxLargeClass);
 
   // Calculate known-valid settings before entering the mRunSize
   // expansion loop, so that the first part of the loop always copies
   // valid settings.
   //
   // The do..while loop iteratively reduces the number of regions until
   // the run header and the regions no longer overlap.  A closed formula
   // would be quite messy, since there is an interdependency between the
   // header's mask length and the number of regions.
   try_run_size = min_run_size;
-  try_nregs = ((try_run_size - sizeof(arena_run_t)) / bin->mSizeClass) +
+  try_nregs = ((try_run_size - kFixedHeaderSize) / bin->mSizeClass) +
               1; // Counter-act try_nregs-- in loop.
   do {
     try_nregs--;
     try_mask_nelms =
       (try_nregs >> (LOG2(sizeof(int)) + 3)) +
       ((try_nregs & ((1U << (LOG2(sizeof(int)) + 3)) - 1)) ? 1 : 0);
     try_reg0_offset = try_run_size - (try_nregs * bin->mSizeClass);
-  } while (sizeof(arena_run_t) + (sizeof(unsigned) * (try_mask_nelms - 1)) >
+  } while (kFixedHeaderSize + (sizeof(unsigned) * try_mask_nelms) >
            try_reg0_offset);
 
   // mRunSize expansion loop.
   while (true) {
     // Copy valid settings before trying more aggressive settings.
     good_run_size = try_run_size;
     good_nregs = try_nregs;
     good_mask_nelms = try_mask_nelms;
     good_reg0_offset = try_reg0_offset;
 
     // Try more aggressive settings.
     try_run_size += gPageSize;
-    try_nregs = ((try_run_size - sizeof(arena_run_t)) / bin->mSizeClass) +
+    try_nregs = ((try_run_size - kFixedHeaderSize) / bin->mSizeClass) +
                 1; // Counter-act try_nregs-- in loop.
     do {
       try_nregs--;
       try_mask_nelms =
         (try_nregs >> (LOG2(sizeof(int)) + 3)) +
         ((try_nregs & ((1U << (LOG2(sizeof(int)) + 3)) - 1)) ? 1 : 0);
       try_reg0_offset = try_run_size - (try_nregs * bin->mSizeClass);
-    } while (sizeof(arena_run_t) + (sizeof(unsigned) * (try_mask_nelms - 1)) >
+    } while (kFixedHeaderSize + (sizeof(unsigned) * try_mask_nelms) >
              try_reg0_offset);
 
     // Don't allow runs larger than the largest possible large size class.
     if (try_run_size > gMaxLargeClass) {
       break;
     }
 
     // Try to keep the run overhead below kRunOverhead.
@@ -3022,23 +3024,23 @@ arena_bin_run_size_calc(arena_bin_t* bin
     // The run header includes one bit per region of the given size. For sizes
     // small enough, the number of regions is large enough that growing the run
     // size barely moves the needle for the overhead because of all those bits.
     // For example, for a size of 8 bytes, adding 4KiB to the run size adds
     // close to 512 bits to the header, which is 64 bytes.
     // With such overhead, there is no way to get to the wanted overhead above,
     // so we give up if the required size for regs_mask more than doubles the
     // size of the run header.
-    if (try_mask_nelms * sizeof(unsigned) >= sizeof(arena_run_t)) {
+    if (try_mask_nelms * sizeof(unsigned) >= kFixedHeaderSize) {
       break;
     }
 
   }
 
-  MOZ_ASSERT(sizeof(arena_run_t) + (sizeof(unsigned) * (good_mask_nelms - 1)) <=
+  MOZ_ASSERT(kFixedHeaderSize + (sizeof(unsigned) * good_mask_nelms) <=
              good_reg0_offset);
   MOZ_ASSERT((good_mask_nelms << (LOG2(sizeof(int)) + 3)) >= good_nregs);
 
   // Copy final settings.
   bin->mRunSize = good_run_size;
   bin->mRunNumRegions = good_nregs;
   bin->mRunNumRegionsMask = good_mask_nelms;
   bin->mRunFirstRegionOffset = good_reg0_offset;