1878884 - [GPU-canvas] 32x regression on a canvas demo (https://www.fxhash.xyz/generative/23090)

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Mayank Bansal]

1.Set the following preferences:
gfx.direct2d.disabled = true
gfx.canvas.accelerated = true
gfx.canvas.remote.worker-threads = 0

2. Go to https://www.fxhash.xyz/generative/23090
3. Click on Run

Jan2023: https://share.firefox.dev/42s3ksE (1.2s)
6Feb2024: https://share.firefox.dev/4882y5k (32s)

Regression:
Bug 1829026 - Handle KERN_ABORTED from semaphore_wait. r=aosmond

semaphore_wait and semaphore_timedwait can return KERN_ABORTED, much
like POSIX semaphores can return EINTR, and need to restart their
waits for correct behavior.

Differential Revision: https://phabricator.services.mozilla.com/D195952

Comment 1

[Mayank Bansal]

Attachment: about:support

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1829026

:lsalzman, since you are the author of the regressor, bug 1829026, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[Lee Salzman [:lsalzman]]

Attachment: Bug 1878884 - Keep RecordedTextureData snapshot around until it actually changes. r?aosmond

Every BorrowSnapshot use would create and destroy a SourceSurfaceCanvasRecording wrapper
that would go away because it was held by a ThreadSafeWeakPtr, causing us to do a readback
for every GetImageData call, especially when there are multiple such GetImageData calls
in a row.

This replaces the ThreadSafeWeakPtr with a RefPtr, and then adds some MarkChanged tracking
so that we can orphan the snapshot when the underlying DrawTargetRecording changes, but
otherwise keep around the snapshot for successive BorrowSnapshot calls.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ae707f4ae6d
Keep RecordedTextureData snapshot around until it actually changes. r=aosmond

Comment 5

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/7ae707f4ae6d

Comment 6

[Mayank Bansal]

Profile from a build containng this fix : https://share.firefox.dev/3usKaWU (861ms)
So we are faster than ever. Thanks!

Comment 7

[Lee Salzman [:lsalzman]]

Attachment: Bug 1878884 - Allow snaphots to reattach to a DT for shmem readbacks if unmodified. r?aosmond

For certain use-cases that borrow a snapshot, but don't immediately read it, if we then hand
out the same snapshot again on a subsequent call to BorrowSnapshot, the second use will be
penalized by the first use, as the snapshot will be detached for surviving past ReturnSnapshot,
thus disabling shmem-accelerated readbacks.

Now that we can track modifications to DrawTargetRecording, we can make the optimization that
if between the first and second BorrowSnapshot, the DT was unmodified, that we can temporarily
reattach the snapshot to the DT for shmem-accelerated readback, as no conflicting drawing
events have been sent over IPDL to the backend DrawTargetWebgl.

This allows us to reuse the original snapshot until it actually gets modified by something,
and so long as its unmodified, and all readbacks happen between BorrowSnapshot and ReturnSnapshot,
they can then be properly accelerated.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b09350ff8440
Allow snaphots to reattach to a DT for shmem readbacks if unmodified. r=aosmond

Comment 9

[Sandor Molnar[:smolnar]]

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

Comment 10

[Mayank Bansal]

Profile with latest Nightly: https://share.firefox.dev/49sXklJ

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1829026