1812649 - Consider to have a cache in SetFontInternal

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, task)

Description

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

https://share.firefox.dev/3jbg8RZ from an "unminified react-stockcharts profile" by jrmuizel
SetFontInternal seems to call into stylo always. Could we try to avoid that by having some kind of cache, perhaps MruCache?

Comment 1

[Denis Palmeiro [:denispal]]

I tried a very naive prototype of this and it does seem to help stockcharts by about 5%, https://treeherder.mozilla.org/perfherder/compare?originalProject=try&newProject=try&newRevision=3606fd02989e42cf750aadbce97ae1d1056a96e6&framework=13&originalRevision=2c3badf1064fb834656d2370006a688721778f48&page=1&showOnlyImportant=1. Although I'm not convinced the assumptions I made for the cache are correct.

Comment 2

[Jonathan Kew [:jfkthame]]

Hah - just saw your comment come in, while I'm also in the middle of experimenting with a patch here! I did a similar thing, but using mfbt/MruCache.h rather than a linear array.... I suspect that may be better if we allow the cache to have rather more entries (MruCache default is 31) rather than just a handful.

I'll post my patch if tryserver seems happy with it.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1812649 - Add a simple MRU cache to SetFontInternal, to reduce stylo usage from canvas2d contexts. r=lsalzman,#gfx-reviewers

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/76e73894c489
Add a simple MRU cache to SetFontInternal, to reduce stylo usage from canvas2d contexts. r=gfx-reviewers,lsalzman

Comment 5

[Noemi Erli[:noemi_erli]]

Backed out changeset 76e73894c489 (Bug 1812649) for causing failures in test_canvas_font_setter.html CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&resultStatus=testfailed%2Cbusted%2Cexception&revision=76e73894c4896acafa53ec46869594e775933c11&searchStr=mochitest&selectedTaskRun=AZ1hirNZQdmvecbjNgATdA.0

Log: https://treeherder.mozilla.org/logviewer?job_id=405652710&repo=autoland&lineNumber=4080

Backout: https://hg.mozilla.org/integration/autoland/rev/32a400972840d8a35550554e4fd8cc861a623528

Comment 6

[Jonathan Kew [:jfkthame]]

(In reply to Noemi Erli[:noemi_erli] from comment #5)

Backed out changeset 76e73894c489 (Bug 1812649) for causing failures in test_canvas_font_setter.html CLOSED TREE

Oh, right - the cache here is a little too aggressive. I think a simple fix will be to include the restyle generation in the cache key, to avoid the risk of using stale entries.

Try run to double-check: https://treeherder.mozilla.org/jobs?repo=try&revision=ff3129316852f1af9c134e6f861cfcf6b758df27

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f32fdf85ea83
Add a simple MRU cache to SetFontInternal, to reduce stylo usage from canvas2d contexts. r=gfx-reviewers,lsalzman

Comment 8

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/f32fdf85ea83