528208 - setInterval clamping seems bogus to me

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

I ran into this while debugging why the testcase in bug 497788 continued to run at about 68-70fps no matter what I did.

That testcase has the following structure:

  var frames;
  var prevFrames = 0;
  var checkpoint = new Date();
  function doSomething() {
    ++frames;
    /* do stuff that takes about 4-5ms */
  }

  function measure() {
    var curTime = new Date();
    log((frames - prevFrames)*1000.0/(curTime - checkpoint));
    checkpoint = curTime;
    prevFrames = frames;
  }

  setInterval(doSomething, 5);
  setInterval(measure, 3000);

Now the way our interval code works is that it ensures that the new firing time for the interval is at least 10ms after the _completion_ of the interval function execution.  So in my case, we were resetting it for typically 14-15ms, which of course gives 67-71fps.

Wouldn't it make more sense to enforce 10ms from the time the interval fired instead?

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, and I can pretty easily write a test for this; I'm just not sure I can  easily write one that won't randomly go random orange on tinderboxen under load....

Comment 3

[Peter Van der Beken [:peterv]]

Comment on attachment 412007 [details] [diff] [review]
Proposed patch

Makes sense to me. Yes, tests would be nice, but it's fairly hard (I gave up in the clamping bug).

Comment 4

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/3dae66874786

I think this is probably too risky for 1.9.2 at this stage... any objections?