828046 - Save/load profiles to/from disk

Status: RESOLVED FIXED

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

Description

[Anton Kovalyov (:anton)]

It's nice to be able to save your profiles for later. For example, you can profile your app, save the data, optimize the code, then load your data and compare with the new state.

Comment 1

[Anton Kovalyov (:anton)]

Attachment: UI work for Save/Import functionality

Comment 2

[Anton Kovalyov (:anton)]

Attachment: UI work for Save/Import functionality

Comment 3

[Anton Kovalyov (:anton)]

Attachment: UI to save/load profiles

I really tried to use sdk/io/fs module but addon SDK has two blocker bugs in the Buffer implementation (will file a bug for that) so I decided not to wait.

Try: https://tbpl.mozilla.org/?tree=Try&rev=858616fd15b5

Comment 4

[Anton Kovalyov (:anton)]

OK, try is green.

Comment 5

[Rob Campbell [:rc] (:robcee)]

looks good. This is next on my review list.

Comment 6

[Anton Kovalyov (:anton)]

Attachment: UI to save/load profiles

Rebased the patch.

Comment 7

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 775919 [details] [diff] [review]
UI to save/load profiles

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

ok, just a few nits about comments, otherwise it's good.

Looks good and plays nice. I didn't test with importing more than one saved profile though.

::: browser/devtools/profiler/panel.js
@@ +260,5 @@
>  
>      return profile;
>    },
>  
> +  importProfile: function (name, data, opts={}) {

wat? no doc comment??

@@ +480,5 @@
>        panelWin.DebuggerView.Sources.preferredSource = data.uri;
>      }.bind(this));
>    },
>  
> +  openFileDialog: function (opts={}) {

no doc comment?

@@ +503,5 @@
> +
> +    return deferred.promise;
> +  },
> +
> +  saveProfile: function (file, data) {

no doc comment. :(

@@ +516,5 @@
> +    let deferred = promise.defer();
> +    let ch = NetUtil.newChannel(file);
> +    ch.contentType = "application/json";
> +
> +    NetUtil.asyncFetch(ch, (input, status) => {

surprised to see NetUtil here and not OS.File.read(). Not a bad thing though.

::: browser/devtools/profiler/sidebar.js
@@ +36,5 @@
>  }
>  
>  Sidebar.prototype = Heritage.extend(WidgetMethods, {
> +  /**
> +   *

nit: extra empty line.

Comment 8

[Anton Kovalyov (:anton)]

Attachment: Ability to save/load profiles

Fixed nits. Carrying over r+.

Comment 9

[Anton Kovalyov (:anton)]

https://hg.mozilla.org/integration/fx-team/rev/cca017679bfa

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/cca017679bfa