1023018 - Make the framerate actor take 'start' and 'end' params for specifying a range when retrieving the refresh driver ticks

Status: RESOLVED FIXED

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

Comment 1

[Victor Porof [:vporof][:vp]]

Attachment: v1

This is needed to synchronize the profiler data time range with the framerate recording range, since any given profile only contains a fixed specified amount of most recent samples (which is pretty large, but enough to overflow after a few mintues), while the framerate actor just keeps on going without scraping off old data.

Comment 2

[Patrick Brosset <:pbro>]

Comment on attachment 8437387 [details] [diff] [review]
v1

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

looking good

::: toolkit/devtools/server/actors/framerate.js
@@ +59,5 @@
>        return [];
>      }
>      this._recording = false;
>  
>      // We don't need to store the ticks array for future use, release it.

nit: this comment line should perhaps be moved just before 'this._ticks=null;'

::: toolkit/devtools/server/tests/mochitest/test_framerate_03.html
@@ +47,5 @@
> +      var front = FramerateFront(client, form);
> +
> +      front.startRecording().then(() => {
> +        window.setTimeout(() => {
> +          front.stopRecording(START_TICK, STOP_TICK).then(rawData => {

Should this test also be checking that using stopRecording() with no params returns the expected default values?
(or perhaps test_framerate_01/02.html already do that)

Comment 3

[Victor Porof [:vporof][:vp]]

(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #2)
> Comment on attachment 8437387 [details] [diff] [review]
> v1
> 
> ::: toolkit/devtools/server/tests/mochitest/test_framerate_03.html
> @@ +47,5 @@
> > +      var front = FramerateFront(client, form);
> > +
> > +      front.startRecording().then(() => {
> > +        window.setTimeout(() => {
> > +          front.stopRecording(START_TICK, STOP_TICK).then(rawData => {
> 
> Should this test also be checking that using stopRecording() with no params
> returns the expected default values?
> (or perhaps test_framerate_01/02.html already do that)

test_framerate_02 does that already.

Comment 4

[Victor Porof [:vporof][:vp]]

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

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7c1667fd1f27