Bug 1413096 - Replace ffs with MathAlgorithms functions. r?njn draft
authorMike Hommey <mh+mozilla@glandium.org>
Mon, 30 Oct 2017 17:44:16 +0900
changeset 689817 479baef24e221af0b4008b947319137d380a4b1a
parent 689816 9660a148afc288c819cc25bbe89995947c80c55d
child 689818 226f802512f03734b32678b93e28698853251127
push id87110
push userbmo:mh+mozilla@glandium.org
push dateWed, 01 Nov 2017 00:18:09 +0000
reviewersnjn
bugs1413096
milestone58.0a1
Bug 1413096 - Replace ffs with MathAlgorithms functions. r?njn - In the cases where it's used on powers of 2, replace it with FloorLog2() + 1. - In the cases where it's used on any kind of number, replace it with CountTrailingZeroes, which is `ffs(x) - 1`. - In the case of tiny allocations in arena_t::MallocSmall, we rearrange the code so that the intent is clearer, which also simplifies the expression for the mBins offset: mBins[0] is the first tiny bucket, for allocations of sizes 1 << TINY_MIN_2POW, mBins[1] for allocations of size 1 << (TINY_MIN_2POW + 1), etc. up to small_min. So the offset is really the log2 of the normalized size.
memory/build/mozjemalloc.cpp
--- a/memory/build/mozjemalloc.cpp
+++ b/memory/build/mozjemalloc.cpp
@@ -158,29 +158,16 @@ using namespace mozilla;
 // Some defines from the CRT internal headers that we need here.
 #define _CRT_SPINCOUNT 5000
 #include <io.h>
 #include <windows.h>
 #include <intrin.h>
 
 #define STDERR_FILENO 2
 
-// Use MSVC intrinsics.
-#pragma intrinsic(_BitScanForward)
-static __forceinline int
-ffs(int x)
-{
-  unsigned long i;
-
-  if (_BitScanForward(&i, x) != 0) {
-    return i + 1;
-  }
-  return 0;
-}
-
 // Implement getenv without using malloc.
 static char mozillaMallocOptionsBuf[64];
 
 #define getenv xgetenv
 static char*
 getenv(const char* name)
 {
 
@@ -225,19 +212,16 @@ typedef long ssize_t;
 #include <pthread.h>
 #include <sched.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
-#ifndef XP_DARWIN
-#include <strings.h>
-#endif
 #include <unistd.h>
 
 #ifdef XP_DARWIN
 #include <libkern/OSAtomic.h>
 #include <mach/mach_error.h>
 #include <mach/mach_init.h>
 #include <mach/vm_map.h>
 #include <malloc/malloc.h>
@@ -2199,17 +2183,17 @@ arena_run_reg_alloc(arena_run_t* run, ar
 
   // Move the first check outside the loop, so that run->regs_minelm can
   // be updated unconditionally, without the possibility of updating it
   // multiple times.
   i = run->regs_minelm;
   mask = run->regs_mask[i];
   if (mask != 0) {
     // Usable allocation found.
-    bit = ffs((int)mask) - 1;
+    bit = CountTrailingZeroes32(mask);
 
     regind = ((i << (SIZEOF_INT_2POW + 3)) + bit);
     MOZ_ASSERT(regind < bin->nregs);
     ret =
       (void*)(((uintptr_t)run) + bin->reg0_offset + (bin->reg_size * regind));
 
     // Clear bit.
     mask ^= (1U << bit);
@@ -2217,17 +2201,17 @@ arena_run_reg_alloc(arena_run_t* run, ar
 
     return ret;
   }
 
   for (i++; i < bin->regs_mask_nelms; i++) {
     mask = run->regs_mask[i];
     if (mask != 0) {
       // Usable allocation found.
-      bit = ffs((int)mask) - 1;
+      bit = CountTrailingZeroes32(mask);
 
       regind = ((i << (SIZEOF_INT_2POW + 3)) + bit);
       MOZ_ASSERT(regind < bin->nregs);
       ret =
         (void*)(((uintptr_t)run) + bin->reg0_offset + (bin->reg_size * regind));
 
       // Clear bit.
       mask ^= (1U << bit);
@@ -2974,33 +2958,29 @@ arena_t::MallocSmall(size_t aSize, bool 
 {
   void* ret;
   arena_bin_t* bin;
   arena_run_t* run;
 
   if (aSize < small_min) {
     // Tiny.
     aSize = RoundUpPow2(aSize);
-    bin = &mBins[ffs((int)(aSize >> (TINY_MIN_2POW + 1)))];
-
-    // Bin calculation is always correct, but we may need
-    // to fix size for the purposes of assertions and/or
-    // stats accuracy.
     if (aSize < (1U << TINY_MIN_2POW)) {
       aSize = 1U << TINY_MIN_2POW;
     }
+    bin = &mBins[FloorLog2(aSize >> TINY_MIN_2POW)];
   } else if (aSize <= small_max) {
     // Quantum-spaced.
     aSize = QUANTUM_CEILING(aSize);
     bin = &mBins[ntbins + (aSize >> QUANTUM_2POW_MIN) - 1];
   } else {
     // Sub-page.
     aSize = RoundUpPow2(aSize);
     bin = &mBins[ntbins + nqbins +
-                 (ffs((int)(aSize >> SMALL_MAX_2POW_DEFAULT)) - 2)];
+                 (FloorLog2(aSize >> SMALL_MAX_2POW_DEFAULT) - 1)];
   }
   MOZ_DIAGNOSTIC_ASSERT(aSize == bin->reg_size);
 
   {
     MutexAutoLock lock(mLock);
     if ((run = bin->runcur) && run->nfree > 0) {
       ret = MallocBinEasy(bin, run);
     } else {
@@ -4115,17 +4095,19 @@ static
     _malloc_message(
       _getprogname(),
       "Compile-time page size does not divide the runtime one.\n");
     MOZ_CRASH();
   }
 #else
   pagesize = (size_t)result;
   pagesize_mask = (size_t)result - 1;
-  pagesize_2pow = ffs((int)result) - 1;
+  pagesize_2pow = FloorLog2(result);
+  MOZ_RELEASE_ASSERT(1ULL << pagesize_2pow == pagesize,
+                     "Page size is not a power of two");
 #endif
 
   // Get runtime configuration.
   if ((opts = getenv("MALLOC_OPTIONS"))) {
     for (i = 0; opts[i] != '\0'; i++) {
       unsigned j, nreps;
       bool nseen;