Bug 1361009 - Remove MOZ_WIDGET_GONK from the profiler. r=jseward.

This removes the need for PROFILER_LIKELY_MEMORY_CONSTRAINED.

The patch also removes PROFILE_JAVA, USE_FAULTY_LIB, CONFIG_CASE_1,
CONFIG_CASE_2 and replaces all their uses with GP_OS_linux or GP_OS_android.

Finally, the patch removes a bogus |defined(GP_OS_darwin)| condition in
platform-linux-lul.cpp.

--HG--
extra : rebase_source : 77d1c625d65ddf551ab8cd4b962ae48c1a54466c

tools/profiler/core/platform.cpp

@@ -44,7 +44,7 @@
 #include "GeckoTaskTracer.h"
 #endif
 
-#if defined(PROFILE_JAVA)
+#if defined(GP_OS_android)
 # include "FennecJNINatives.h"
 # include "FennecJNIWrappers.h"
 #endif
@@ -88,7 +88,7 @@ using namespace mozilla;
 
 mozilla::LazyLogModule gProfilerLog("prof");
 
-#if defined(PROFILE_JAVA)
+#if defined(GP_OS_android)
 class GeckoJavaSampler : public mozilla::java::GeckoJavaSampler::Natives<GeckoJavaSampler>
 {
 private:
@@ -258,7 +258,7 @@ class ActivePS
     // Filter out any features unavailable in this platform/configuration.
     aFeatures &= profiler_get_available_features();
 
-#if defined(PROFILE_JAVA)
+#if defined(GP_OS_android)
     if (!mozilla::jni::IsFennec()) {
       aFeatures &= ~ProfilerFeature::Java;
     }
@@ -1459,7 +1459,7 @@ StreamMetaJSCustomObject(PSLockRef aLock, SpliceableJSONWriter& aWriter)
   }
 }
 
-#if defined(PROFILE_JAVA)
+#if defined(GP_OS_android)
 static void
 BuildJavaThreadJSObject(SpliceableJSONWriter& aWriter)
 {
@@ -1564,7 +1564,7 @@ locked_profiler_stream_json_for_this_process(PSLockRef aLock,
                        CorePS::ProcessStartTime(aLock), aSinceTime);
     }
 
