Closed Bug 1417699 Opened 3 years ago Closed 2 years ago

Stop using a property bag for filter attributes

Categories

(Core :: Web Painting, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: jrmuizel, Assigned: dthayer)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [qf:p1:f64])

Attachments

(3 files)

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

https://perfht.ml/2z5hw0e
Priority: -- → P2
Whiteboard: [qf:p1][qf:f64]
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Whiteboard: [qf:p1][qf:f64] → [qf:p1:f64]
Markus, are you still looking at this? I might be able to take a look if you're swamped.
Flags: needinfo?(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.
Assignee: mstange → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(mstange)
Assignee: nobody → dothayer
Status: NEW → ASSIGNED
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.
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 on attachment 9006144 [details]
Bug 1417699 - Avoid copies of filter attributes when possible r=mstange

Markus Stange [:mstange] has approved the revision.
Attachment #9006144 - Flags: review+
OS: Unspecified → All
Hardware: Unspecified → All
Version: unspecified → Trunk
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.)
Attachment #9006145 - Flags: review?(mstange)
It's on my radar, my review queue is just a bit backed up.
Comment on attachment 9006145 [details]
Bug 1417699 - Replace hash map with tagged union r=mstange

Markus Stange [:mstange] has approved the revision.
Attachment #9006145 - Flags: review+
Attachment #9006145 - Flags: review?(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 on attachment 9010091 [details]
Bug 1417699 - Remove PrimitiveType enum r=mstange

Markus Stange [:mstange] has approved the revision.
Attachment #9010091 - Flags: review+
Pushed by dothayer@mozilla.com:
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
Depends on: 1635658
You need to log in before you can comment on or make changes to this bug.