1338518 - Consider to not run 40ms GC slices in parent process when user is interacting with the browser

Status: RESOLVED FIXED

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

Description

[Olli Pettay [:smaug][[e-mail]]]

If input events go to a child process, nothing in parent is possibly painted and refresh driver doesn't tick there, meaning that it uses longer gc slices. Yet that may affect when the input events are sent to the child, or if child does some sync IPC message to parent, it may be blocked because of GC.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Attachment: wip

Is this a silly idea?

Comment 2

[Jon Coppeard (:jonco)]

Comment on attachment 8836118 [details] [diff] [review]
wip

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

This sounds like a good idea to me.

I don't know if we've got any way of measuring responsiveness in this situation.  But if this is the way that things worked pre-e10s then I think we should do that.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/51ba586bd63a
make parent process to use 10ms gc slices when user is active, r=jonco

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/51ba586bd63a

Comment 5

[:Cykesiopka]

Attachment: Bug 1338518 - Followup: Fix incorrect `if` block in SetMemoryGCSliceTimePrefChangedCallback().

This was found by -Wmisleading-indentation:
> nsJSEnvironment.cpp: In function ‘void SetMemoryGCSliceTimePrefChangedCallback(const char*, void*)’:
> nsJSEnvironment.cpp:2329:3: error: this ‘if’ clause does not guard... [-Werror=misleading-indentation]
>    if (pref > 0 && pref < 100000)
>    ^~
> nsJSEnvironment.cpp:2331:5: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the ‘if’
>      JS_SetGCParameter(sContext, JSGC_SLICE_TIME_BUDGET, pref);

Review commit: https://reviewboard.mozilla.org/r/111888/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/111888/

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8836470 [details]
Bug 1338518 - Followup: Fix incorrect `if` block in SetMemoryGCSliceTimePrefChangedCallback().

https://reviewboard.mozilla.org/r/111888/#review113148

oops, thanks.

Comment 7

[:Cykesiopka]

Comment on attachment 8836470 [details]
Bug 1338518 - Followup: Fix incorrect `if` block in SetMemoryGCSliceTimePrefChangedCallback().

Thanks.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8fba5c26a855cc121f60a6545e2b83ca38e6c358

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/464e94458039
add missing {}, r=smaug

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/464e94458039

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8836470 [details]
Bug 1338518 - Followup: Fix incorrect `if` block in SetMemoryGCSliceTimePrefChangedCallback().

(This landed, so, it is not obsolete)