Retty Tech Blog

実名口コミグルメサービスRettyのエンジニアによるTech Blogです。プロダクト開発にまつわるナレッジをアウトプットして、世の中がHappyになっていくようなコンテンツを発信します。

過去施策コードが消せなくなってしまう構造

この記事は Retty Advent Calendar 2022 Part1 の19日目の記事です。

今年も Part2 があるのでこちらもよろしくお願いします。

Webチームの@sntree_suzunokiです。GoとGraphQLが大好きなバックエンドエンジニアです。今年も去年と変わらず施策開発の傍らパフォーマンス改善に勤しんでいて、その活動の最中にコードの書き方ってやっぱり重要だなと思うものがあったのでそのお話をします。

パフォーマンス改善の内容

改善の対象

パフォーマンス改善の対象となったのはお店の顔ともなるお店詳細ページでした。こちらのページは新しいシステムに移行中であり一部店舗以外は古いシステムで提供されています。
今回はその古いシステムで提供されているページのパフォーマンスが悪く、体験に影響を及ぼしているということでパフォーマンスの改善を行いました。

お店詳細ページ

結果

結果から言うと、関数の分割とコードのお掃除をしただけで、平均で0.3秒のレイテンシ削減を行いました。

平均0.3秒の削減

パフォーマンスを悪くしていた原因は消されなかった過去施策コード

多すぎるSpanの数

原因調査のためにまずtraceを見たところ、表示しているコンテンツに対してDBへのアクセスが異様に多いことがわかり、今回やることはコードのお掃除だと一瞬で方針が決まりました。

実際にコードを追いかけて無駄な処理を洗い出してみると、そのほとんどが過去表示していたコンテンツのデータ取得処理でした。エンジニアとして、出面からコンテンツを取り下げる際に取得の処理から消すことは考えなければいけないことなので、反省するべきだろうなとは思いつつ、 そこにはコードの削除がされづらくなるような構造も見えたのでそちらについて書こうと思います。

過去施策コードが消せなくなってしまう構造

過去施策コードが消されなかったことの理由の一つとしてそうなりやすい環境があったからだと思っています。その環境を作り出していた要因として今回の改善活動で観測できたものが以下の二つです。

名前のカバーする範囲が大きいモデル

今回対応したシステムでは、MVCフレームワークを用いており、Controllerがリクエストを受けたらModelの関数を呼び出してデータ取得および加工を行います。Viewではそのモデルのデータを読み取って画面を構成します。

お店詳細ページでは、RestaurantModel と名付けられたModelが用いられていますが、お店の情報を扱うサービスにおいて、RestaurantModelという名前はとても便利な名前で、ほぼ全てのデータがその配下に入っています。

ほぼ全てのデータやデータストアへのアクセス手段が搭載されたそのモデルは新しい機能を作る分にはとてもイージーです。
新しいコンテンツを表示したい際にはこのモデルに取得の手段を追加するのが一番早いですし、既存の情報を加工して表示したい場合、モデルの中にすでにその情報が入っているのでControllerの最後で加工の処理を呼べば実装できます。新しいページを作る時にもそのモデルを使えばあとはViewを作るだけでできます。

万能モデル

しかし、新しい機能の追加は簡単な反面その強い引力で長年コードを溜め続けたモデルは、複数のページで使われているため影響範囲が広い、そのモデルに生えている各関数や変数が複雑に依存しあっていると言う状態だったため、機能を消すことがとても困難でした。

Rettyではお掃除を積極的に行っている文化があり、これでも小さくなった方ですがRestaurantModelはまだ一万行を超えています。

一万行を超えるモデル

CommonFunction

上でも書いた通り、RestaurantModelは全てのページで用いることが可能で、お店詳細の各ページ(TOP/写真/口コミ/メニュー/座席/地図)はRestaurantModelを用いて実装されいています。

各ページに共通する情報をセットするために、決まった関数を決まった順序で呼び出す必要があり、そんな制約を持った関数群はCommonFunctionとして一つの関数にまとめられていました。

CommonFunction

この関数の中には、全ページで表示されているお店の詳細情報や、各ページで個別に呼び出す関数の前準備に必要な関数の呼び出しが行われています。

今回の改善結果を出した修正のほとんどがこの関数に関連するもので

  1. 一つのページでだけ使うものが何個もあった
  2. CommonFunctionの中に大量の不要なデータ取得処理があった
  3. CommonFunctionで呼んでいる関数をCommonFunction呼び出しの後に個別で呼んでいた

と言う問題があったものを、コードの削除と関数の分割で対応していました。

CommonFunctionがもたらしていた弊害

一つ目の問題点に関しては、コードのパターンが似ているという理由で共通化してしまったため発生した問題です。 残る二つの問題に関しては、CommonFunctionと言う名前をつけてしまったが故に中身を確認しない方向に判断する心理的な環境が出来上がっていたのかなと推測しています。

これら二つの要因により、コードの追加は容易で削除は困難な構造ができていました。

これからの改修方針

今までで問題点を挙げてきたので、ここではその問題点に対してのこれからの改修方針について書こうと思います。

まず前提として、これから自分が入れようと思っている改修の目的は「パフォーマンスが悪化しづらくすること」です。

目的なしに理想を上げようとするならば

  • RestaurantModelが適切なサブドメインに分割されている
  • CommonFunctionがなくなっていて、意図により共通化された関数になっている

を挙げたりするのかなと考えるのですが、お店詳細ページは、新システムへの移行中であるので長期的に見た時にメリットが出る改修は意味がありません。
また、自分の作業する時間も施策開発の合間で出しているので、大きな時間はかけられないです。

従って、コスパ良く対応できて、移行完了までパフォーマンスが悪化しづらくなる構造を作り上げることにフォーカスしようと思っています。

これらの背景から、改修方針は「RestaurantModelの分割はせず、CommonFunctionをなくす」に決定しました。

RestaurantModelの分割は規模が大きい割にパフォーマンスには直接影響しないのでこれからの改修からは外していています。パフォーマンスだけに着目すると、CommonFunctionが原因で発生している関数呼び出しの無駄や重複が大半であるのでこの方針に決定しました。

さらに具体的な方針としては、お店詳細の各ページ(TOP/写真/口コミ/メニュー/座席/地図)でCommonFunctionを呼ぶのをやめてベタ書きするようにする。また、SPとPCでコンテンツ量が違うのでデバイスによって呼ぶ関数を分けるようにしています。

デメリットとしてコードパターンの重複が出てくるのですが、これはそもそも関数呼び出し順序を作っているRestaurantModelをなんとかしないと解消できない問題なので、デメリットとして受けることを決めました。

終わりに

今回挙げた二つの問題は、コードの書き方の問題なので〇〇の原則みたいな名前で言い表すことができると思いますが、具体的にどんなことが起こるみたいなことを語れることはあまりないのでよりリアルな形で紹介しました。

飲食のドメインに限らず他のドメインでも同様の問題は起き得ること、開発者のレベルや役割に関わらず自分達の今書いているコードが最終的にこの結果に至ることができるので気をつけようと思います。