Closed Bug 873425 Opened 11 years ago Closed 7 years ago

Slow D3.js example due to invalidation when textContext is set to the current value

Categories

(Core :: SVG, defect)

Product:
Core
Component:
SVG
Version:
20 Branch
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED INCOMPLETE

People

(Reporter: emepyc, Unassigned)

References

(Depends on 1 open bug,
URL
)

Details

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

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.63 Safari/537.31

Steps to reproduce:

Go to
http://www.ebi.ac.uk/~mp/ePeek/clients/minimal2.html
Drag horizontally the svg
I am running Mozilla v20 on Linux kernel 3.5.0-17



Actual results:

The performance of the movement is slow and clunky compared to Chrome, Opera & Safari.


Expected results:

It should be much smoother.
The same happens with many other svgs like:
http://jsfiddle.net/emepyc/C6bsM/7/
http://www.brolinembedded.se/misc/erlandsson.svg
http://bl.ocks.org/mbostock/2206340
http://bost.ocks.org/mike/fisheye/
etc...

Drag & drop results in clunky movements.
Component: Untriaged → SVG
Product: Firefox → Core
Can you try the latest Nightly build, please?

https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central/

There have been a lot of major performance improvements since v20, particularly some in the last few weeks that could help you.
Keywords: perf
Tried on Nightly (24.0a1 (2013-05-17)) with similar results. I can't tell for sure if I see any improvement or not in the examples I have posted (I would say no).
If you take the http://www.brolinembedded.se/misc/erlandsson.svg example save it locally and change one of the javascript functions to this...

function Move(evt)
{
  if( ! down )
    return

  evt.preventDefault();

  var x = evt.clientX
  var y = evt.clientY
  var dx = x - lastX
  var dy = y - lastY
  document.rootElement.currentTranslate.x = origX + dx
  document.rootElement.currentTranslate.y = origY + dy
}

