1447391 - Excessive time spent in GCCellPtr::mayBeOwnedByOtherRuntimeSlow()

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Steve Fink [:sfink] [:s:]]

jesup was looking at a profile that takes 863ms out of a several-second CC in the above function.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Early return for strings and symbols when tracing for CC

We only need to do mayBeOwnedByOtherRuntimeSlow for strings and symbols, neither of which can be a member of a cycle.

Comment 2

[Steve Fink [:sfink] [:s:]]

The profile is https://perf-html.io/public/4e702d827135a52bf292c4e5035bb4748051d3fb/calltree/?hiddenThreads=&range=91.6517_100.0366&thread=5&threadOrder=0-2-3-4-5-6-8-9-1-7&v=3 and was posted on reddit at https://www.reddit.com/r/firefox/comments/85lqf9/random_stutters_and_lags_in_600b4/

Comment 3

[Andrew McCreight (out of office until 8/21) [:mccr8]]

Comment on attachment 8960700 [details] [diff] [review]
Early return for strings and symbols when tracing for CC

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

Thanks for investigating and fixing this.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a65fcbcfe819
Early return for strings and symbols when tracing for CC, r=mccr8

Comment 5

[Cristina Coroiu [:ccoroiu]]

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

Comment 7

[Andrew McCreight (out of office until 8/21) [:mccr8]]

[Tracking Requested - why for this release]: Users are seeing severe performance problems in Firefox 60 that would be helped by this patch. It isn't clear if this is a complete solution or not. (for example: https://www.reddit.com/r/firefox/comments/8if1fs/anyone_else_seeing_massive_cpu_spikes_with_ff_60/ )

Comment 8

[Andrew McCreight (out of office until 8/21) [:mccr8]]

One user on Reddit was still experiencing problems with Firefox 62, so this may not fix the worst of it.
https://www.reddit.com/r/firefox/comments/8if1fs/anyone_else_seeing_massive_cpu_spikes_with_ff_60/dyrh099/

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Per IRC discussion with mccr8, it doesn't appear that we need to backport this to 60 after all.