Closed Bug 1345315 Opened 8 years ago Closed 8 years ago

_handleTabTelemetryEnd can be too expensive

Categories

(Firefox :: Tabbed Browser, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 55
Iteration:
55.4 - May 1
Performance Impact low
Tracking Status
firefox55 --- fixed

People

(Reporter: ehsan.akhgari, Assigned: dao)

References

(Blocks 1 open bug)

Details

(Whiteboard: [photon-performance])

Attachments

(1 file)

Whiteboard: [qf-]
Leaving this in the Photon backlog (for bug 1344302), but I'm not sure if this got markef qf:- because it's not really all that important? Or just isn't platform work? If it's not notable, we should maybe just close it?
I talked to Naveed about this. Apparently, qf- means "Triaged. Not a Quantum Flow bug. We don't care about this from a performance perspective for the 57 effort." So it sounds like we should close this out - though I want to make sure we're clear on why. This view of the profile that ehsan is referring to is, I believe, illustrative: https://perf-html.io/public/008281d4c8917aa11d4b7d59f97b2fcee471be7d/calltree/?callTreeFilters=postfixjs-Rl&range=3.5175_13.8879&thread=0 123ms over the course of a 14s profile isn't huge, but it's also not great for what should be a really cheap operation. Was this qf-'d in triage for a reason we're not seeing here?
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(nihsanullah)
Resolution: --- → INVALID
Uh, whoops, didn't mean to close this as INVALID.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Whiteboard: [qf-] → [photon] [qf-]
I see FX_TAB_ANIM_ANY_FRAME_INTERVAL_MS for instance doesn't have alert_emails set. Is anyone actively keeping track of these probes? Looks like they're more than four years old -- what's their track record of catching regressions?
Blocks: 828097
Good question. digitarald, do you know if anybody is paying attention to that probe?
Flags: needinfo?(hkirschner)
I looked at FX_TAB_ANIM_ANY_FRAME_INTERVAL_MS and found it only focused on the easiest path (1/2 tabs), not measuring perceived performance. Given that all animations should be on the compositor in the future, I don't see the need for having this. Marking qf:p3 as it impacts performance and is a low hanging fruit.
Flags: needinfo?(nihsanullah)
Flags: needinfo?(hkirschner)
Whiteboard: [photon] [qf-] → [photon] [qf:p3]
Whiteboard: [photon] [qf:p3] → [qf:p3]
Flags: qe-verify?
Priority: -- → P2
Whiteboard: [qf:p3] → [photon-performance] [qf:p3]
I also checked that tart doesn't depend on this code.
Assignee: nobody → dao+bmo
Status: REOPENED → ASSIGNED
Flags: qe-verify? → qe-verify-
Priority: P2 → P1
Comment on attachment 8858650 [details] Bug 1345315 - Remove obsolete tab animation telemetry. https://reviewboard.mozilla.org/r/130624/#review133940 Yeah, as the new Photon tab animation is going to be on the GPU, I don't see this being very useful anymore. Thanks dao!
Attachment #8858650 - Flags: review?(mconley) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 48dafd25edaf -d 4e489e84adfd: rebasing 390561:48dafd25edaf "Bug 1345315 - Remove obsolete tab animation telemetry. r=mconley" (tip) merging browser/base/content/tabbrowser.xml merging toolkit/components/telemetry/Histograms.json warning: conflicts while merging browser/base/content/tabbrowser.xml! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1685702d2d6a Remove obsolete tab animation telemetry. r=mconley
Status: ASSIGNED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Iteration: --- → 55.4 - May 1
Performance Impact: --- → P3
Whiteboard: [photon-performance] [qf:p3] → [photon-performance]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: