Bug 1496362 - Limit control of tracelogger to only the gecko profiler and the previous environment variables implementation that triggers tracelogger only on startup. r=sfink
authorDenis Palmeiro <dpalmeiro@mozilla.com>
Wed, 10 Oct 2018 18:36:53 +0000
changeset 496331 71160e8bcfb743129388fa3d3948a8d036263785
parent 496330 6bfe39bf5e4dd380f6caa96e0e65ab9cdb149a71
child 496332 2c21c87e773ee1bfd796079fe5ffe06c2057b9ae
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1496362
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1496362 - Limit control of tracelogger to only the gecko profiler and the previous environment variables implementation that triggers tracelogger only on startup. r=sfink Tracelogger is currently not setup to be toggled on and off while in the shell. Removing access to the "jit.enable-tracelogger" will force the tracelogger to be invoked either through the new tracelogger API which is intended to be used by the profiler, or through setting the environment variables TLDIR, TLOPTIONS, and TLLOG. Differential Revision: https://phabricator.services.mozilla.com/D8250
js/src/jit-test/tests/tracelogger/bug1138265.js
js/src/jsapi.cpp
js/src/jsapi.h
js/src/shell/js.cpp
js/src/vm/TraceLogging.cpp
--- a/js/src/jit-test/tests/tracelogger/bug1138265.js
+++ b/js/src/jit-test/tests/tracelogger/bug1138265.js
@@ -1,9 +1,8 @@
-setJitCompilerOption("jit.enable-tracelogger", 1);
 try {
     (function(b, foreign, p) {
          "use asm"
          var ff = foreign.ff
          function f() {
             ff() | 0
          }
          return f
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -6603,21 +6603,16 @@ JS_SetGlobalJitCompilerOption(JSContext*
             jit::DefaultJitOptions defaultValues;
             value = defaultValues.jumpThreshold;
         }
         jit::JitOptions.jumpThreshold = value;
         break;
       case JSJITCOMPILER_TRACK_OPTIMIZATIONS:
         jit::JitOptions.disableOptimizationTracking = !value;
         break;
-#ifdef JS_TRACE_LOGGING
-      case JSJITCOMPILER_ENABLE_TRACELOGGER:
-        jit::JitOptions.enableTraceLogger = !!value;
-        break;
-#endif
       case JSJITCOMPILER_SPECTRE_INDEX_MASKING:
         jit::JitOptions.spectreIndexMasking = !!value;
         break;
       case JSJITCOMPILER_SPECTRE_OBJECT_MITIGATIONS_BARRIERS:
         jit::JitOptions.spectreObjectMitigationsBarriers = !!value;
         break;
       case JSJITCOMPILER_SPECTRE_OBJECT_MITIGATIONS_MISC:
         jit::JitOptions.spectreObjectMitigationsMisc = !!value;
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -4510,17 +4510,16 @@ JS_SetOffthreadIonCompilationEnabled(JSC
     Register(ION_ENABLE, "ion.enable")                                      \
     Register(ION_CHECK_RANGE_ANALYSIS, "ion.check-range-analysis")          \
     Register(ION_FREQUENT_BAILOUT_THRESHOLD, "ion.frequent-bailout-threshold") \
     Register(BASELINE_ENABLE, "baseline.enable")                            \
     Register(OFFTHREAD_COMPILATION_ENABLE, "offthread-compilation.enable")  \
     Register(FULL_DEBUG_CHECKS, "jit.full-debug-checks")                    \
     Register(JUMP_THRESHOLD, "jump-threshold")                              \
     Register(TRACK_OPTIMIZATIONS, "jit.track-optimizations")                \
-    Register(ENABLE_TRACELOGGER, "jit.enable-tracelogger")                  \
     Register(SIMULATOR_ALWAYS_INTERRUPT, "simulator.always-interrupt")      \
     Register(SPECTRE_INDEX_MASKING, "spectre.index-masking")                \
     Register(SPECTRE_OBJECT_MITIGATIONS_BARRIERS, "spectre.object-mitigations.barriers") \
     Register(SPECTRE_OBJECT_MITIGATIONS_MISC, "spectre.object-mitigations.misc") \
     Register(SPECTRE_STRING_MITIGATIONS, "spectre.string-mitigations")      \
     Register(SPECTRE_VALUE_MASKING, "spectre.value-masking")                \
     Register(SPECTRE_JIT_TO_CXX_CALLS, "spectre.jit-to-C++-calls")          \
     Register(WASM_FOLD_OFFSETS, "wasm.fold-offsets")                        \
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -10516,19 +10516,16 @@ main(int argc, char** argv, char** envp)
         || !op.addStringOption('z', "gc-zeal", "LEVEL(;LEVEL)*[,N]", gc::ZealModeHelpText)
 #else
         || !op.addStringOption('z', "gc-zeal", "LEVEL(;LEVEL)*[,N]", "option ignored in non-gc-zeal builds")
 #endif
         || !op.addStringOption('\0', "module-load-path", "DIR", "Set directory to load modules from")
         || !op.addBoolOption('\0', "no-async-stacks", "Disable async stacks")
         || !op.addMultiStringOption('\0', "dll", "LIBRARY", "Dynamically load LIBRARY")
         || !op.addBoolOption('\0', "suppress-minidump", "Suppress crash minidumps")
-#ifdef JS_TRACE_LOGGING
-        || !op.addBoolOption('\0', "enable-tracelogger","Enable Trace Logging")
-#endif
     )
     {
         return EXIT_FAILURE;
     }
 
     op.setArgTerminatesOptions("script", true);
     op.setArgCapturesRest("scriptArgs");
 
@@ -10582,22 +10579,16 @@ main(int argc, char** argv, char** envp)
         loader.load(path);
         dllPaths.popFront();
     }
 
     if (op.getBoolOption("suppress-minidump")) {
         js::NoteIntentionalCrash();
     }
 
-#ifdef JS_TRACE_LOGGING
-    if (op.getBoolOption("enable-tracelogger")) {
-        jit::JitOptions.enableTraceLogger = true;
-    }
-#endif
-
     if (!InitSharedObjectMailbox()) {
         return 1;
     }
 
     JS::SetProcessBuildIdOp(ShellBuildId);
 
     // The fake CPU count must be set before initializing the Runtime,
     // which spins up the thread pool.
--- a/js/src/vm/TraceLogging.cpp
+++ b/js/src/vm/TraceLogging.cpp
@@ -669,16 +669,17 @@ TraceLoggerThread::startEvent(uint32_t i
                 graph->addTextId(otherId, text);
             } else {
                 TraceLoggerEventPayload *p = traceLoggerState->getPayload(id);
                 if (p) {
                     const char *filename = traceLoggerState->maybeEventText(p);
                     mozilla::Maybe<uint32_t> line   = p->line();
                     mozilla::Maybe<uint32_t> column = p->column();
                     graph->addTextId(otherId, filename, line, column);
+                    p->release();
                 }
             }
         }
     }
 
     log(id);
 }
 
