1950558 - 3.89 - 2.86% browser_ml_autofill_perf.js AUTOFILL-total-memory-usage / browser_ml_summarizer_perf.js SUM-XENOVA-DISTILBART-CNN-12-6_BIG-peak-memory-usage + 4 more (Linux, Windows) regression on Wed February 19 2025

Status: RESOLVED WORKSFORME

(Core :: Memory Allocator, defect)

Description

[Treeherder Bug Filer]

Perfherder has detected a mozperftest performance regression from push f9e3f8796616bf24fd9d1c7260bc126fe531d0e0. 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)
4%	browser_ml_autofill_perf.js AUTOFILL-total-memory-usage	linux1804-64-shippable		173.71 -> 180.46
3%	browser_ml_autofill_perf.js AUTOFILL-total-memory-usage	windows11-64-2009-hw-ref-shippable		221.08 -> 228.42
3%	browser_ml_autofill_perf.js AUTOFILL-total-memory-usage	windows11-64-24h2-hw-ref-shippable		221.21 -> 228.29
3%	browser_ml_autofill_perf.js AUTOFILL-total-memory-usage	windows11-64-shippable-qr		221.92 -> 229.00
3%	browser_ml_autofill_perf.js AUTOFILL-total-memory-usage	windows11-64-24h2-shippable		221.96 -> 228.88
3%	browser_ml_summarizer_perf.js SUM-XENOVA-DISTILBART-CNN-12-6_BIG-peak-memory-usage	linux1804-64-shippable		1,298.04 -> 1,335.12

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 all of these tests on try with ./mach try perf --alert 44020

The following documentation link provides more information about this command.

For more information on performance sheriffing please see our FAQ.

If you have any questions, please do not hesitate to reach out to [e-mail].

Comment 1

[Jens Stutte [:jstutte]]

I struggle a bit with reading browser_ml_autofill_perf.js, but my assumption would be that the AUTOFILL-total-memory-usage is taken right after loading the model, such that it would be subject of seeing the situation before a lazy purge ever happens.

Could it be that bug 1944040 comment 6 applies also here or is this test already using the very same getTotalMemoryUsage that we tweaked in bug 1944040 ?

IIRC I cannot get a profile for those runs, nonetheless I triggered it FWIW, maybe I misremember.

Comment 2

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

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

Comment 3

[Paul Bone [:pbone]]

It'll be interesting to retest lazy purge on this benchmark once Bug 1928254 and Bug 1947661 land

Comment 4

[Jens Stutte [:jstutte]]

(In reply to Jens Stutte [:jstutte] from comment #1)

IIRC I cannot get a profile for those runs, nonetheless I triggered it FWIW, maybe I misremember.

This time it worked.

In the meantime I noticed that bug 1944790 reverted the change we made in bug 1944040, so I am not surprised to see the same sensitivity of this measure to lazy purge again.

Comment 5

[Jens Stutte [:jstutte]]

(In reply to Paul Bone [:pbone] from comment #3)

It'll be interesting to retest lazy purge on this benchmark once Bug 1928254 and Bug 1947661 land

Yeah, though I think it is quite clear what happened here. So I can see two possible actions for this bug here:

• go back to a way of memory reporting equivalent to what we did in bug 1944040.
• ignore this regression and take the new baseline for good.

In general I think the way ML wants to do memory reporting needs some careful thinking about what those metrics want to actually measure.

Comment 6

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

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

Comment 7

[Jens Stutte [:jstutte]]

IIUC bug 1944790 also introduced some interesting peak-memory-usage test variants. Those did not regress, AFAICT, which confirms that my patch works like expected in that taking memory measurements right after an intense processing ends will most likely see a higher value now for memory.lazypurge.reuse_grace_period ms, at least, than after a purge happened as soon as we went idle. So I'd opt for:

ignore this regression and take the new baseline for good.

with the remaining recommendation to keep in mind that the measured overhead can change lazily based on how long you wait before taking the measurement. The good thing is that the reuse grace period is probably long enough to ensure we do not see too much random noise in those measurements. I'd still recommend for those measures (not for the peak!) to ensure that at least a jemalloc_free_excess_dirty_pages() is called in that process before taking the measurement, but I see no easy way to achieve that from JS, currently.

Marking this WFM as in practice from the ML perspective nothing changed in memory consumption at all.