1417699 - Stop using a property bag for filter attributes

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement, P2)

Description

[Jeff Muizelaar [:jrmuizel]]

We don't have time for that. (mallocs and hashtables and all that jazz)

https://perfht.ml/2z5hw0e

Comment 1

[Alex Thayer [:alexical] (she/her)]

Markus, are you still looking at this? I might be able to take a look if you're swamped.

Comment 2

[Markus Stange [:mstange]]

I'm not working on this at the moment. I thought I had written down some notes somewhere, but I can't find them unfortunately.

Comment 3

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1417699 - Avoid copies of filter attributes when possible r=mstange

This is a more conservative optimization for bug 1417699. There's no
reason we need to be copying these everywhere, so let's just go ahead
and implement moves.

Comment 4

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1417699 - Replace hash map with tagged union r=mstange

This replaces the hash map of attributes with a tagged union. In
this case, all filter attributes will be stored in line, with the
exception of some more complex attributes which have an internal
nsTArray of floats. This should help avoid all the hashing and
extra heap allocations.

Depends on D4899

Comment 5

[Markus Stange [:mstange]]

Comment on attachment 9006144 [details]
Bug 1417699 - Avoid copies of filter attributes when possible r=mstange

Markus Stange [:mstange] has approved the revision.

Comment 6

[Alex Thayer [:alexical] (she/her)]

Comment on attachment 9006145 [details]
Bug 1417699 - Replace hash map with tagged union r=mstange

(Adding the review flag back to make sure this is on your radar.)

Comment 7

[Markus Stange [:mstange]]

It's on my radar, my review queue is just a bit backed up.

Comment 8

[Markus Stange [:mstange]]

Comment on attachment 9006145 [details]
Bug 1417699 - Replace hash map with tagged union r=mstange

Markus Stange [:mstange] has approved the revision.

Comment 9

[Alex Thayer [:alexical] (she/her)]

Attachment: Bug 1417699 - Remove PrimitiveType enum r=mstange

Ran into a couple of test failures because I was leaving
mAttributes empty for empty things like MergeAttributes, and
only setting mType. Since mType is now redundant though, and
since it's the only use of PrimitiveType, I figured I'd just
remove it entirely.

Depends on D4900

Comment 10

[Markus Stange [:mstange]]

Comment on attachment 9010091 [details]
Bug 1417699 - Remove PrimitiveType enum r=mstange

Markus Stange [:mstange] has approved the revision.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f43cff9efd7
Avoid copies of filter attributes when possible r=mstange
https://hg.mozilla.org/integration/autoland/rev/4f3360c4f104
Replace hash map with tagged union r=mstange
https://hg.mozilla.org/integration/autoland/rev/87694c630c01
Remove PrimitiveType enum r=mstange

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/6f43cff9efd7
https://hg.mozilla.org/mozilla-central/rev/4f3360c4f104
https://hg.mozilla.org/mozilla-central/rev/87694c630c01