-#if defined(PROFILE_JAVA)
+#if defined(GP_OS_android)
     if (ActivePS::FeatureJava(aLock)) {
       java::GeckoJavaSampler::Pause();
 
@@ -2010,7 +2010,7 @@ profiler_init(void* aStackTop)
   MOZ_RELEASE_ASSERT(!CorePS::Exists());
 
   uint32_t features =
-#if defined(PROFILE_JAVA)
+#if defined(GP_OS_android)
                       ProfilerFeature::Java |
 #endif
                       ProfilerFeature::JS |
@@ -2043,7 +2043,7 @@ profiler_init(void* aStackTop)
     mozilla::tasktracer::InitTaskTracer();
 #endif
 
-#if defined(PROFILE_JAVA)
+#if defined(GP_OS_android)
     if (mozilla::jni::IsFennec()) {
       GeckoJavaSampler::Init();
     }
@@ -2262,7 +2262,7 @@ profiler_get_available_features()
   #undef ADD_FEATURE
 
   // Now remove features not supported on this platform/configuration.
-#if !defined(PROFILE_JAVA)
+#if !defined(GP_OS_android)
   ProfilerFeature::ClearJava(features);
 #endif
 #if !defined(HAVE_NATIVE_UNWIND)
@@ -2360,7 +2360,7 @@ locked_profiler_start(PSLockRef aLock, int aEntries, double aInterval,
   }
 #endif
 
-#if defined(PROFILE_JAVA)
+#if defined(GP_OS_android)
   if (ActivePS::FeatureJava(aLock)) {
     int javaInterval = interval;
     // Java sampling doesn't accurately keep up with 1ms sampling.

tools/profiler/core/platform.h

@@ -69,10 +69,6 @@ static inline pid_t gettid()
 #endif
 #endif
 
-#if defined(GP_OS_android) && !defined(MOZ_WIDGET_GONK)
-#define PROFILE_JAVA
-#endif
-
 extern mozilla::LazyLogModule gProfilerLog;
 
 // These are for MOZ_LOG="prof:3" or higher. It's the default logging level for

tools/profiler/core/shared-libraries-linux.cc

@@ -23,53 +23,22 @@
 
 #include "common/linux/file_id.h"
 #include <algorithm>
-
-
-// There are three different configuration cases:
-//
-// (1) GP_OS_linux
-//       Use dl_iterate_phdr for almost everything and /proc/self/{exe,maps}
-//       to identify the main executable name.
-//
-// (2) GP_OS_android non-GONK
-//       If dl_iterate_phdr doesn't exist, give up immediately.  Otherwise use
-//       dl_iterate_phdr for almost all info and /proc/self/maps to get the
-//       mapping for /dev/ashmem/dalvik-jit-code-cache.
-//
-// (3) GP_OS_android GONK
-//       Use /proc/self/maps for everything.
-
-#undef CONFIG_CASE_1
-#undef CONFIG_CASE_2
-#undef CONFIG_CASE_3
+#include <dlfcn.h>
+#include <features.h>
+#include <sys/types.h>
 
 #if defined(GP_OS_linux)
-# define CONFIG_CASE_1 1
-# include <link.h> // dl_phdr_info
-# include <features.h>
-# include <dlfcn.h>
-# include <sys/types.h>
-
-#elif defined(GP_OS_android) && !defined(MOZ_WIDGET_GONK)
-# define CONFIG_CASE_2 1
+# include <link.h>      // dl_phdr_info
+#elif defined(GP_OS_android)
 # include "ElfLoader.h" // dl_phdr_info
-# include <features.h>
-# include <dlfcn.h>
-# include <sys/types.h>
 extern "C" MOZ_EXPORT __attribute__((weak))
 int dl_iterate_phdr(
           int (*callback)(struct dl_phdr_info *info, size_t size, void *data),
           void *data);
-
-#elif defined(GP_OS_android) && defined(MOZ_WIDGET_GONK)
-# define CONFIG_CASE_3 1
-  // No config-specific includes.
-
 #else
 # error "Unexpected configuration"
 #endif
 
-
 // Get the breakpad Id for the binary file pointed by bin_name
 static std::string getId(const char *bin_name)
 {
@@ -107,8 +76,6 @@ SharedLibraryAtPath(const char* path, unsigned long libStart,
                        "", "");
 }
 
-// Config cases (1) and (2) use dl_iterate_phdr.
-#if defined(CONFIG_CASE_1) || defined(CONFIG_CASE_2)
 static int
 dl_iterate_callback(struct dl_phdr_info *dl_info, size_t size, void *data)
 {
@@ -137,14 +104,12 @@ dl_iterate_callback(struct dl_phdr_info *dl_info, size_t size, void *data)
 
   return 0;
 }
-#endif // config cases (1) and (2)
-
 
 SharedLibraryInfo SharedLibraryInfo::GetInfoForSelf()
 {
   SharedLibraryInfo info;
 
-#if defined(CONFIG_CASE_1)
+#if defined(GP_OS_linux)
   // We need to find the name of the executable (exeName, exeNameLen) and the
   // address of its executable section (exeExeAddr) in the running image.
   char exeName[PATH_MAX];
@@ -165,7 +130,7 @@ SharedLibraryInfo SharedLibraryInfo::GetInfoForSelf()
   unsigned long exeExeAddr = 0;
 #endif
 
-#if defined(CONFIG_CASE_2)
+#if defined(GP_OS_android)
   // If dl_iterate_phdr doesn't exist, we give up immediately.
   if (!dl_iterate_phdr) {
     // On ARM Android, dl_iterate_phdr is provided by the custom linker.
@@ -176,9 +141,7 @@ SharedLibraryInfo SharedLibraryInfo::GetInfoForSelf()
   }
 #endif
 
-  // Read info from /proc/self/maps.  We do this for all config cases, but only
-  // in case (3) are we building the module list from that information.  For
-  // cases (1) and (2) we're just collecting some auxiliary information.
+  // Read info from /proc/self/maps. We ignore most of it.
   pid_t pid = getpid();
   char path[PATH_MAX];
   SprintfLiteral(path, "/proc/%d/maps", pid);
@@ -204,49 +167,30 @@ SharedLibraryInfo SharedLibraryInfo::GetInfoForSelf()
       continue;
     }
 
-#if defined(CONFIG_CASE_1)
+#if defined(GP_OS_linux)
     // Try to establish the main executable's load address.
     if (exeNameLen > 0 && strcmp(modulePath, exeName) == 0) {
       exeExeAddr = start;
     }
-    continue;
-    // NOTREACHED
-#elif defined(CONFIG_CASE_2)
+#elif defined(GP_OS_android)
     // Use /proc/pid/maps to get the dalvik-jit section since it has no
     // associated phdrs.
-    if (0 != strcmp(modulePath, "/dev/ashmem/dalvik-jit-code-cache")) {
-      continue;
-    }
-    // Otherwise proceed to the tail of the loop, so as to record the entry.
-#elif defined(CONFIG_CASE_3)
-    if (strcmp(perm, "r-xp") != 0) {
-      // Ignore entries that are writable and/or shared.
-      // At least one graphics driver uses short-lived "rwxs" mappings
-      // (see bug 926734 comment 5), so just checking for 'x' isn't enough.
-      continue;
-    }
-    // Record all other entries.
-#endif
-
-#if !defined(CONFIG_CASE_1)
-    // This section has to be conditionalised so as to avoid compiler warnings
-    // about dead code in case (1).
-    info.AddSharedLibrary(SharedLibraryAtPath(modulePath, start, end, offset));
-    if (info.GetSize() > 10000) {
-      LOG("SharedLibraryInfo::GetInfoForSelf(): "
-          "implausibly large number of mappings acquired");
-      break;
+    if (0 == strcmp(modulePath, "/dev/ashmem/dalvik-jit-code-cache")) {
+      info.AddSharedLibrary(SharedLibraryAtPath(modulePath, start, end,
+                                                offset));
+      if (info.GetSize() > 10000) {
+        LOG("SharedLibraryInfo::GetInfoForSelf(): "
+            "implausibly large number of mappings acquired");
+        break;
+      }
     }
 #endif
   }
 
-#if defined(CONFIG_CASE_1) || defined(CONFIG_CASE_2)
-  // For config cases (1) and (2), we collect the bulk of the library info using
-  // dl_iterate_phdr.
+  // We collect the bulk of the library info using dl_iterate_phdr.
   dl_iterate_phdr(dl_iterate_callback, &info);
-#endif
 
-#if defined(CONFIG_CASE_1)
+#if defined(GP_OS_linux)
   // Make another pass over the information we just harvested from
   // dl_iterate_phdr.  If we see a nameless object mapped at what we earlier
   // established to be the main executable's load address, attach the

tools/profiler/lul/AutoObjectMapper.cpp

@@ -16,7 +16,7 @@
 #include "PlatformMacros.h"
 #include "AutoObjectMapper.h"
 
-#if defined(USE_FAULTY_LIB)
+#if defined(GP_OS_android)
 # include <dlfcn.h>
 # include "mozilla/Types.h"
   // FIXME move these out of mozglue/linker/ElfLoader.h into their
@@ -108,7 +108,7 @@ bool AutoObjectMapperPOSIX::Map(/*OUT*/void** start, /*OUT*/size_t* length,
 }
 
 
-#if defined(USE_FAULTY_LIB)
+#if defined(GP_OS_android)
 // A helper function for AutoObjectMapperFaultyLib::Map.  Finds out
 // where the installation's lib directory is, since we'll have to look
 // in there to get hold of libmozglue.so.  Returned C string is heap
@@ -204,4 +204,4 @@ bool AutoObjectMapperFaultyLib::Map(/*OUT*/void** start, /*OUT*/size_t* length,
   }
 }
 
-#endif // defined(USE_FAULTY_LIB)
+#endif // defined(GP_OS_android)

tools/profiler/lul/AutoObjectMapper.h

@@ -12,10 +12,6 @@
 #include "mozilla/Attributes.h"
 #include "PlatformMacros.h"
 
-#if defined(GP_OS_android) && !defined(MOZ_WIDGET_GONK)
-#define USE_FAULTY_LIB
-#endif
-
 // A (nearly-) RAII class that maps an object in and then unmaps it on
 // destruction.  This base class version uses the "normal" POSIX
 // functions: open, fstat, close, mmap, munmap.
@@ -65,7 +61,7 @@ class MOZ_STACK_CLASS AutoObjectMapperPOSIX {
   void  operator delete[](void*);
 };
 
-#if defined(USE_FAULTY_LIB)
+#if defined(GP_OS_android)
 // This is a variant of AutoObjectMapperPOSIX suitable for use in
 // conjunction with faulty.lib on Android.  How it behaves depends on
 // the name of the file to be mapped.  There are three possible cases:
@@ -113,6 +109,6 @@ class MOZ_STACK_CLASS AutoObjectMapperFaultyLib : public AutoObjectMapperPOSIX {
   void  operator delete[](void*);
 };
 
-#endif // defined(USE_FAULTY_LIB)
+#endif // defined(GP_OS_android)
 
 #endif // AutoObjectMapper_h

tools/profiler/lul/platform-linux-lul.cpp

@@ -26,15 +26,15 @@ read_procmaps(lul::LUL* aLUL)
 {
   MOZ_ASSERT(aLUL->CountMappings() == 0);
 
-# if defined(GP_OS_linux) || defined(GP_OS_android) || defined(GP_OS_darwin)
+# if defined(GP_OS_linux) || defined(GP_OS_android)
   SharedLibraryInfo info = SharedLibraryInfo::GetInfoForSelf();
 
   for (size_t i = 0; i < info.GetSize(); i++) {
     const SharedLibrary& lib = info.GetEntry(i);
 
     std::string nativePath = lib.GetNativeDebugPath();
 
-#   if defined(USE_FAULTY_LIB)
+#   if defined(GP_OS_android)
     // We're using faulty.lib.  Use a special-case object mapper.
     AutoObjectMapperFaultyLib mapper(aLUL->mLog);
 #   else

tools/profiler/public/GeckoProfiler.h

@@ -452,14 +452,7 @@ void profiler_add_marker(const char *aMarker,
 # define PROFILER_PLATFORM_TRACING(name)
 #endif
 
-// FIXME/bug 789667: memory constraints wouldn't much of a problem for this
-// small a sample buffer size, except that serializing the profile data is
-// extremely, unnecessarily memory intensive.
-#ifdef MOZ_WIDGET_GONK
-# define PROFILER_LIKELY_MEMORY_CONSTRAINED
-#endif
-
-#if !defined(PROFILER_LIKELY_MEMORY_CONSTRAINED) && !defined(ARCH_ARMV6)
+#if !defined(ARCH_ARMV6)
 # define PROFILER_DEFAULT_ENTRIES 1000000
 #else
 # define PROFILER_DEFAULT_ENTRIES 100000
@@ -469,15 +462,7 @@ void profiler_add_marker(const char *aMarker,
 // for a single backtrace.
 #define PROFILER_GET_BACKTRACE_ENTRIES 1000
 
-// A 1ms sampling interval has been shown to be a large perf hit (10fps) on
-// memory-constrained (low-end) platforms, and additionally to yield different
-// results from the profiler. Where this is the important case, b2g, there are
-// also many gecko processes which magnify these effects.
-#if defined(PROFILER_LIKELY_MEMORY_CONSTRAINED)
-# define PROFILER_DEFAULT_INTERVAL 10
-#else
-# define PROFILER_DEFAULT_INTERVAL 1
-#endif
+#define PROFILER_DEFAULT_INTERVAL 1
 
 namespace mozilla {