Bug 1697479 - Turn extra GC poisoning off by default in nightly builds r=sfink,perftest-reviewers,sparky

This changes the default so that extra GC poisoning is off in nightly opt builds unless enabled by an environment variable. This should give us more useful telemetry on nightly performance and make it easier for people trying to benchmark our browser. The poisoning is still enabled in debug builds.

The variable is renamed from JSGC_DISABLE_POISONING to JSGC_EXTRA_POISONING.

Differential Revision: https://phabricator.services.mozilla.com/D108419

js/src/jsfriendapi.cpp

@@ -612,8 +612,8 @@ extern JS_FRIEND_API bool JS::ForceLexicalInitialization(JSContext* cx,
 }
 
 extern JS_FRIEND_API int JS::IsGCPoisoning() {
-#ifdef JS_GC_POISONING
-  return !js::gDisablePoisoning;
+#ifdef JS_GC_ALLOW_EXTRA_POISONING
+  return js::gExtraPoisoningEnabled;
 #else
   return false;
 #endif

js/src/jsfriendapi.h

@@ -152,8 +152,8 @@ extern JS_FRIEND_API bool ForceLexicalInitialization(JSContext* cx,
 
 /**
  * Whether we are poisoning unused/released data for error detection. Governed
- * by the JS_GC_POISONING #ifdef as well as the $JSGC_DISABLE_POISONING
- * environment variable.
+ * by the JS_GC_ALLOW_EXTRA_POISONING #ifdef as well as the
+ * $JSGC_EXTRA_POISONING environment variable.
  */
 extern JS_FRIEND_API int IsGCPoisoning();
 

js/src/util/Poison.h

@@ -25,9 +25,13 @@
 #include "js/Value.h"
 #include "util/DiagnosticAssertions.h"
 
-/* Enable poisoning in crash-diagnostics and zeal builds. */
+/*
+ * Allow extra GC poisoning to be enabled in crash-diagnostics and zeal
+ * builds. Except in debug builds, this must be enabled by setting the
+ * JSGC_EXTRA_POISONING environment variable.
+ */
 #if defined(JS_CRASH_DIAGNOSTICS) || defined(JS_GC_ZEAL)
-#  define JS_GC_POISONING 1
+#  define JS_GC_ALLOW_EXTRA_POISONING 1
 #endif
 
 namespace mozilla {
@@ -182,16 +186,17 @@ static inline void AlwaysPoison(void* ptr, uint8_t value, size_t num,
   SetMemCheckKind(ptr, num, kind);
 }
 
-// JSGC_DISABLE_POISONING environment variable
-extern bool gDisablePoisoning;
+#if defined(JS_GC_ALLOW_EXTRA_POISONING)
+extern bool gExtraPoisoningEnabled;
+#endif
 
-// Poison a region of memory in debug and nightly builds (plus builds where GC
-// zeal is configured). Can be disabled by setting the JSGC_DISABLE_POISONING
-// environment variable.
+// Conditionally poison a region of memory in debug builds and nightly builds
+// when enabled by setting the JSGC_EXTRA_POISONING environment variable. Used
+// by the GC in places where poisoning has a performance impact.
 static inline void Poison(void* ptr, uint8_t value, size_t num,
                           MemCheckKind kind) {
-#if defined(JS_GC_POISONING)
-  if (!js::gDisablePoisoning) {
+#if defined(JS_GC_ALLOW_EXTRA_POISONING)
+  if (js::gExtraPoisoningEnabled) {
     PoisonImpl(ptr, value, num);
   }
 #endif
@@ -199,7 +204,7 @@ static inline void Poison(void* ptr, uint8_t value, size_t num,
 }
 
 // Poison a region of memory in debug builds. Can be disabled by setting the
-// JSGC_DISABLE_POISONING environment variable.
+// JSGC_EXTRA_POISONING environment variable.
 static inline void DebugOnlyPoison(void* ptr, uint8_t value, size_t num,
                                    MemCheckKind kind) {
 #if defined(DEBUG)

js/src/util/Utility.cpp

@@ -93,7 +93,11 @@ void InitLargeAllocLimit() {
 }  // namespace js
 #endif
 
-bool js::gDisablePoisoning = false;
+#ifdef DEBUG
+bool js::gExtraPoisoningEnabled = true;
+#else
+bool js::gExtraPoisoningEnabled = false;
+#endif
 
 JS_PUBLIC_DATA arena_id_t js::MallocArena;
 JS_PUBLIC_DATA arena_id_t js::ArrayBufferContentsArena;

js/src/vm/Initialization.cpp

@@ -157,7 +157,11 @@ JS_PUBLIC_API const char* JS::detail::InitWithFailureDiagnostic(
   js::oom::InitLargeAllocLimit();
 #endif
 
-  js::gDisablePoisoning = bool(getenv("JSGC_DISABLE_POISONING"));
+#if defined(JS_GC_ALLOW_EXTRA_POISONING)
+  if (getenv("JSGC_EXTRA_POISONING")) {
+    js::gExtraPoisoningEnabled = true;
+  }
+#endif
 
   js::InitMallocAllocator();
 

testing/jsshell/benchmark.py

@@ -114,9 +114,6 @@ class Benchmark(object):
         # Update the environment variables
         env = os.environ.copy()
 
-        # disable "GC poisoning" Bug# 1499043
-        env["JSGC_DISABLE_POISONING"] = "1"
-
         process_args = {
             "cmd": self.command,
             "cwd": self.path,

testing/mozharness/mozharness/mozilla/testing/raptor.py

@@ -1056,9 +1056,6 @@ class Raptor(
         env["SCRIPTSPATH"] = scripts_path
         env["EXTERNALTOOLSPATH"] = external_tools_path
 
-        # disable "GC poisoning" Bug# 1499043
-        env["JSGC_DISABLE_POISONING"] = "1"
-
         # Needed to load unsigned Raptor WebExt on release builds
         if self.is_release_build:
             env["MOZ_DISABLE_NONLOCAL_CONNECTIONS"] = "1"

testing/talos/talos/ttest.py

@@ -132,7 +132,6 @@ class TTest(object):
             setup.env["MOZ_INSTRUMENT_EVENT_LOOP_INTERVAL"] = "10"
             global_counters["responsiveness"] = []
 
-        setup.env["JSGC_DISABLE_POISONING"] = "1"
         setup.env["MOZ_DISABLE_NONLOCAL_CONNECTIONS"] = "1"
 
         # instantiate an object to hold test results