Closed Bug 1603678 Opened 5 years ago Closed 3 months ago

2.29 - 3.18% Explicit Memory (windows7-32, windows7-32-shippable) regression on push 3a083701018bf872acfc5e391312042d8d246aa4 (Wed December 4 2019)

Categories

(Core :: Graphics, defect, P3)

defect

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox-esr68 --- unaffected
firefox71 --- unaffected
firefox72 --- unaffected
firefox73 --- disabled
firefox74 --- disabled
firefox75 --- fix-optional

People

(Reporter: alexandrui, Unassigned)

References

(Regression)

Details

(Keywords: perf, perf-alert, regression)

We have detected an awsy regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=3a083701018bf872acfc5e391312042d8d246aa4

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

3% Explicit Memory windows7-32-shippable opt 292,050,017.04 -> 301,342,667.60
3% Explicit Memory windows7-32 opt 292,771,546.53 -> 300,284,872.30
2% Explicit Memory windows7-32 opt 292,529,978.24 -> 299,596,814.31
2% Explicit Memory windows7-32 opt 293,403,644.06 -> 300,135,796.56

You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=24471

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://wiki.mozilla.org/AWSY/Tests

Flags: needinfo?(htwyford)
Component: Performance → Address Bar
Product: Testing → Firefox
Target Milestone: --- → Firefox 73
Version: Version 3 → unspecified

It looks like the same as bug 1584101 that happened the last time we enabled the feature, the fixes there should have helped, but apparently they didn't.

Flags: needinfo?(htwyford) → needinfo?(dao+bmo)
Priority: -- → P2
Points: --- → 3
See Also: → 1584101

(In reply to Marco Bonardo [:mak] from comment #1)

It looks like the same as bug 1584101 that happened the last time we enabled the feature, the fixes there should have helped, but apparently they didn't.

They did, I confirmed on Try before landing and post-landing improvements were noted in the bug.

Flags: needinfo?(dao+bmo)

looking at sub tests, the regression is on tabs closed, and afaict from comparing the memory logs, the difference is still in
8.50 MB (112.91%) ── gfx/heap-textures

Seems like we need to go back in time on Try to figure out if and when this regressed again.

Flags: needinfo?(htwyford)

I'll investigate this.

We don't need to track this for 73 because the megabar currently targets 74, that is, it won't merge to beta 73.

Assignee: nobody → dao+bmo
Flags: needinfo?(htwyford)
Blocks: 1607747
No longer blocks: 1600879
Target Milestone: Firefox 73 → Firefox 74

Hi Dao, is the megabar still targeting 74? And who's working on it?

Flags: needinfo?(dao+bmo)

(In reply to Ethan Tseng [:ethan] from comment #7)

Hi Dao, is the megabar still targeting 74?

Potentially.

And who's working on it?

I've been investigating this but don't have a definite solution yet.

Bug 1584101's patch is working in the sense that we set and remove the urlbar-exceeds-toolbar-bounds attribute as expected. That is, we set it when focusing the address bar (because it then renders outside the toolbar) and remove it when removing focus. With the megabar design disabled, we only set this attribute when opening the results panel, not when merely focusing the address bar. When bug 1584101's patch landed, the megabar design was temporarily disabled and the regression went away. Now with the megabar enabled, the regression is back.

I think this means that either AWSY finishes with the address bar focused or Gecko reserves that texture despite the fact that we set overflow: -moz-hidden-unscrollable; on the toolbar after unfocusing the address bar. Timothy or Matt, is the latter theory plausible and is this something that could be fixed on the Gecko side?

Flags: needinfo?(tnikkel)
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(dao+bmo)

(In reply to Dão Gottwald [::dao] from comment #8)

I think this means that either AWSY finishes with the address bar focused or Gecko reserves that texture despite the fact that we set overflow: -moz-hidden-unscrollable; on the toolbar after unfocusing the address bar. Timothy or Matt, is the latter theory plausible and is this something that could be fixed on the Gecko side?

It's not obvious how it would (but hard to reason about given multiple backgrounds). We should be clipping everything to the bound rect when -moz-hidden-unscrollable is present.

Flags: needinfo?(matt.woodrow)

I guess we finish the test with the address bar focused then. Alexandru, how can I run this test locally?

Flags: needinfo?(aionescu)

Dao, I don't know. Maybe it's worth asking Drew.

Flags: needinfo?(aionescu) → needinfo?(adw)

Standard8 told me it's ./mach awsy-test but that takes way too long to run. Luckily there's ./mach awsy-test --quick. Based on this I can say that the address bar is only focused at the very beginning of the test. I also checked that the urlbar-exceeds-toolbar-bounds attribute is not set on the toolbar at the end of the test.

Now I'm looking again at the subtests:
https://treeherder.mozilla.org/perf.html#/comparesubtest?framework=4&originalProject=autoland&originalSignature=2118213&newProject=autoland&newSignature=2118213&originalRevision=bcedfb13f4b2591a811e4dd693c479218e2cd24c&newRevision=3a083701018bf872acfc5e391312042d8d246aa4

The alert is for Tabs closed, but the regression is already there for After tabs open, it's just relatively smaller. Ironically, there doesn't seem to be a regression for Fresh start which is when the address bar is or just had been focused.

I'm not sure where to go from here, but I still suspect that in the end this is either a graphics bug or just invalid (i.e. it's expected that the toolbar using a larger texture at the beginning of the test somehow affects the above subtests). I'll move this to Core::Graphics for now. :/

Assignee: dao+bmo → nobody
Points: 3 → ---
Component: Address Bar → Graphics
Flags: needinfo?(adw)
Priority: P2 → --
Product: Firefox → Core
Target Milestone: Firefox 74 → ---
Priority: -- → P3
Blocks: 1614561
No longer blocks: 1607747
Has Regression Range: --- → yes
Severity: normal → S3
Flags: needinfo?(tnikkel)

Hi :dao, I'm closing this as WONTFIX based on comment #12. Furthermore, we don't test on windows 7 anymore so it could be difficult to reproduce this. Feel free to change this if needed.

Status: NEW → RESOLVED
Closed: 3 months ago
Flags: needinfo?(dao+bmo)
Resolution: --- → WONTFIX

If there was still a problem here, bug 1921811 likely solved this problem by changing the front-end implementation to using the newish popover API.

Depends on: 1921811
Flags: needinfo?(dao+bmo)
Resolution: WONTFIX → WORKSFORME
You need to log in before you can comment on or make changes to this bug.