Closed Bug 608495 Opened 9 years ago Closed 4 years ago

The IE test drive helicopter demo is glacially slow

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: bzbarsky, Unassigned)

References

(Blocks 1 open bug, )

Details

(Keywords: perf, Whiteboard: [in-the-wild] [external-report] [ietestdrive])

See bug 608072 comment 4.  There's some attr parsing there, and more stuff happenig in frame land.
These two lines in Demo.js

270 this.dx += this.width/this.numClouds;
271 this.smokeGroup.setAttributeNS(null,"transform",this.dx);

are a bug in the program that cause us to create a proxy. I'm guessing that line 271 needs to be something like this...

271 this.smokeGroup.setAttributeNS(null,"transform","translate(" + this.dx + ",0)"); 

I wonder how much fixing that affects performance? If it is significant then we can look to remove the proxy when it is no longer required. Could also see whether Microsoft will fix this bug in their program.
I'm happy to test a fixed version....
bug 690486 fixes this. It's just landed on inbound.
Depends on: 690486
Or more accurately bug 690486 fixes the comment 1 issue. The attr parsing should be fixed by bug 629200.
Depends on: 629200
Bug 629200 doesn't apply here. For the helicopter demo the time spent inside nsGenericElement::SetAttrAndNotify is almost entirely in nsNodeUtils::AttributeChanged (and subsequently PresShell::AttributeChanged). i.e. there's no serialisation going on which is what bug 629200 fixes.

There is some time spent in nsGenericElement::SetAttr but it's minor (<2%) and it's mostly parsing / triggering reflow and so unrelated to bug 629200.
No longer depends on: 629200
Brian, where does AttributeChanged spend its time in this case?
(In reply to Boris Zbarsky (:bz) from comment #6)
> Brian, where does AttributeChanged spend its time in this case?

With an Win 7 build I did a few days ago I was seeing the following break-down in xperf:

nsGenericElement::SetAttrAndNotify                      14%
  nsNodeUtils::AttributeChanged                         14%
    PresShell::AttributeChanged                         13%
      nsSVGUtils::UpdateGraphic                         11%
        nsIFrame::InvalidateWithFlags                   7%
          nsIFrame::InvalidateInternal                  7%
          ...
        nsSVGOuterSVGFrame::UpdateAndInvalidateCoveredRegion  4%
          nsSVGPathGeometryFrame::UpdateCoveredRegion         3%
          nsSVGUtils::FindFilterInvalidation                  1%
          ...
        ...
      nsCSSFrameConstructor::AttributeChanged            1%
      ...

All percentages are absolute percentages of samples inside firefox.exe.

Re-doing the test with the most recent nightly the breakdown is slightly different, but still it's mostly invalidation.

There's a little bit of time spent in parsing transform lists (about 1.6%) but I'm not seeing any time spent in serialisation so bug 629200 isn't going to help here.
Ah, yes.  So should this depend on display-list-based invalidation?
With Boris' help I've looked into this a bit more. I added some logging to nsSVGPathGeometryFrame to see if we ever call nsSVGUtils::UpdateGraphic more often than we get calls to nsSVGPathGeometryFrame::PaintSVG.

It seems like we sometimes make 2 calls to nsSVGUtils::UpdateGraphic between subsequent calls to PaintSVG. Once as a result of a call to nsSVGPathGeometryFrame::AttributeChanged and once as a result of a call to nsSVGPathGeometryFrame::NotifySVGChanged (called by nsSVGGFrame::AttributeChanged iterating over its children).

Occasionally we make up to 33 calls to UpdateGraphic per paint but generally we average about 1.25. One idea Boris suggested is to lazily update the graphic using a hashtable and thereby coalesce those calls.

Currently we're getting about 20fps.

The test calculates a "speed" using a formula something like:

 speed = fps * 5 / (maxdt/16)

Where maxdt is the maximum time between samples for the current second (calculated as new Date() - lastTime). This is to punish uneven framerates. It is reset every second.

The 16 is because the animation is updated using setInterval with an interval of 16ms.

I'm currently seeing a speed of about 30mph where maxdt is generally in the range of 50 or 60 but sometimes up to 400 or so. IE9 gives me ~1000mph.

(Interestingly there's a bug in setting the transform attribute for the smoke that uses the wrong syntax causing us to produce a console warning which has a very minor impact on our performance. It's not significant at this stage.)
> IE9 gives me ~1000mph.

Given the formula above, that involves IE having an fps of about 200 or a maxdt on the order of 5ms or some combination thereof, right?  I mean... if the benchmark is using a 16ms timeout then maxdt ought to be at least 16 and fps ought to be about 60, for a speed of 300....
(In reply to Boris Zbarsky (:bz) from comment #10)
> Given the formula above, that involves IE having an fps of about 200 or a
> maxdt on the order of 5ms or some combination thereof, right?  I mean... if
> the benchmark is using a 16ms timeout then maxdt ought to be at least 16 and
> fps ought to be about 60, for a speed of 300....

Actually, the maxdt has a minimum value of 16. Anything less is scaled up to 16. So they have to be getting 200fps.

I double-checked. In "auto-play mode", which is what I was using, they use setInterval(autoPlay_drawFrame, 0).
In normal playing mode (where it is setInterval with 16ms) I get about 250mph.
Keywords: perf
The fix for bug 766429 has made a _huge_ difference here. For me the autoplay mode has gone from 2 mph to ~300 mph, so something like a 150x speedup.
Depends on: 766429
OS: Mac OS X → All
Hardware: x86 → All
Summary: Lots of time spent in path attribute change handling on IE test drive helicopter demo → The IE test drive helicopter demo is glacially slow
FWIW, on the latest nightly, i get ~ 34FPS with HWA on.
On the same hardware, on IE10 , i get 800-900 FPS 

Profile : http://people.mozilla.com/~bgirard/cleopatra/#report=614e613ecbd60eb2bf4afe5c21e406f5bf8d3bef
Depends on: 869611
For me, bug 869611 sped up the IE Test Drive Helicopter demo from 15-20 "fps" to 30-35 "fps".
Whiteboard: ietestdrive → [in-the-wild] [external-report] [ietestdrive]
Massively regressed between Release 32 and current Nightly. Now unplayable. The only way to get FPS number is to click the "autoplay" link at the bottom of the page and wait about 30 seconds before it decides that the FPS is zero.
Seems OK on windows.
I filed bug 1078109 on comment 16, since we'll want to remeasure once that's fixed.
Depends on: 1078109
The demo was really fast last time I tried it but it's gone now.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.