Closed Bug 701068 Opened 13 years ago Closed 11 years ago

investigate the locale setting code and implications on android


(Firefox for Android Graveyard :: General, defect, P3)




Firefox 28
Tracking Status
fennec - ---


(Reporter: Pike, Assigned: rnewman)



Doug mentioned that the getting/setting locale dance was removed, but it's unclear what the fall-out is.

Doug, can you provide pointers on what exactly the code change was, so that we can try to reproduce?
Assignee: nobody → doug.turner
Priority: -- → P3
we removed the following from the startup path:

We want to disable this code if possible.  It is about 145ms

SharedPreferences settings = getPreferences(Activity.MODE_PRIVATE);
String localeCode = settings.getString(getPackageName() + ".locale", "");

if (localeCode != null && localeCode.length() > 0)
We're talking about, right?

So, here's my take:

This is only an issue if we're trying to do explicit locale selection in a multi-locale build. In my local testing with the multi-locale builds I did post bug 702302, things seemed to work allright for the matched locales. And that tree had the commenting out already.

I assume in our planned setup with single-locale builds with an option to do multi, that should be OK.

I also speculate that the expensive call is setting the locale, as that's probably ripping out strings in the android process and paging in new ones. Not sure if getting the pref is the expensive part.

Which leads me to, was the conditional and the pref handling working on the right conditions? Maybe it'd be cheap to keep, if we make sure the pref isn't set accidently. Which might be easier, if we restrict it to explicitly set within java code, and not an observer of stuff within gecko.
Depends on: 702302
tracking-fennec: --- → 11+
i am not working on these right now.  resetting assignee.
Assignee: doug.turner → nobody
tracking-fennec: 11+ → ?
tracking-fennec: ? → -
I recall that Brad added more detail:

The costly part is reading the android pref.

Which makes all other optimizations mute. I'm wondering if there'd be a point in our startup path where we read prefs anyway and the getter may be cheap. In that scenario, we could be able to make startup regressions only affect users that actually do switch locales, which might be OK.
Things of note:

* That code ran in a background thread.
* And in the exact same runnable, we check for Sync migration, _write to prefs_ for our last launch time, and send some broadcasts to trigger FHR and product announcements.
* Reading a prefs file that's already been loaded, or will be loaded shortly thereafter, is essentially free.

In other words, I think there's a path forward here that doesn't impact main-thread perf.
We fixed this in Bug 936756.
Closed: 11 years ago
Depends on: 936756
Resolution: --- → FIXED
Assignee: nobody → rnewman
Target Milestone: --- → Firefox 28
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.