Closed Bug 701068 Opened 11 years ago Closed 9 years ago
investigate the locale setting code and implications on android
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?
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) GeckoAppShell.setSelectedLocale(localeCode);
We're talking about http://hg.mozilla.org/users/dougt_mozilla.com/birch_native_ui_pre_wipe/rev/5b8a01f59171, 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
i am not working on these right now. resetting assignee.
Assignee: doug.turner → nobody
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.
Status: NEW → RESOLVED
Closed: 9 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.