@@ -999,16 +1000,17 @@ TraceLoggerThreadState::init()
         if (strstr(options, "EnableOffThread")) {
             helperThreadEnabled = true;
         }
         if (strstr(options, "EnableGraph")) {
             graphEnabled = true;
         }
         if (strstr(options, "EnableGraphFile")) {
             graphFileEnabled = true;
+            jit::JitOptions.enableTraceLogger = true;
         }
         if (strstr(options, "Errors")) {
             spewErrors = true;
         }
     } else {
             mainThreadEnabled = true;
             helperThreadEnabled = true;
             graphEnabled = false;
@@ -1249,17 +1251,17 @@ JS_PUBLIC_API(void)
 JS::StartTraceLogger(JSContext *cx)
 {
     if (jit::JitOptions.enableTraceLogger || !traceLoggerState)  {
         return;
     }
 
     LockGuard<Mutex> guard(traceLoggerState->lock);
     traceLoggerState->enableTextIdsForProfiler();
-    JS_SetGlobalJitCompilerOption(cx, JSJITCOMPILER_ENABLE_TRACELOGGER, true);
+    jit::JitOptions.enableTraceLogger = true;
 
     // Reset the start time to profile start so it aligns with sampling.
     traceLoggerState->startupTime = rdtsc();
 
     if (cx->traceLogger) {
         cx->traceLogger->enable();
     }
 }
@@ -1268,13 +1270,13 @@ JS_PUBLIC_API(void)
 JS::StopTraceLogger(JSContext *cx)
 {
     if (!jit::JitOptions.enableTraceLogger || !traceLoggerState) {
         return;
     }
 
     LockGuard<Mutex> guard(traceLoggerState->lock);
     traceLoggerState->disableTextIdsForProfiler();
-    JS_SetGlobalJitCompilerOption(cx, JSJITCOMPILER_ENABLE_TRACELOGGER, false);
+    jit::JitOptions.enableTraceLogger = false;
     if (cx->traceLogger) {
         cx->traceLogger->disable();
     }
 }