Cache for box shadows

[ZilverBlade]

PR to solve issue #799

[mikke89]

Hey, I've finally gotten around to go through this PR now, and some testing with it.

First of all, thanks a lot for making the pull request. It's nice to see a focus on performance improvements too. I've made some comments throughout the PR, so please take a look at them.

I didn't measure it yet, but I am somewhat worried that all the extra data copies, hashing, and comparisons have some measurable performance impact. I have some suggestions in the comments that could alleviate some of that, but not all. Could you try to make some measurements, comparing it to master? Ideally, testing both cases: many unique box shadows, and another one for equivalent box shadows. An addition to the "Benchmarks" project would be very valuable here.

It would also be interesting to do the same for the geometry of all backgrounds/borders, as I suspect it is just as common to share styles in these cases. But that might as well be something to follow up with later.

[ZilverBlade]

Okay i tried all of the issues you mentioned, i'm still working on the cache part. However, I'm still trying to figure out how to deal with the shadow Geometry handle. To avoid recomputing it, it should probably be cached or passed by reference, but the GetBackground() in ElementBackgroundBorder makes it a bit hard to deal with that as it's just stored as a value. The texture cache though i believe i have it figured out.

[mikke89]

Very nice progress.

I'm still trying to figure out how to deal with the shadow Geometry handle.

Yeah, I think it must be stored in the BoxShadowData, and then it needs to be retrieved from that data somehow. I understand it's a bit tricky with the other Geometry handles, I'll leave you to it :)

[ZilverBlade]

Finally got back to this, I think i implemented it correctly, I do need to fix the build issues, and merge the master branch back into this.

[mikke89]

Yeah nice, it's getting closer now, keep it up, and let me know when it's ready to take a new look at.

[ZilverBlade]

I finally got around to trying this, I installed vckpg so i could finally try out the visual tests, and turns out I think i did some copy paste error somewhere beacuse the box shadows are incorrectly cached

[ZilverBlade]

I got it to work, i compared it to the visual tests and they are identical now, the tough part is profiling the performance difference now, I wasn't able to get the tracy profiler to work. Could you aid here?

[mikke89]

Hey, thanks a lot! I made some changes, and added some benchmarks.

Measurements look really nice. I have attached some measurements at the end here. Huge improvements to repeated box shadows in particular. Unique box shadows are about the same. We didn't expect any improvements for the latter, and it's not slower, so that is great success in my view. I changed the benchmarks a bit after these measurements, but they should be representative.

Please feel free to look over my changes. If you're happy with them then I am ready to merge this, but let me know if you have some comments.

---

master w/GL3 backend

relative	ns/op	op/s	err%	total	Box-shadow (repeated)
100.0%	1,055,511.00	947.41	3.4%	0.01	Reference (update + render)
4.8%	22,044,981.00	45.36	3.9%	0.24	Box-shadow boxshadow_blur
10.7%	9,866,896.00	101.35	1.4%	0.11	Box-shadow boxshadow_trail
16.6%	6,343,124.00	157.65	1.6%	0.07	Box-shadow boxshadow_inset

relative	ns/op	op/s	err%	total	Box-shadow (unique)
100.0%	320,129.00	3,123.74	10.7%	0.01	〰️ Reference (update + render) (Unstable with ~2.8 iters. Increase minEpochIterations to e.g. 28)
3.7%	8,563,449.00	116.78	2.0%	0.09	Box-shadow (unique)

PR w/GL3 backend

relative	ns/op	op/s	err%	total	Box-shadow (repeated)
100.0%	755,962.00	1,322.82	4.9%	0.01	Reference (update + render)
112.2%	673,759.00	1,484.21	4.6%	0.01	Box-shadow boxshadow_blur
104.3%	724,603.50	1,380.07	4.7%	0.01	Box-shadow boxshadow_trail
105.0%	719,994.00	1,388.90	4.4%	0.01	Box-shadow boxshadow_inset

relative	ns/op	op/s	err%	total	Box-shadow (unique)
100.0%	387,968.50	2,577.53	7.1%	0.01	〰️ Reference (update + render) (Unstable with ~1.8 iters. Increase minEpochIterations to e.g. 18)
4.6%	8,391,220.00	119.17	1.5%	0.09	Box-shadow (unique)

---

master w/no backend

relative	ns/op	op/s	err%	total	Box-shadow (repeated)
100.0%	220,471.50	4,535.73	2.3%	0.01	Reference (update + render)
182.6%	120,764.25	8,280.60	0.4%	0.01	Box-shadow boxshadow_blur
190.3%	115,866.62	8,630.61	1.3%	0.01	Box-shadow boxshadow_trail
368.0%	59,910.75	16,691.50	1.4%	0.01	Box-shadow boxshadow_inset

relative	ns/op	op/s	err%	total	Box-shadow (unique)
100.0%	3,590.59	278,506.11	2.0%	0.01	Reference (update + render)
2.0%	183,089.00	5,461.82	0.8%	0.01	Box-shadow (unique)

PR w/no backend

relative	ns/op	op/s	err%	total	Box-shadow (repeated)
100.0%	211,200.00	4,734.85	0.9%	0.01	Reference (update + render)
172.8%	122,196.00	8,183.57	1.1%	0.01	Box-shadow boxshadow_blur
183.7%	114,976.12	8,697.46	0.1%	0.01	Box-shadow boxshadow_trail
347.2%	60,835.88	16,437.67	0.1%	0.01	Box-shadow boxshadow_inset

relative	ns/op	op/s	err%	total	Box-shadow (unique)
100.0%	3,743.18	267,152.26	0.3%	0.01	Reference (update + render)
1.7%	214,936.50	4,652.54	0.2%	0.01	Box-shadow (unique)

[ZilverBlade]

Cool, I didn't expect that high of a performance liftup, so that's great to see, I like the changes you fixed, I had forgotten to change some of the documentation.

[mikke89]

Great work! Thanks a lot, very happy to merge this one.

Yeah, the exact performance numbers depend entirely on exactly how they are measured. But the main takeaway is that the box shadows are no longer regenerated when they can be reused. Which helps massively in some situations!

Cheers!