Closed Bug 1482289 Opened 2 years ago Closed 2 years ago

Enlarge index buffer validation cache max size to handle CAD workloads

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- fixed
firefox61 --- wontfix
firefox62 --- fixed
firefox63 --- unaffected

People

(Reporter: jgilbert, Assigned: jgilbert)

References

Details

Attachments

(1 file)

CAD workloads blow out our validation cache. While bug 1476327 fixes this in general, it's a pretty invasive change to uplift, and while bug 1477817 detects the Windows-omnipresent RBAB extension which disables index validation checks, MacOS is left in the cold for ESR60 and Beta62.

A quick run of the testcase from 1476327 shows that increasing the index buffer validation cache size to 1M instead of 256 is enough to stop the testcase from blowing out the cache, and performance seems good.
It's not ideal, but this is simpler and easier to uplift.
Attachment #8999051 - Flags: review?(kvark) → review+
See Also: → 1476327
Depends on: 1339256
Comment on attachment 8999051 [details] [diff] [review]
0001-Bug-1482289-Enlarge-index-buffer-validation-cache-si.patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Easy fix for perf issue on ESR60 on MacOS.
User impact if declined: Reduced performance on CAD WebGL workloads on MacOS.
Fix Landed on Version: 63
Risk to taking this patch (and alternatives if risky): Low risk, but we could just let it ride. It's a double-digit perf hit on these things, but it might be tolerable.
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1339256
[User impact if declined]: Reduced performance on CAD WebGL workloads on MacOS.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: Nightly has a different fix. This bug is aiming to address the issue more simply, but not as thoroughly.
[Needs manual test from QE? If yes, steps to reproduce]: Not needed, but bug 1476327 has an STR testcase.
[List of other uplifts needed for the feature/fix]: esr60, beta62
[Is the change risky?]: Low risk.
[Why is the change risky/not risky?]: We're just increasing the size of a cache, so memory usage for some webgl workloads might increase, but only by a couple MB, and most webgl workloads will remain unaffected.
[String changes made/needed]:
Attachment #8999051 - Flags: approval-mozilla-esr60?
Attachment #8999051 - Flags: approval-mozilla-beta?
Comment on attachment 8999051 [details] [diff] [review]
0001-Bug-1482289-Enlarge-index-buffer-validation-cache-si.patch

Perf fix for MacOS/WebGL, let's take this for beta 17. 
Seems reasonable for ESR but I'll leave that up to Ryan.
Attachment #8999051 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8999051 [details] [diff] [review]
0001-Bug-1482289-Enlarge-index-buffer-validation-cache-si.patch

Agreed that this looks reasonably safe to take. Approved for ESR 60.2.
Attachment #8999051 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
See Also: → 1477817
You need to log in before you can comment on or make changes to this bug.