nsAppShell::Init can wait quite a while calling CGSSetDebugOptions

RESOLVED FIXED in Firefox 56

Status

()

P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mconley, Assigned: jaas)

Tracking

(Blocks: 1 bug)

unspecified
mozilla56
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [reserve-photon-performance][qf:p3][tpi:+])

Attachments

(1 attachment)

This is some kind of undocumented API for mac OS which allows us to disable event logging, I believe.

http://searchfox.org/mozilla-central/rev/61054508641ee76f9c49bcf7303ef3cfb6b410d2/widget/cocoa/nsAppShell.mm#335

This was added as a workaround to security bug 1092855.

It appears, however, that this was fixed in an update to OS X 10.10.2: https://bugzilla.mozilla.org/show_bug.cgi?id=1092855#c200 (see http://support.apple.com/en-us/HT204244 in particular).

Here's a profile where it shows us waiting for 34ms before first paint to talk to this API:

https://perfht.ml/2s6A5dD

Presumably we're waiting on some other thread to finish up some work before it's able to return to us.

Now that Apple has released a fix, I wonder if it's been sufficient time to remove this workaround and save us this potential first paint bottleneck.

Hey florian, I know that OS X isn't exactly our most popular platform, but would this be interesting to put under photon-startup?
Flags: needinfo?(florian)
(Reporter)

Comment 1

2 years ago
(For context, I noticed this while debugging ts_paint regressions in bug 1357093)
(In reply to Mike Conley (:mconley) from comment #0)

> Hey florian, I know that OS X isn't exactly our most popular platform, but
> would this be interesting to put under photon-startup?

I would say no, because I don't expect to ever be able to work on this myself within the photon timeframe, but it would be very appreciated if someone fixes it.
Flags: needinfo?(florian)
Whiteboard: [photon-performance] → [photon-performance][qf]
Whiteboard: [photon-performance][qf] → [photon-performance][qf] [triage]
Flags: qe-verify?
Priority: -- → P3
Whiteboard: [photon-performance][qf] [triage] → [reserve-photon-performance][qf]

Updated

2 years ago
Whiteboard: [reserve-photon-performance][qf] → [reserve-photon-performance][qf][tpi:+]
Whiteboard: [reserve-photon-performance][qf][tpi:+] → [reserve-photon-performance][qf:p3][tpi:+]
(Assignee)

Updated

2 years ago
Assignee: nobody → jaas
(Assignee)

Comment 3

2 years ago
Posted patch fix v1.0Splinter Review
Let's just only make this call on OS versions 10.10.0 and 10.10.1 -- the only two OS version affected by the bug.
Attachment #8884176 - Flags: review?(mstange)
Status: NEW → ASSIGNED
Priority: P3 → P1
Comment on attachment 8884176 [details] [diff] [review]
fix v1.0

Review of attachment 8884176 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8884176 - Flags: review?(mstange) → review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 6

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/80e1f0ebd048
Only turn off CGEvent logging on buggy versions of OSX to avoid delays in first paint. r=mstange
Keywords: checkin-needed

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/80e1f0ebd048
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Iteration: --- → 56.3 - Jul 24
Flags: qe-verify? → qe-verify-
You need to log in before you can comment on or make changes to this bug.