Roberto Agostino Vitillo (:rvitillo) Reporter
	Description • 9 years ago

There is a high fraction (up to 10%), for recent build-ids, of non e10s saved_session submissions that have child histograms but don't have a browser window set to e10s. See [1]. Is this expected?

[1] http://nbviewer.ipython.org/gist/vitillo/743c26f0a1fa71bd8f00

	Mike Conley (:mconley) (:⚙️)
	Comment 1 • 9 years ago

jimm - could the child payloads be from plugin processes?

	Jim Mathies [:jimm]
	Comment 2 • 9 years ago

Hmm, not sure. A couple thoughts - first we have child processes that run even when e10s is turned off. For example the background page thumbs service does this. Would that process collect GC_MS_children telemetry? I do not know. The other idea I had was that some users startup with e10s but then get prompted to disable and restart. This probably doesn't constitute a single session of telemetry data but I'm not 100% sure on that. I don't know what a saved_session can include.

	Vladan Djeric (:vladan)
	Comment 3 • 9 years ago

Roberto, can you do some manual testing here? Trigger the thumbs service (talk to Tim Taubert) in single-process and see what kind of Telemetry is collected

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 4 • 9 years ago

Not sure offhand here, but it looks like investigation is ongoing already.
Can we have any scenarios with e10s on but E10S_AUTOSTART==false, e.g. like the former "start e10s window" functionality?

	Roberto Agostino Vitillo (:rvitillo) Reporter
	Comment 5 • 9 years ago

I can confirm that in a non-e10s session the thumbs service generates a child payload with many histograms among which also GC_MS.

George, is there a way to discern between the process type (e.g. content or thumbs service) in telemetry payloads?

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 6 • 9 years ago

So, per IRC the thumbnailing probably runs in the normal content process if e10s is on.
So it should be enough to check if we are in a child and e10s is enabled via `Services.appinfo.browserTabsRemoteAutostart` and not enable Telemetry accordingly here:
https://hg.mozilla.org/mozilla-central/annotate/2ddec2dedced/toolkit/components/telemetry/TelemetryController.jsm#l593

I wonder what fx versions are affected here, given storage cost server-side this could be shipping-relevant for unified telemetry.

	Mike Conley (:mconley) (:⚙️)
	Comment 7 • 9 years ago

vladan brought up a good point in IRC - if we're doing thumbnailing in the same content process, we risk janking the content process when generating the thumbnails.

Sequestering the thumbnail generation into a separate content process, distinct from the ... uh, main? content process, might be a nice perf win.

I've filed bug 1187441 about this.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 8 • 9 years ago

This only initializes Telemetry in content processes if e10s is enabled. This exposed that the child histogram test is pretty fragile, so i made that more predictable.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 9 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7233e84e726b

	Alessio Placitelli [:Dexter]
	Comment 10 • 9 years ago

Comment on attachment 8639357 [details] [diff] [review]
Non e10s submissions with child payloads

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

Looks good.

Nit: I would suggest changing the commit message to something a bit more descriptive about what the patch does. It's not mandatory, though, just a matter of personal taste.

	Pulsebot
	Comment 11 • 9 years ago

https://hg.mozilla.org/integration/fx-team/rev/060139fdfbfd

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 12 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/060139fdfbfd

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 14 • 9 years ago

Comment on attachment 8641651 [details] [diff] [review]
Don't generate child Telemetry payloads for the thumbnail service in non-e10s

Approval Request Comment
[Feature/regressing bug #]:
Telemetry Unification
[User impact if declined]:
This is a fixup for Telemetry. We were submitting data for child processes with e10s off because of the Thumbnailer service.
This has storage costs both client-side and server-side, also bandwidth costs.
It is part of a mostly fixup & diagnostic uplift batch for 41: http://bit.ly/1LYhA16
[Describe test coverage new/current, TreeHerder]:
We have automated test-coverage, it baked on Nightly.
[Risks and why]:
Low-risk, we haven't seen anything crop up on Nightly here.
[String/UUID change made/needed]:
None.

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 15 • 9 years ago

Comment on attachment 8641651 [details] [diff] [review]
Don't generate child Telemetry payloads for the thumbnail service in non-e10s

Approved since reporting childProcess telemetry data increases server storage even though this data is not needed in e10s-off mode.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 16 • 9 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/2fe3e641556d