1068399 - Enable GGC on the mobile browser

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

tbpl likes it: https://tbpl.mozilla.org/?tree=Try&rev=46c26a4a68a7

Comment 1

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

Attachment: Enable GGC on the mobile browser

Hm... I need a build peer who isn't on vacation. Ted!

The only question is whether this is the right file to touch for mobile Firefox.

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 8490455 [details] [diff] [review]
Enable GGC on the mobile browser

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

Is there any reason not to just enable this globally in configure at this point? Will it break Thunderbird or something?

Comment 3

[Terrence Cole [:terrence]]

I was having the same thought. I don't recall comm-central having any JSAPI code that needed fixing when we implemented exact rooting, so I don't think there would be any problem just enabling this globally.

Comment 4

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

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #2)
> Is there any reason not to just enable this globally in configure at this
> point? Will it break Thunderbird or something?

I was thinking that too, but then decided that might be better as a separate step, in case we need to roll any of these back individually sometime soon.

Comment 5

[(not currently active) Ted Mielczarek]

I am totally fine with your risk management. File a followup to enable it globally?

Comment 6

[Carsten Book [:Tomcat]]

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

Comment 7

[Bob Clary [:bc] (inactive)]

It appears this patch may be responsible for a very small regression in Autophone Throbber stop times. It is difficult to see in the zoomed graph for 9/25 but it is more apparent for longer time ranges. 

http://phonedash.mozilla.org/#/org.mozilla.fennec/throbberstop/local-twitter/norejected/2014-09-25/2014-09-26/notcached/noerrorbars/standarderror

It is most apparent for the nexus-s-3 device running Android 2.3 and testing mozilla-inbound. If you click on a device name in the lower left of the page, it will turn off the graph for that device. If you deselect all of the nexus-s devices except for nexus-s-3 testing mozilla-inbound, you will be able to see it better. If you click on a data point in the graph, you will see a popup showing the build date, device, revision and measurement for that point.

The apparent regression range is https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=dc8fac10012d&tochange=1763f5d09746 which includes this check in.

The tests consists of measuring the Throbber start and stop times for loading simple web pages. The pages may be too small and involve too little js object allocation to benefit from ggc and the regression may just be a result of additional overhead required for ggc.

I'm not sure this regression in Throbber stop times would warrant a bug, but wanted to get your opinion before I decided whether to file one. Is this behavior be expected or would it justify a bug?

Comment 8

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

We looked at it and can see the regression, but after staring at the various machines and things, we're kind of thinking it's too minor to worry about. We would expect *some* difference, either positive or negative.

That said, bug 1075053 may affect this too. It should reduce the memory overhead somewhat, and might even resolve this regression. It hasn't landed yet, but it would be good if you could keep an eye on whether it has some effect.

Comment 9

[Bob Clary [:bc] (inactive)]

Steve, cool. I figured it was too minor to worry about but didn't want to ignore it completely. I'll follow bug 1075053 and keep an eye on it.