1891263 - LastVisualChange unexplainably changed in Yahoo mail (18.59 - 10.5% yahoo-mail LastVisualChange / yahoo-mail LastVisualChange + 3 more (OSX) regression on Thu April 4 2024)

Status: RESOLVED WONTFIX

(Testing :: Performance, defect, P2)

Description

[Andra Esanu (needinfo me)]

Perfherder has detected a browsertime performance regression from push 68a4c335386adefc383ce398e25bc882f3ff4f97. As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
19%	yahoo-mail LastVisualChange	macosx1015-64-shippable-qr	bytecode-cached fission warm webrender	442.95 -> 525.30	Before/After
14%	yahoo-mail LastVisualChange	macosx1015-64-shippable-qr	fission warm webrender	594.47 -> 678.52	Before/After
12%	yahoo-mail LastVisualChange	macosx1015-64-shippable-qr	bytecode-cached cold fission webrender	809.94 -> 910.02	Before/After
12%	yahoo-mail LastVisualChange	macosx1015-64-shippable-qr	fission warm webrender	601.13 -> 672.66	Before/After
11%	yahoo-mail LastVisualChange	macosx1015-64-shippable-qr	cold fission webrender	803.34 -> 887.65	Before/After

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests. Please follow our guide to handling regression bugs and let us know your plans within 3 business days, or the patch(es) may be backed out in accordance with our regression policy.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 42245

For more information on performance sheriffing please see our FAQ.

Comment 1

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

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

Comment 2

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

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

Comment 3

[Emilio Cobos Álvarez (:emilio)]

This was a UI change that adds 1px border to the top. If I look at the side by side comparison, the last visual change in the left seems to be some dates changing in the page.

However, it seems like the text also changes in the old recording, and that change was somehow not detected before my patch.

In any case, this doesn't seem like a real performance regression.

Andrew, Bas mentioned you might be interested in this... If you go at the time the last visual change is recorded in the video, you see some dates in the email list going backwards. That gets recorded as last visual change on the left, but on the right it also happens!

So, this doesn't seem like a real regression in any case...

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: warm-side-by-side.mp4

(For posterity)

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Andrew, fyi, see comment 3

Comment 6

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

This was a UI change that adds 1px border to the top. If I look at the side by side comparison, the last visual change in the left seems to be some dates changing in the page.

@emilio do you have a link to the bug that added the top border? I agree that this is not a performance regression. We should close as wontfix, but we should reference the bug that caused the change.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Bug 1889463. But I think we should at least investigate what's going wrong with the differences there / have a plausible theory that could explain it.

Comment 8

[Donal Meehan [:dmeehan]]

(In reply to Dave Hunt [:davehunt] [he/him] ⌚BST from comment #6)

We should close as wontfix, but we should reference the bug that caused the change.

Should this be closed?

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Donal Meehan [:dmeehan] from comment #8)

(In reply to Dave Hunt [:davehunt] [he/him] ⌚BST from comment #6)

We should close as wontfix, but we should reference the bug that caused the change.

Should this be closed?

As @emilio said in his comment: "we should at least investigate what's going wrong with the differences there / have a plausible theory that could explain it". I understood this to mean keeping the bug open. Thoughts @emilio?

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Yes, I think it's worth digging into that.

Comment 11

[Andrew Creskey [:acreskey]]

Andrew, Bas mentioned you might be interested in this... If you go at the time the last visual change is recorded in the video, you see some dates in the email list going backwards. That gets recorded as last visual change on the left, but on the right it also happens!

Ah, yes, I see it now.
I'm going to guess that the visual difference engine just happened to miss the changes in the right-side video, but not in the left.
The changes are so minor that arguably they shouldn't be considered as part of the "LastVisualChange" metric.

If we tune the visual metric sensitivity, then this would be a good test case to consider.

Comment 12

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

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

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Andrew, is this still something you want to leave open ?

Comment 14

[Andrew Creskey [:acreskey]]

I'm happy with this closed, thank you.