2043603 - 3.74 - 2.63% stylebench Dynamic media queries/Resizing to 800px - 2/Sync + 3 more (OSX) regression on Tue May 19 2026

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect, P3)

Description

[Treeherder Bug Filer]

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

Please acknowledge, and begin investigating this alert within 3 business days, or the patch(es) may be backed out in accordance with our regression policy.

Regression	Test	Platform	Options	Absolute values [old vs new]	Performance Profiles
4%	stylebench Dynamic media queries/Resizing to 800px - 2/Sync (doc)	macosx1470-64-shippable	fission webrender	2.54 ms -> 2.64 ms	Before/After
4%	stylebench Dynamic media queries/Resizing to 800px - 1/Sync (doc)	macosx1470-64-shippable	fission webrender	2.54 -> 2.63	Before/After
3%	stylebench Dynamic media queries/Resizing to 800px - 2 (doc)	macosx1470-64-shippable	fission webrender	3.54 ms -> 3.64 ms	Before/After
3%	stylebench Dynamic media queries/Resizing to 800px - 1 (doc)	macosx1470-64-shippable	fission webrender	3.54 -> 3.63	Before/After

Support & Actionable Steps

For questions, please contact [e-mail]. You can also find help on Slack (#perf-help) or Matrix (#perftest).

Action guide details:

• See the guide to handling regression bugs for investigation steps.
• Review the alert summary for affected tests, graphs, and comparisons.
• Check the PerfCompare results.
• Tests on Try: Run all tests using ./mach try perf --alert 50483 (See documentation link for details).
• Profiling jobs: Trigger jobs from Treeherder job view or request them from [e-mail].

Comment 1

[BugBot [:suhaib / :marco]]

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

Comment 2

[Sajid Anwar [:sajidanwar]]

Looking at stylebench, I'm not seeing any references to vertical writing modes so it seems unlikely to be the first patch which is (almost) exclusively related to vertical metric synthesis. There is a change in that patch to add a new HarfBuzz font extents function, but I don't think that would have a negative effect on media query resizes here either since font sizes aren't changing in the tests.

So then it must be the inline layout changes in nsLineLayout::VerticalAlignFrames, I think. The old code just assumes that the default baseline (which was the alphabetic baseline in horizontal writing modes) was exactly equivalent to the pfd->mAscent and just used it directly — which doesn't work for fonts with non-default baselines. The new code instead always gets the font-sourced baseline for the frame, which will be correct for all fonts, but which does extra work for the cases where the font baseline is the same as the pfd->mAscent (which is probably nearly all of them).

Jonathan, any chance you have advice about how to deal with this performance regression?

Comment 3

[Jonathan Kew [:jfkthame]]

It's a small thing, but looking at the code in nsLineLayout::VerticalAlignFrames, I notice that the GetFontBaseline helper takes a RefPtr<nsFontMetrics> argument, which I think means there'll be an extra AddRef/Release happening each time it is called. That shouldn't be necessary; you could pass a raw nsFontMetrics*, given that the caller is holding a strong reference anyhow.

Having said that, I doubt this would make a significant difference here.

I don't see any obvious "hotspots" in the After profiles that point to a particular part of the new/changed code as being problematic. It's true that it does a little more work, but it doesn't seem to be contributing significantly to the profile.

Given that the report concerns just one very specific part ("Resizing to 800px") of one subtest of stylebench, on a single platform only, I'm not sure it really represents a meaningful regression. I can see there does seem to be a visible shift on the graph, but it's fairly small compared to the ongoing variability (noise) of the measurement, and doesn't appear on other subtests that would be expected to show similar effects. I wonder if this is just a "random" artifact of code getting moved around a bit, allocations working out differently in relation to memory pages, or something.

Might be worth asking in the Perf channel on matrix to see if someone there could take a look; maybe they'd have further insights.

Comment 4

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:emilio, could you have a look please?

For more information, please visit BugBot documentation.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

I think comment 3 seems worth trying out, but yeah other than that seems like we could take this regression. Sajid want to give comment 3 a try?

Comment 6

[Sajid Anwar [:sajidanwar]]

Thanks Jonathan for taking a look, really appreciate it (and Emilio for pinging, I had lost this in my inbox). Will publish a patch shortly for the pointer change.

Comment 7

[Sajid Anwar [:sajidanwar]]

Attachment: Bug 2043603 - Remove unnecessary RefPtr in font baseline retrieval. r=#layout-reviewers

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/194f92ebae0e
https://hg.mozilla.org/integration/autoland/rev/7ec9fd92b9b6
Remove unnecessary RefPtr in font baseline retrieval. r=layout-reviewers,dholbert

Comment 9

[Sandor Molnar[:smolnar]]

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