Bug 1551106 part 1. Stop using [array] in StartProfiler. r=gerald
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 21 May 2019 14:42:57 +0000
changeset 474756 f454d7e7502cdade0993677166911085b231963d
parent 474755 1dad417c28ad49a3fb16130ef3b9272dc424e2c9
child 474757 09369fc85b211379428243a53c1e5d00d95f99fb
push id36046
push useraiakab@mozilla.com
push dateTue, 21 May 2019 21:45:52 +0000
treeherdermozilla-central@257f2c96cef5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgerald
bugs1551106
milestone69.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 1551106 part 1. Stop using [array] in StartProfiler. r=gerald Differential Revision: https://phabricator.services.mozilla.com/D30967
devtools/client/webconsole/test/chrome/test_render_perf.html
devtools/server/actors/perf.js
devtools/server/performance/profiler.js
testing/talos/talos/pageloader/chrome/Profiler.js
testing/talos/talos/scripts/Profiler.js
testing/talos/talos/talos-powers/api.js
testing/talos/talos/tests/tart/addon/content/Profiler.js
testing/talos/talos/tests/tresize/addon/content/Profiler.js
toolkit/components/backgroundhangmonitor/tests/child_cause_hang.js
toolkit/components/backgroundhangmonitor/tests/test_BHRObserver.js
toolkit/components/extensions/parent/ext-geckoProfiler.js
tools/profiler/gecko/nsIProfiler.idl
tools/profiler/gecko/nsProfiler.cpp
tools/profiler/tests/browser/head.js
tools/profiler/tests/chrome/profiler_test_utils.js
tools/profiler/tests/xpcshell/test_asm.js
tools/profiler/tests/xpcshell/test_enterjit_osr.js
tools/profiler/tests/xpcshell/test_enterjit_osr_disabling.js
tools/profiler/tests/xpcshell/test_enterjit_osr_enabling.js
tools/profiler/tests/xpcshell/test_feature_mainthreadio.js
tools/profiler/tests/xpcshell/test_pause.js
tools/profiler/tests/xpcshell/test_run.js
tools/profiler/tests/xpcshell/test_start.js
--- a/devtools/client/webconsole/test/chrome/test_render_perf.html
+++ b/devtools/client/webconsole/test/chrome/test_render_perf.html
@@ -185,19 +185,17 @@ window.onload = async function() {
     interval: 1,
     features: ["js"],
     threads: ["GeckoMain"],
   };
   Services.profiler.StartProfiler(
     settings.entries,
     settings.interval,
     settings.features,
-    settings.features.length,
     settings.threads,
-    settings.threads.length,
   );
   info("Profiler has started");
 
   await wait(500);
 
   await testBulkLogging(wrapper);
 
   await wait(500);
--- a/devtools/server/actors/perf.js
+++ b/devtools/server/actors/perf.js
@@ -64,19 +64,17 @@ exports.PerfActor = ActorClassWithSpec(p
     };
 
     try {
       // This can throw an error if the profiler is in the wrong state.
       Services.profiler.StartProfiler(
         settings.entries,
         settings.interval,
         settings.features,
-        settings.features.length,
         settings.threads,
-        settings.threads.length,
         settings.duration
       );
     } catch (e) {
       // In case any errors get triggered, bailout with a false.
       return false;
     }
 
     return true;
