1958651 - addPath with a DOMMatrix is slow

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Calixte Denizet (:calixte)]

Attachment: plop.html

With the attached test case, 90% of the time is spent in just initializing the matrix:
https://share.firefox.dev/44cvmLp

Maybe it could be interesting to allow a Float(32|64)Array for the second parameter or six numbers (i.e. addPath(path, a, b, c , d, e, f)).

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Oh, the testcase passes DOMMatrix as input, function is defined to take DOMMatrix2DInit as input. And getting the data out of native implementation is slower than it would be to get it from normal dictionary.
If we could detect earlier that it is a real DOMMatrix, then this should be faster.
Though, on JS side one could override the normal getters, I think, and those should get called.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Calixte, in your code, would it be possible to use a simple dictionary and does that improve performance?
addPath(path, { a: 123, b: 345, .... })

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Lots of time is spent also in creating (temporary?) DOMMatrix internally
https://searchfox.org/mozilla-central/rev/a0c761cb7958c0c9a3e7e34d944f12b56e0737fa/dom/canvas/CanvasRenderingContext2D.cpp#7088-7089

Kagami, do you recall why it was implemented that way? Does something require us creating that seemingly temporary object?
This is a hot code path, so any extra allocations should be avoided if possible

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

It's the spec: https://html.spec.whatwg.org/multipage/canvas.html#dom-path2d-addpath

Let matrix be the result of creating a DOMMatrix from the 2D dictionary transform.

Might be nice to have a struct than a full blown DOMMatrix though.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Well, specs algorithms do all sorts of extra things just to make it easier to define things on that side. Implementation need to optimize.
But ok, sounds like we could just have a stack allocated struct.

Comment 6

[Calixte Denizet (:calixte)]

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #2)

Calixte, in your code, would it be possible to use a simple dictionary and does that improve performance?
addPath(path, { a: 123, b: 345, .... })

Interesting, yes it's a way faster, either in pdf.js or with the test case above.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1958651, don't create temporary DOMMatrixReadonly when calling addPath, r=saschanaz

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4184609c3365
don't create temporary DOMMatrixReadonly when calling addPath,  r=saschanaz

Comment 9

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/4184609c3365