言いたいことはそれだけか

KotlinとかAndroidとかが好きです。調べたことをメモします。٩( 'ω' )و

Android N Easter Egg (neko) に見るアンチパターン

🐱が70匹を超えたあたりから挙動がめちゃめちゃ重くなってきたAndroid N Easter Egg (neko) アプリ。 コードを読んで何が悪そうか調べてみました。

あくまで静的解析の結果なので検証してません。ごめんなさい。「ふーん」くらいに思ってください。ツッコミ大歓迎。

調査対象

下記2パターンのタイミングで重くなります。特に2は困った。

  1. 🐱一覧のダイアログでスクロールするとき
  2. Foodを選択した後、Dialogが消えず端末全体がほぼ操作不能になる

何が悪かったか

1. 🐱一覧のスクロールが重い原因

まず、アプリ全体がどうやって集まっている🐱を保存しているか。 SharedPreferenceに seedname を保存しています。

▼PrefState.java

    // Can also be used for renaming.
    public void addCat(Cat cat) {
        mPrefs.edit()
              .putString(CAT_KEY_PREFIX + String.valueOf(cat.getSeed()), cat.getName())
              .commit();
    }

seed は🐱の各パーツの色をretrieveするために使います。 (デフォルトで🐱の名前はseedになっていた気がしますが、🐱の名前はユーザが変えられるので、名前をvalueとして保持しています。)

🐱一覧のActivityの onCreate() でSharedPreferenceから全ての値をとってきて seed から各パーツの色を計算し、Drawableを描画するという処理を🐱の匹数分メインスレッドでやります。ちなみにパーツは全部で28か所くらいあります。そりゃ重くもなりますね。

2. Foodを置いた直後が重い原因

Foodが選択された後の処理ですが、🐱と同じくSharedPreferenceに書き込みます。

▼PrefState.java

    public void setFoodState(int foodState) {
        mPrefs.edit().putInt(FOOD_STATE, foodState).commit();
    }

これは別にいいんですけど、問題(の原因の一つ)は🐱と同じPreferenceに書いてしまっていること。 このSharedPreferenceを監視しているのは下記の2クラス。

A) NekoTile.java (QuickSettingsにあるFood用のタイル)
B) NekoLand.java (🐱一覧画面のActivity)

Aについて、Foodが選択された後にそのFoodの画像とテキストをタイルに表示するために監視しています。ここは別に問題なし。
問題はB。Bの方は🐱が増えたタイミングを取得するために監視しているようです。同じSharedPreferenceを監視しているせいでFoodの値が変わったタイミングでもここが動いてしまいます。

▼NekoLand.java

    @Override
    public void onPrefsChanged() {
        updateCats();
    }

    private void updateCats() {
        Cat[] cats;
        if (CAT_GEN) {
            cats = new Cat[50];
            for (int i = 0; i < cats.length; i++) {
                cats[i] = Cat.create(this);
            }
        } else {
            cats = mPrefs.getCats().toArray(new Cat[0]);
        }
        mAdapter.setCats(cats);
    }

▼PrefState.java

    public List<Cat> getCats() {
        ArrayList<Cat> cats = new ArrayList<>();
        Map<String, ?> map = mPrefs.getAll();
        for (String key : map.keySet()) {
            if (key.startsWith(CAT_KEY_PREFIX)) {
                long seed = Long.parseLong(key.substring(CAT_KEY_PREFIX.length()));
                Cat cat = new Cat(mContext, seed);
                cat.setName(String.valueOf(map.get(key)));
                cats.add(cat);
            }
        }
        return cats;
    }

ちなみにCatのコンストラクタで各パーツの色の計算とDrawableへのtintが行われています。差分チェック?知らない子ですね…
さらにAdapterのsetCats()notifyDataSetChanged() を呼び出しちゃっています。おそらく全ての🐱に対して再描画が走るんじゃないかな…

さらに悪いのが、SharedPreferenceの監視リスナーをonCreate()でつけています。(当然removeするタイミングは対となるonDestroy()です。)つまり、アプリがバックグラウンドにいる間も上記処理が走ってしまいます。画面に🐱一覧が表示されていないのに、Foodを置いただけで重くなってたのはそのためっぽい。

そもそもFoodの値が変更されたタイミングで🐱の再描画を走らせる必要がないのだから同じSharedPreferenceを利用しお互いそこを監視するのよくないですね。

どうしたらよかったか

ざっと思いついたのでこんな感じ。

  • まずはcatとfoodの保存場所を分離し、それぞれ必要な範囲のみ監視すること。Foodに関してはおそらくこれだけで問題ない。
  • cat側の監視タイミングを見直す。バックグラウンドにいる間に更新が必要なければ監視をonResume() / onPause()の間だけにする
  • 🐱の描画はできるだけ非同期で別スレッドで行う。実際にviewを触らないといけない部分だけUI Threadで。AOSPだと気軽にKotlinで書いたり外部ライブラリ導入したりできないからちょっと非同期処理面倒くさそう。

まぁEaster Egg ですもんね。作った人も100匹くらい集めるような使われ方するとも思ってなかったのかもしれない…