Closed Bug 1257623 Opened 8 years ago Closed 8 years ago

CC / GC between each cycle of tpaint

Categories

(Testing :: Talos, defect)

defect
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: mconley, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

I think at least part of the e10s regression on tpaint is accounted for by the fact that both processes are free'd up enough to trigger CC / GC during some cycles, causing spikes in the time to open windows.

We should force CC / GC between each window open.
https://reviewboard.mozilla.org/r/40909/#review37475

::: testing/talos/talos/cmdline.py:89
(Diff revision 1)
>      add_arg('--spsProfileInterval', dest='sps_profile_interval', type=int,
>              help="How frequently to take samples (ms)")
>      add_arg('--spsProfileEntries', dest="sps_profile_entries", type=int,
>              help="How many samples to take with the profiler")
>      add_arg('--extension', dest='extensions', action='append',
> -            default=['${talos}/talos-powers/talos-powers-signed.xpi',
> +            default=['${talos}/talos-powers/',

we shouldn't remove this unless you have a trick I don't know about.

::: testing/talos/talos/config.py:184
(Diff revision 1)
>          'devtools.chrome.enabled': False,
>          'devtools.debugger.remote-enabled': False,
>          'devtools.theme': "light",
>          'devtools.timeline.enabled': False,
> -        'identity.fxaccounts.migrateToDevEdition': False
> +        'identity.fxaccounts.migrateToDevEdition': False,
> +        'xpinstall.signatures.required': False,

I assume this is debugging?

::: testing/talos/talos/startup_test/tpaint.html:40
(Diff revision 1)
>    if (windowIndex >= REPEAT_COUNT) {
>      window.setTimeout(reportTimes, 0);
>      return;
>    }
> +
> +  TalosPowersContent.forceCCAndGC();

iirc, cc | gc requires a bit of time to come back, do we need to wait for a callback?
Yeah, sorry - this patch was a WIP (work-in-progress), which is why there's still some debugging cruft in there, and I hadn't requested review yet.

A for-reals patch is coming up soon.
https://reviewboard.mozilla.org/r/40909/#review37475

> iirc, cc | gc requires a bit of time to come back, do we need to wait for a callback?

I'm using a sync message here to communicate with the parent, which means that it'll return only once the parent has finished the CC / GC.
Attachment #8731890 - Attachment description: MozReview Request: Bug 1257623 - [WIP] CC / GC between each cycle of tpaint. r?jmaher → MozReview Request: Bug 1257623 - CC / GC between each cycle of tpaint. r?jmaher
Attachment #8731890 - Flags: review?(jmaher)
Comment on attachment 8731890 [details]
MozReview Request: Bug 1257623 - CC / GC between each cycle of tpaint. r?jmaher

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40909/diff/1-2/
Comment on attachment 8731890 [details]
MozReview Request: Bug 1257623 - CC / GC between each cycle of tpaint. r?jmaher

https://reviewboard.mozilla.org/r/40909/#review37653

::: testing/talos/talos/talos-powers/components/TalosPowersService.js:62
(Diff revision 2)
>          this.receiveProfileCommand(message);
>          break;
>        }
> +      case "TalosPowersContent:ForceCCAndGC": {
> +        Cu.forceCC();
> +        Cu.forceGC();

does this really do a synchronous execution, or does this fire a command and take time to properly execute?
Attachment #8731890 - Flags: review?(jmaher)
https://reviewboard.mozilla.org/r/40909/#review37653

> does this really do a synchronous execution, or does this fire a command and take time to properly execute?

I believe both forceCC and forceGC are synchronous.
Comment on attachment 8731890 [details]
MozReview Request: Bug 1257623 - CC / GC between each cycle of tpaint. r?jmaher

https://reviewboard.mozilla.org/r/40909/#review37661

given the assertion that forceCC and forceGC are synchronous, lets ship it
Attachment #8731890 - Flags: review+
Blocks: e10s-perf
https://hg.mozilla.org/mozilla-central/rev/77caf1897d7a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.