Closed
Bug 528208
Opened 15 years ago
Closed 15 years ago
setInterval clamping seems bogus to me
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(1 file)
2.74 KB,
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
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?
Assignee | ||
Updated•15 years ago
|
blocking2.0: --- → ?
Assignee | ||
Comment 1•15 years ago
|
||
Assignee | ||
Comment 2•15 years ago
|
||
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•15 years ago
|
||
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).
Attachment #412007 -
Flags: review?(peterv) → review+
Assignee | ||
Comment 4•15 years ago
|
||
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?
Status: ASSIGNED → RESOLVED
blocking2.0: ? → ---
Closed: 15 years ago
Flags: in-testsuite?
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•