Child INPUT_EVENT_RESPONSE_MS p50 regressed from 9.28 to 10.22ms on 2017-05-06 build

RESOLVED INCOMPLETE

Status

()

Core
General
RESOLVED INCOMPLETE
5 months ago
4 months ago

People

(Reporter: Away for a while, Assigned: farre)

Tracking

({leave-open})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 obsolete attachments)

(Reporter)

Description

5 months ago
Telemetry: https://mzl.la/2rp2aeP
(Reporter)

Comment 1

5 months ago
Andreas, I think the telemetry collection code in bug 1322184 may be the reason for this regression...

The good news is that if this is the case, AutoCollectVsyncTelemetry automatically turns itself off for the release channel, so we can just ignore the regression.  But we should verify whether this is really the regression we are looking at.  I would like to disable this probe for a few days and watch INPUT_EVENT_RESPONSE_MS to see if it drops down and in that case, mark this bug as WONTFIX and move on!
Blocks: 1322184
Keywords: leave-open
(Reporter)

Comment 2

5 months ago
Created attachment 8869307 [details] [diff] [review]
Temporarily disable the collection of content js delay event telemetry probes to investigate whether they are the cause of child process INPUT_EVENT_RESPONSE_MS regressions
Attachment #8869307 - Flags: review?(afarre)
(Assignee)

Updated

5 months ago
Attachment #8869307 - Flags: review?(afarre) → review+
(Assignee)

Updated

5 months ago
Keywords: checkin-needed
(Assignee)

Comment 3

5 months ago
Sorry, shouldn't have set the checkin-needed flag there. Don't know what happened.
Keywords: checkin-needed
(Assignee)

Comment 4

5 months ago
Created attachment 8869396 [details] [diff] [review]
0001-Bug-1366156-Temporarily-stop-recording-vsync-message.patch

What I should've said is that we should probably do this as well.
Assignee: nobody → afarre
Attachment #8869396 - Flags: review?(ehsan)
(Reporter)

Comment 5

5 months ago
Comment on attachment 8869396 [details] [diff] [review]
0001-Bug-1366156-Temporarily-stop-recording-vsync-message.patch

Yeah makes sense.  I'll fold this into my patch to make it easier to back the whole thing out after a few days.  Thanks!
Attachment #8869396 - Flags: review?(ehsan) → review+

Comment 6

5 months ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfdebc342d2e
Temporarily disable the collection of content js delay event telemetry probes to investigate whether they are the cause of child process INPUT_EVENT_RESPONSE_MS regressions; r=farre

Comment 7

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/dfdebc342d2e
(Reporter)

Comment 8

5 months ago
Comment on attachment 8869307 [details] [diff] [review]
Temporarily disable the collection of content js delay event telemetry probes to investigate whether they are the cause of child process INPUT_EVENT_RESPONSE_MS regressions

It doesn't look like this has caused any measurable change in the INPUT_EVENT_RESPONSE_MS telemetry probe.  I'm going to revert this patch now...
Attachment #8869307 - Attachment is obsolete: true
(Reporter)

Updated

5 months ago
Attachment #8869396 - Attachment is obsolete: true
(Reporter)

Comment 9

5 months ago
(In reply to :Ehsan Akhgari (super long backlog, slow to respond) from comment #8)
> Comment on attachment 8869307 [details] [diff] [review]
> Temporarily disable the collection of content js delay event telemetry
> probes to investigate whether they are the cause of child process
> INPUT_EVENT_RESPONSE_MS regressions
> 
> It doesn't look like this has caused any measurable change in the
> INPUT_EVENT_RESPONSE_MS telemetry probe.  I'm going to revert this patch
> now...

https://hg.mozilla.org/integration/mozilla-inbound/rev/1658b1c63226
Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/1658b1c63226
Backed out bug 1352889 today to see whether it is the cause.
The backout of bug 1352889 didn't make any difference.  I'm going to reland it, but that means you need to look for a different culprit here.
Flags: needinfo?(kchen)
See bug 1362094 comment 25. We decided this bug is INCOMPLETE due to the limitation of the probes.
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Flags: needinfo?(kchen)
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.