Does that help/fix it for you for that example?
Flags: needinfo?(emepyc)
Looking at the original link, I'm seeing a ton of over-invalidation. I've so far identified two sources:

 1) D3.js seems to set the .textContent of all the <text> elements to the
    same value they currently have every time the visualization is panned,
    and we invalidate and repaint them each time. That's especially
    annoying given that much of the text is moving them using a transform
    which would otherwise give us excellent performance. This is bug
    725221, which seems to have stalled due to DOM spec complexities.

 2) We are hitting bug 867596 when svg.text.css-frames.enabled is on
    (it's off by default). I'll be fixing that soon.
Depends on: 725221, 867596
(In reply to Robert Longson from comment #3)
> If you take the http://www.brolinembedded.se/misc/erlandsson.svg example
> save it locally and change one of the javascript functions to this...
> 
> function Move(evt)
> {
>   if( ! down )
>     return
> 
>   evt.preventDefault();
> 
>   var x = evt.clientX
>   var y = evt.clientY
>   var dx = x - lastX
>   var dy = y - lastY
>   document.rootElement.currentTranslate.x = origX + dx
>   document.rootElement.currentTranslate.y = origY + dy
> }
> 
> Does that help/fix it for you for that example?

No, I can't see any improvement in performance.
Flags: needinfo?(emepyc)
(In reply to Jonathan Watt [:jwatt] from comment #4)
> Looking at the original link, I'm seeing a ton of over-invalidation. I've so
> far identified two sources:
> 
>  1) D3.js seems to set the .textContent of all the <text> elements to the
>     same value they currently have every time the visualization is panned,
>     and we invalidate and repaint them each time. That's especially
>     annoying given that much of the text is moving them using a transform
>     which would otherwise give us excellent performance. This is bug
>     725221, which seems to have stalled due to DOM spec complexities.
> 

Do you mean?
https://github.com/mbostock/d3/blob/master/d3.js#L7407
I have commented that specific line and the results are roughly the same. (But I really don't know what I am doing here).

I have also tried removing all panned text:
http://www.ebi.ac.uk/~mp/ePeek/clients/minimal2.html

The performance doesn't really improve that much.

Are these tests useful?
If not, please let me know if I can do something else from my part.

>  2) We are hitting bug 867596 when svg.text.css-frames.enabled is on
>     (it's off by default). I'll be fixing that soon.

Is there a test I can do to test this? AFAIK, I haven't changed the default option, how can I be sure that I am running with svg.text.css-frames.disabled to compare performance?
I've reached out to the creator of D3 to see whether issue 1 is something that could be fixed within that toolkit.
Depends on: 875175
(In reply to emepyc from comment #6)
> Do you mean?
> https://github.com/mbostock/d3/blob/master/d3.js#L7407
> I have commented that specific line and the results are roughly the same.
> (But I really don't know what I am doing here).

I don't know for sure. I just know that I'm seeing a ton of set-textContent-to-its-current-value calls internally, and if I made that just do nothing it stopped a lot of the invalidation and repainting work that was going on.

> I have also tried removing all panned text:
> http://www.ebi.ac.uk/~mp/ePeek/clients/minimal2.html

There seems to be just as much panned text in there as before.

> The performance doesn't really improve that much.

Can you retest with the very latest Nightly build? Now that bug 875175 is fixed we should be doing less expensive work.

> Are these tests useful?
> If not, please let me know if I can do something else from my part.

Nothing else at the moment other than retesting.

> >  2) We are hitting bug 867596 when svg.text.css-frames.enabled is on
> >     (it's off by default). I'll be fixing that soon.
> 
> Is there a test I can do to test this? AFAIK, I haven't changed the default
> option, how can I be sure that I am running with
> svg.text.css-frames.disabled to compare performance?

Don't worry about that comment. It was just for completeness for myself and other implementers. Also note that there is no svg.text.css-frames.disabled, only svg.text.css-frames.enabled, and that currently defaults to false if you haven't changed it.
(In reply to emepyc from comment #6)
> Are these tests useful?
> If not, please let me know if I can do something else from my part.

I believe that the only outstanding issue that is making this slow is bug 725221. You could help prove/disprove that that is the case for you by creating a test where you replace all the logic that looks like this:

  foo.textContent = bar;

with:

  if (foo.textContent != bar) {
    foo.textContent = bar;
  }

I guess that just means searching through the D3 source for "textContent".

If you can make that test available online I can confirm that it is stopping the set-textContent-to-its-current-value calls internally, and if you are still seeing slowness with that testcase then we need to investigate further.
This is weird. I don't see any performance improvement in latest nightly (24.0a1/2013-06-11)

Here is the test again:
http://www.ebi.ac.uk/~mp/ePeek/clients/minimal2.html

I have replaced all the "foo.textContent = bar" in the D3 code as suggested.
Any idea?
emepyc, we're looking into this again, but the demo seems to be quite broken now. Any chance you could restore it to its original state at the time that you filed this bug?
Done,
http://www.ebi.ac.uk/~mp/ePeek/clients/minimal2.html

Please, let me know if I can help in any way to solve this.

M;
URL: http://www.ebi.ac.uk/~mp/ePeek/client...
Whiteboard: [jwatt:invalidation] [in-the-wild] [external-report]
emepyc: darn, it seems the copy of minimal2.html that I made locally doesn't work, and the live examples are gone. Any chance you still have a working example of the minimal testcase you referenced in comment 12?
Summary: Slow SVG performance → Slow D3.js example due to invalidation when textContext is set to the current value
That example is gone. But I have similar (less minimal) examples that were performing poor in Firefox and at some point (I would say around 2 years ago?) started to perform much better.

Here is a similar example of the minimal testcase I originally shared:

http://bl.ocks.org/emepyc/2a3dd22933aa8cf4055f

As I said, this is performing similar to Chrome for the last couple of years.
The other examples I shared seem to perform much better as well:

http://jsfiddle.net/emepyc/C6bsM/7/
http://www.brolinembedded.se/misc/erlandsson.svg
http://bl.ocks.org/mbostock/2206340
http://bost.ocks.org/mike/fisheye/
A shame it's gone, but I think we extracted the pain points from it will it was around, and of those only bug 725221 seemed to be outstanding.

Thanks for the other examples. As you say, most of them seem to perform fine now, but I did see some slowness on http://bl.ocks.org/mbostock/2206340 . I filed bug 1349159 for that.

If you come across any more performance issues we'd love to have bugs filed on them. Feel free to CC me. For now though I think we can close this one.
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.