1384337 - Ghost window telemetry is broken

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, defect, P1)

Description

[Andrew McCreight [:mccr8]]

Since 7/12, ghost windows have been reported as 0, which is bad.

Comment 1

[Andrew McCreight [:mccr8]]

This is bad because we're trying to track no regressions in this metric for 57.

Comment 2

[Andrew Overholt [:overholt]]

(Temporarily giving to mccr8 as he's the most connected to this issue right now)

Comment 3

[Andrew McCreight [:mccr8]]

Eric said he'd look at it.

Comment 4

[Eric Rahm [:erahm]]

We know that ghost window reporting still works when generating memory reports so at least that part isn't broken. That implies that the logic to periodically calculate ghost windows triggered by windows closing [1] or cycle collection finishing [2] is somehow broken. Before we just always calculated the value when telemetry asked for it, but now we depend on a cached value from the periodic calcuation.

Andrew pointed out a few spikes on telemetry [1] where we see a drop on 7/1 (corresponding with bug 1375281 which switched to request idle callback) and rise on 7/6 (corresponding with bug 1377767 which fixed us using an old api for idle callback), so those are probably red herrings. It's pretty clear that bug 1376038 is the regression.

[1] http://searchfox.org/mozilla-central/rev/8a61c71153a79cda2e1ae7d477564347c607cc5f/dom/base/nsWindowMemoryReporter.cpp#611-634
[2] http://searchfox.org/mozilla-central/rev/8a61c71153a79cda2e1ae7d477564347c607cc5f/dom/base/nsWindowMemoryReporter.cpp#636-648
[3] https://mzl.la/2w1fU1E

Comment 5

[Eric Rahm [:erahm]]

Attachment: Always update cached ghost window count

Currently the cached count is only updated if a table was passed in to keep
track of the window IDs. This changes the behavior to always update the count
regardless of whether a table is passed in.

MozReview-Commit-ID: EkfzLemVJyV

Comment 6

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c8f400fc23a72fab985d3baa08b5cc22d2190326
Bug 1384337 - Always update cached ghost window count. r=mccr8

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c8f400fc23a7

Comment 8

[Andrew McCreight [:mccr8]]

I'm seeing ghost windows in telemetry again. Thanks, Eric.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Backed out from mozilla-central along with bug 1376038 to help with the investigation of bug 1388111.

https://hg.mozilla.org/integration/mozilla-inbound/rev/69b75c23e0301ffda2e2bb5bb40d4e803ac6e242

Note that the status is a little screwy here because Beta56 still has the patches in question.

Comment 10

[Carsten Book [:Tomcat]]

also backedout from m-c
https://hg.mozilla.org/mozilla-central/rev/69b75c23e030

Comment 11

[Jim Mathies [:jimm]]

Hey Eric, what's the plan here, reland?

Comment 12

[Andrew McCreight [:mccr8]]

This bug is "fixed" in the sense that ghost window telemetry is working. If erahm relands, I assume he'll just roll that into his patch for the other bug.

Comment 13

[Eric Rahm [:erahm]]

Yeah I can look into relanding tomorrow. Keeping ni for now.

Comment 14

[Eric Rahm [:erahm]]

Okay as :mccr8 noted I'll just roll this up into bug 1376038.