1378092 - Consider to remove MaybeGC from AutoEntryScript::~AutoEntryScript()

Status: RESOLVED FIXED

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

Description

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

Attachment: remove_maybegc.diff

remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=c46a27e09549cd895a9bb495508b5ba8fa37350b
remote: 
remote: It looks like this try push has talos jobs. Compare performance against a baseline revision:
remote:   https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=c46a27e09549cd895a9bb495508b5ba8fa37350b

Comment 1

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

Comment on attachment 8883258 [details] [diff] [review]
remove_maybegc.diff

Assuming bug 1377131 gets fixed, I think we should try to remove this.
Calling GC at somewhat random times is bad.

Comment 2

[Jon Coppeard (:jonco)]

Comment on attachment 8883258 [details] [diff] [review]
remove_maybegc.diff

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

It would be good to get rid of this if we can without regressing our benchmarks tests too much.

Comment 3

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

I couldn't find sunspider on talos, but kraken might actually get faster
(I tested on top of bug 1377131)

Comment 4

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This is an assigned P1 bug without activity in two weeks. 

If you intend to continue working on this bug for the current release/iteration/sprint, remove the 'stale-bug' keyword.

Otherwise we'll reset the priority of the bug back to '--' on Monday, August 28th.

Comment 5

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

remote: View the pushlog for these changes here:
remote:   https://hg.mozilla.org/try/pushloghtml?changeset=6e55d3bb1a35613482f1e9c437cf254e606507cd
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e55d3bb1a35613482f1e9c437cf254e606507cd
remote: 
remote: It looks like this try push has talos jobs. Compare performance against a baseline revision:
remote:   https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=6e55d3bb1a35613482f1e9c437cf254e606507cd
remote: recorded changegroup in replication log in 0.017s

Comment 6

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

I did a AWFY push, and couldn't see difference in sunspider. (Win64).
So, I'm planning to push this, and if there are too bad regressions, we can back this out.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/76f8a8ccd626
Consider to remove MaybeGC from AutoEntryScript::~AutoEntryScript(), r=jonco

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/76f8a8ccd626