Closed Bug 1131397 Opened 5 years ago Closed 5 years ago

Re-add profiler actor APIs that the Gecko Profiler addon needs

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect)

defect
Not set

Tracking

(firefox38 fixed)

RESOLVED FIXED
Firefox 38
Tracking Status
firefox38 --- fixed

People

(Reporter: mstange, Assigned: mstange)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
The Gecko Profiler addon uses the debugger protocol + the profiler actor when profiling Fennec. This has been broken for a long time now and nobody noticed until a few weeks ago.

The APIs we need are: getSharedLibraries, getFeatures, and the ability to specify thread filters in startProfiler.

This patch adds them. Do I need to add tests? If so, where?
Attachment #8561788 - Flags: review?(vporof)
Comment on attachment 8561788 [details] [diff] [review]
patch

Review of attachment 8561788 [details] [diff] [review]:
-----------------------------------------------------------------

Tests are in toolkit/devtools/server/tests/unit, named test_profiler_*.js. Clone one of them, or augment an existing one, doesn't matter too much.

r+ with test.

::: toolkit/devtools/server/actors/profiler.js
@@ +9,5 @@
>  
>  let DEFAULT_PROFILER_ENTRIES = 1000000;
>  let DEFAULT_PROFILER_INTERVAL = 1;
>  let DEFAULT_PROFILER_FEATURES = ["js"];
> +let DEFAULT_PROFILER_THREADFILTERS = ["GeckoMain"];

Add this into browser/devtools/performance/modules/front.js and browser/devtools/profiler/utils/shared.js as well (PerformanceFront.prototype._customProfilerOptions). This code duplication is unfortunate, but will be fixed with bug 1075567.

@@ +104,5 @@
> +   * Returns a stringified JSON object that describes the shared libraries
> +   * which are currently loaded into our process.
> +   */
> +  onGetSharedLibraryInformation: function() {
> +    return { sharedLibraryInformation: nsIProfilerModule.getSharedLibraryInformation() };

Do these new methods work while the profiler is not active? Do we care if they don't?
Attachment #8561788 - Flags: review?(vporof) → review+
Attached patch v2Splinter Review
I made the changes you requested and added test_profiler_getfeatures.js and test_profiler_getsharedlibraryinformation.js.

The new methods can be called while the profiler is stopped. I added comments that say so.
Attachment #8561788 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/535453825f6d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.