--- a/devtools/server/performance/profiler.js
+++ b/devtools/server/performance/profiler.js
@@ -107,19 +107,17 @@ const ProfilerManager = (function() {
       // interested in.
       const currentTime = Services.profiler.getElapsedTime();
 
       try {
         Services.profiler.StartProfiler(
           config.entries,
           config.interval,
           config.features,
-          config.features.length,
           config.threadFilters,
-          config.threadFilters.length
         );
       } catch (e) {
         // For some reason, the profiler couldn't be started. This could happen,
         // for example, when in private browsing mode.
         Cu.reportError(`Could not start the profiler module: ${e.message}`);
         return { started: false, reason: e, currentTime };
       }
       this.started = true;
--- a/testing/talos/talos/pageloader/chrome/Profiler.js
+++ b/testing/talos/talos/pageloader/chrome/Profiler.js
@@ -87,18 +87,18 @@ var Profiler;
     },
     initFromURLQueryParams: function Profiler__initFromURLQueryParams(locationSearch) {
       this.initFromObject(searchToObject(locationSearch));
     },
     beginTest: function Profiler__beginTest(testName) {
       currentTest = testName;
       if (_profiler && enabled) {
         _profiler.StartProfiler(profiler_entries, profiler_interval,
-                                ["js", "leaf", "stackwalk", "threads"], 4,
-                                profiler_threadsArray, profiler_threadsArray.length);
+                                ["js", "leaf", "stackwalk", "threads"],
+                                profiler_threadsArray);
         if (_profiler.PauseSampling) {
           _profiler.PauseSampling();
         }
       }
     },
     finishTest: function Profiler__finishTest() {
       if (_profiler && enabled) {
         _profiler.dumpProfileToFile(profiler_dir + "/" + currentTest + ".profile");
--- a/testing/talos/talos/scripts/Profiler.js
+++ b/testing/talos/talos/scripts/Profiler.js
@@ -86,18 +86,18 @@ var Profiler;
     },
     initFromURLQueryParams: function Profiler__initFromURLQueryParams(locationSearch) {
       this.initFromObject(searchToObject(locationSearch));
     },
     beginTest: function Profiler__beginTest(testName) {
       currentTest = testName;
       if (_profiler && enabled) {
         _profiler.StartProfiler(profiler_entries, profiler_interval,
-                                ["js", "leaf", "stackwalk", "threads"], 4,
-                                profiler_threadsArray, profiler_threadsArray.length);
+                                ["js", "leaf", "stackwalk", "threads"],
+                                profiler_threadsArray);
         if (_profiler.PauseSampling) {
           _profiler.PauseSampling();
         }
       }
     },
     finishTest: function Profiler__finishTest() {
       if (_profiler && enabled) {
         _profiler.dumpProfileToFile(profiler_dir + "/" + currentTest + ".profile");
--- a/testing/talos/talos/talos-powers/api.js
+++ b/testing/talos/talos/talos-powers/api.js
@@ -104,18 +104,18 @@ TalosPowersService.prototype = {
    *          The sampling interval in milliseconds.
    *
    *        threadsArray (array of strings):
    *          The thread names to sample.
    */
   profilerBegin(data) {
     Services.profiler
             .StartProfiler(data.entries, data.interval,
-                           ["js", "leaf", "stackwalk", "threads"], 4,
-                           data.threadsArray, data.threadsArray.length);
+                           ["js", "leaf", "stackwalk", "threads"],
+                           data.threadsArray);
 
     Services.profiler.PauseSampling();
   },
 
   /**
    * Assuming the Profiler is running, dumps the Profile from all sampled
    * processes and threads to the disk. The Profiler will be stopped once
    * the profiles have been dumped. This method returns a Promise that
--- a/testing/talos/talos/tests/tart/addon/content/Profiler.js
+++ b/testing/talos/talos/tests/tart/addon/content/Profiler.js
@@ -86,18 +86,18 @@ var Profiler;
     },
     initFromURLQueryParams: function Profiler__initFromURLQueryParams(locationSearch) {
       this.initFromObject(searchToObject(locationSearch));
     },
     beginTest: function Profiler__beginTest(testName) {
       currentTest = testName;
       if (_profiler && enabled) {
         _profiler.StartProfiler(profiler_entries, profiler_interval,
-                                ["js", "leaf", "stackwalk", "threads"], 4,
-                                profiler_threadsArray, profiler_threadsArray.length);
+                                ["js", "leaf", "stackwalk", "threads"],
+                                profiler_threadsArray);
         if (_profiler.PauseSampling) {
           _profiler.PauseSampling();
         }
       }
     },
     finishTest: function Profiler__finishTest() {
       if (_profiler && enabled) {
         _profiler.dumpProfileToFile(profiler_dir + "/" + currentTest + ".profile");
--- a/testing/talos/talos/tests/tresize/addon/content/Profiler.js
+++ b/testing/talos/talos/tests/tresize/addon/content/Profiler.js
@@ -86,18 +86,18 @@ var Profiler;
     },
     initFromURLQueryParams: function Profiler__initFromURLQueryParams(locationSearch) {
       this.initFromObject(searchToObject(locationSearch));
     },
     beginTest: function Profiler__beginTest(testName) {
       currentTest = testName;
       if (_profiler && enabled) {
         _profiler.StartProfiler(profiler_entries, profiler_interval,
-                                ["js", "leaf", "stackwalk", "threads"], 4,
-                                profiler_threadsArray, profiler_threadsArray.length);
+                                ["js", "leaf", "stackwalk", "threads"],
+                                profiler_threadsArray);
         if (_profiler.PauseSampling) {
           _profiler.PauseSampling();
         }
       }
     },
     finishTest: function Profiler__finishTest() {
       if (_profiler && enabled) {
         _profiler.dumpProfileToFile(profiler_dir + "/" + currentTest + ".profile");
--- a/toolkit/components/backgroundhangmonitor/tests/child_cause_hang.js
+++ b/toolkit/components/backgroundhangmonitor/tests/child_cause_hang.js
@@ -6,17 +6,17 @@ const {Services} = ChromeUtils.import("r
 function ensureProfilerInitialized() {
   // Starting and stopping the profiler with the "stackwalk" flag will cause the
   // profiler's stackwalking features to be synchronously initialized. This
   // should prevent us from not initializing BHR quickly enough.
   if (!Services.profiler.CanProfile()) {
     return false;
   }
   let features = ["stackwalk"];
-  Services.profiler.StartProfiler(1000, 10, features, features.length);
+  Services.profiler.StartProfiler(1000, 10, features);
   Services.profiler.StopProfiler();
   return true;
 }
 
 add_task(async function childCauseHang() {
   if (!ensureProfilerInitialized()) {
     return;
   }
--- a/toolkit/components/backgroundhangmonitor/tests/test_BHRObserver.js
+++ b/toolkit/components/backgroundhangmonitor/tests/test_BHRObserver.js
@@ -7,17 +7,17 @@ const { TelemetryUtils } = ChromeUtils.i
 function ensureProfilerInitialized() {
   // Starting and stopping the profiler with the "stackwalk" flag will cause the
   // profiler's stackwalking features to be synchronously initialized. This
   // should prevent us from not initializing BHR quickly enough.
   if (!Services.profiler.CanProfile()) {
     return false;
   }
   let features = ["stackwalk"];
-  Services.profiler.StartProfiler(1000, 10, features, features.length);
+  Services.profiler.StartProfiler(1000, 10, features);
   Services.profiler.StopProfiler();
   return true;
 }
 
 add_task(async function test_BHRObserver() {
   if (!Services.telemetry.canRecordExtended) {
     ok("Hang reporting not enabled.");
     return;
--- a/toolkit/components/extensions/parent/ext-geckoProfiler.js
+++ b/toolkit/components/extensions/parent/ext-geckoProfiler.js
@@ -74,23 +74,21 @@ this.geckoProfiler = class extends Exten
     return {
       geckoProfiler: {
         async start(options) {
           const {bufferSize, windowLength, interval, features, threads} = options;
 
           Services.prefs.setBoolPref(PREF_ASYNC_STACK, false);
           if (threads) {
             Services.profiler.StartProfiler(bufferSize, interval,
-                                            features, features.length,
-                                            threads, threads.length,
+                                            features, threads,
                                             windowLength);
           } else {
             Services.profiler.StartProfiler(bufferSize, interval,
-                                            features, features.length,
-                                            [], 0,
+                                            features, [],
                                             windowLength);
           }
         },
 
         async stop() {
           if (ASYNC_STACKS_ENABLED !== null) {
             Services.prefs.setBoolPref(PREF_ASYNC_STACK, ASYNC_STACKS_ENABLED);
           }
--- a/tools/profiler/gecko/nsIProfiler.idl
+++ b/tools/profiler/gecko/nsIProfiler.idl
@@ -30,21 +30,19 @@ interface nsIProfilerStartParams : nsISu
   [noscript, notxpcom, nostdcall] StringArrayRef getFilters();
 };
 
 [scriptable, builtinclass, uuid(ead3f75c-0e0e-4fbb-901c-1e5392ef5b2a)]
 interface nsIProfiler : nsISupports
 {
   boolean CanProfile();
   void StartProfiler(in uint32_t aEntries, in double aInterval,
-                      [array, size_is(aFeatureCount)] in string aFeatures,
-                      in uint32_t aFeatureCount,
-                      [array, size_is(aFilterCount), optional] in string aFilters,
-                      [optional] in uint32_t aFilterCount,
-                      [optional] in double aDuration);
+                     in Array<AUTF8String> aFeatures,
+                     [optional] in Array<AUTF8String> aFilters,
+                     [optional] in double aDuration);
   void StopProfiler();
   boolean IsPaused();
   void PauseSampling();
   void ResumeSampling();
   void AddMarker(in string aMarker);
   /*
    * Returns the JSON string of the profile. If aSinceTime is passed, only
    * report samples taken at >= aSinceTime.
--- a/tools/profiler/gecko/nsProfiler.cpp
+++ b/tools/profiler/gecko/nsProfiler.cpp
@@ -99,31 +99,54 @@ nsProfiler::Observe(nsISupports* aSubjec
 }
 
 NS_IMETHODIMP
 nsProfiler::CanProfile(bool* aCanProfile) {
   *aCanProfile = !mLockedForPrivateBrowsing;
   return NS_OK;
 }
 
+static nsresult FillVectorFromStringArray(Vector<const char*>& aVector,
+                                          const nsTArray<nsCString>& aArray) {
+  if (NS_WARN_IF(!aVector.reserve(aArray.Length()))) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+  for (auto& entry : aArray) {
+    aVector.infallibleAppend(entry.get());
+  }
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsProfiler::StartProfiler(uint32_t aEntries, double aInterval,
-                          const char** aFeatures, uint32_t aFeatureCount,
-                          const char** aFilters, uint32_t aFilterCount,
+                          const nsTArray<nsCString>& aFeatures,
+                          const nsTArray<nsCString>& aFilters,
                           double aDuration) {
   if (mLockedForPrivateBrowsing) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   ResetGathering();
 
-  uint32_t features = ParseFeaturesFromStringArray(aFeatures, aFeatureCount);
+  Vector<const char*> featureStringVector;
+  nsresult rv = FillVectorFromStringArray(featureStringVector, aFeatures);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  uint32_t features = ParseFeaturesFromStringArray(
+      featureStringVector.begin(), featureStringVector.length());
   Maybe<double> duration = aDuration > 0.0 ? Some(aDuration) : Nothing();
-  profiler_start(aEntries, aInterval, features, aFilters, aFilterCount,
-                 duration);
+
+  Vector<const char*> filterStringVector;
+  rv = FillVectorFromStringArray(filterStringVector, aFilters);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  profiler_start(aEntries, aInterval, features, filterStringVector.begin(),
+                 filterStringVector.length(), duration);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsProfiler::StopProfiler() {
   // If we have a Promise in flight, we should reject it.
   if (mPromiseHolder.isSome()) {
--- a/tools/profiler/tests/browser/head.js
+++ b/tools/profiler/tests/browser/head.js
@@ -9,14 +9,12 @@ function startProfiler() {
     interval: 1, // ms
     features: ["threads"],
     threads: ["GeckoMain"],
   };
   Services.profiler.StartProfiler(
     settings.entries,
     settings.interval,
     settings.features,
-    settings.features.length,
     settings.threads,
-    settings.threads.length,
     settings.duration
   );
 }
--- a/tools/profiler/tests/chrome/profiler_test_utils.js
+++ b/tools/profiler/tests/chrome/profiler_test_utils.js
@@ -3,19 +3,17 @@
 (function() {
 const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 function startProfiler(settings) {
   Services.profiler.StartProfiler(
     settings.entries,
     settings.interval,
     settings.features,
-    settings.features.length,
     settings.threads,
-    settings.threads.length,
     settings.duration
   );
 
   info("Profiler has started");
 }
 
 function getProfile() {
   const profile = Services.profiler.getProfileData();
--- a/tools/profiler/tests/xpcshell/test_asm.js
+++ b/tools/profiler/tests/xpcshell/test_asm.js
@@ -10,17 +10,17 @@ function run_test() {
     // isn't already started.)
     Assert.ok(!Services.profiler.IsActive());
 
     let jsFuns = Cu.getJSTestingFunctions();
     if (!jsFuns.isAsmJSCompilationAvailable())
         return;
 
     const ms = 10;
-    Services.profiler.StartProfiler(10000, ms, ["js"], 1);
+    Services.profiler.StartProfiler(10000, ms, ["js"]);
 
     let stack = null;
     function ffi_function() {
         var delayMS = 5;
         while (1) {
             let then = Date.now();
             do {
               // do nothing
--- a/tools/profiler/tests/xpcshell/test_enterjit_osr.js
+++ b/tools/profiler/tests/xpcshell/test_enterjit_osr.js
@@ -7,17 +7,17 @@ function run_test() {
     }
 
     // This test assumes that it's starting on an empty profiler stack.
     // (Note that the other profiler tests also assume the profiler
     // isn't already started.)
     Assert.ok(!Services.profiler.IsActive());
 
     const ms = 5;
-    Services.profiler.StartProfiler(10000, ms, ["js"], 1);
+    Services.profiler.StartProfiler(10000, ms, ["js"]);
 
     function has_arbitrary_name_in_stack() {
         // A frame for |arbitrary_name| has been pushed.  Do a sequence of
         // increasingly long spins until we get a sample.
         var delayMS = 5;
         while (1) {
             info("loop: ms = " + delayMS);
             const then = Date.now();
--- a/tools/profiler/tests/xpcshell/test_enterjit_osr_disabling.js
+++ b/tools/profiler/tests/xpcshell/test_enterjit_osr_disabling.js
@@ -1,17 +1,17 @@
 function run_test() {
   // Just skip the test if the profiler component isn't present.
   if (!AppConstants.MOZ_GECKO_PROFILER) {
     return;
   }
 
   Assert.ok(!Services.profiler.IsActive());
 
-  Services.profiler.StartProfiler(100, 10, ["js"], 1);
+  Services.profiler.StartProfiler(100, 10, ["js"]);
   // The function is entered with the profiler enabled
   (function() {
     Services.profiler.StopProfiler();
     let n = 10000;
     while (--n); // OSR happens here with the profiler disabled.
     // An assertion will fail when this function returns, if the
     // profiler stack was misbalanced.
   })();
--- a/tools/profiler/tests/xpcshell/test_enterjit_osr_enabling.js
+++ b/tools/profiler/tests/xpcshell/test_enterjit_osr_enabling.js
@@ -2,16 +2,16 @@ function run_test() {
   if (!AppConstants.MOZ_GECKO_PROFILER) {
     return;
   }
 
   Assert.ok(!Services.profiler.IsActive());
 
   // The function is entered with the profiler disabled.
   (function() {
-    Services.profiler.StartProfiler(100, 10, ["js"], 1);
+    Services.profiler.StartProfiler(100, 10, ["js"]);
     let n = 10000;
     while (--n); // OSR happens here with the profiler enabled.
     // An assertion will fail when this function returns, if the
     // profiler stack was misbalanced.
   })();
   Services.profiler.StopProfiler();
 }
--- a/tools/profiler/tests/xpcshell/test_feature_mainthreadio.js
+++ b/tools/profiler/tests/xpcshell/test_feature_mainthreadio.js
@@ -60,18 +60,17 @@ add_task(async () => {
  * Start the profiler and get FileIO payloads.
  * @param {Array} features The list of profiler features
  * @param {string} filename A filename to trigger a write operation
  */
 async function startProfilerAndgetFileIOPayloads(features, filename) {
   const entries = 10000;
   const interval = 10;
   const threads = [];
-  Services.profiler.StartProfiler(entries, interval, features, features.length,
-                                  threads, threads.length);
+  Services.profiler.StartProfiler(entries, interval, features, threads);
 
 
   const file = FileUtils.getFile("TmpD", [filename]);
   if (file.exists()) {
     console.warn(
       "This test is triggering FileIO by writing to a file. However, the test found an " +
       "existing file at the location it was trying to write to. This could happen " +
       "because a previous run of the test failed to clean up after itself. This test " +
--- a/tools/profiler/tests/xpcshell/test_pause.js
+++ b/tools/profiler/tests/xpcshell/test_pause.js
@@ -5,17 +5,17 @@
 function run_test() {
   if (!AppConstants.MOZ_GECKO_PROFILER) {
     return;
   }
 
   Assert.ok(!Services.profiler.IsActive());
   Assert.ok(!Services.profiler.IsPaused());
 
-  Services.profiler.StartProfiler(1000, 10, [], 0);
+  Services.profiler.StartProfiler(1000, 10, []);
 
   Assert.ok(Services.profiler.IsActive());
 
   Services.profiler.PauseSampling();
 
   Assert.ok(Services.profiler.IsPaused());
 
   Services.profiler.ResumeSampling();
--- a/tools/profiler/tests/xpcshell/test_run.js
+++ b/tools/profiler/tests/xpcshell/test_run.js
@@ -4,17 +4,17 @@
 
 function run_test() {
   if (!AppConstants.MOZ_GECKO_PROFILER) {
     return;
   }
 
   Assert.ok(!Services.profiler.IsActive());
 
-  Services.profiler.StartProfiler(1000, 10, [], 0);
+  Services.profiler.StartProfiler(1000, 10, []);
 
   Assert.ok(Services.profiler.IsActive());
 
   do_test_pending();
 
   do_timeout(1000, function wait() {
     // Check text profile format
     var profileStr = Services.profiler.GetProfile();
--- a/tools/profiler/tests/xpcshell/test_start.js
+++ b/tools/profiler/tests/xpcshell/test_start.js
@@ -4,16 +4,16 @@
 
 function run_test() {
   if (!AppConstants.MOZ_GECKO_PROFILER) {
     return;
   }
 
   Assert.ok(!Services.profiler.IsActive());
 
-  Services.profiler.StartProfiler(10, 100, [], 0);
+  Services.profiler.StartProfiler(10, 100, []);
 
   Assert.ok(Services.profiler.IsActive());
 
   Services.profiler.StopProfiler();
 
   Assert.ok(!Services.profiler.IsActive());
 }