N+1問題を解決しても尚、とにかく描画速度が遅いページのクエリチューニングをした話

N+1問題を解決しても尚、とにかくページの描画速度が遅かった

Rails gemのbulletを入れていたので、N+1を検出できるようにはしていたものの、キャッシするデータが多すぎたのか、速度が遅いページがありました。

結論から言うと、ビジネス用件が複雑であった為、クエリキャッシュさせるのではなく、ハッシュデータを作る方法を採用しました。

飛躍的にページ速度改善につながったので、残しておきたいと思います。

改善前
Completed 200 OK in 5301ms (Views: 1691.3ms | ActiveRecord: 3137.2ms)

改善後
Completed 200 OK in 1361ms (Views: 1217.0ms | ActiveRecord: 16.7ms)

既存実装（テーブル関係と、要件）

（命名情報、詳細な実装内容等については実際の物ととは異なり、あくまで例です。）

class Qr < ActiveRecord::Base
  belongs_to :item
  belongs_to :shape
  belongs_to :size
  belongs_to :staff
end

class Item < ActiveRecord::Base
  has_many :shapes
  has_many :sizes
  has_many :qrs
end

class Staff < ActiveRecord::Base
  belongs_to :warehouse
  has_many :qr
end

class Warehouse < ActiveRecord::Base
  belongs_to :warehouse
  has_many :staffs
  has_many :qs
end

class Shape < ActiveRecord::Base
  has_many :items
end

class Size < ActiveRecord::Base
  has_many :items
end

class QrsController < ApplicationController
  def index
    @item = Item.find(params[:item_id])
    @all_warehouses = Warehouse.joins({staffs: :qrs})
                      .where(qrs: {item_id: @item.id})
                      .eager_load({qrs: :shape}, {qrs: :staff}, {qrs: :size})
                      .preload(qrs: {staff: :warehouse})
    @q = @all_warehouses.ransack(params[:q])
    @warehouses = @q&.result&.uniq&.page(params[:page])&.per(10)
    @staffs = @all_warehouses.uniq.flat_map{|warehouse| warehouse.qrs.map(&:staff)}
  end
end

# 今回は可読性を考慮して、これ以降の記述でransack処理を省いてるものの、
# ransackがあったがゆえに奇妙なレコードの取り方が行われていた為controllerでは残しています.
# また上記理由により、view側で使っていない変数を作っています。

# app/views/qrs/index.html.erb

<% @warehouses.each do |warehouse| %>
  <div id="hoge<%= warehouse.id %>" role="tab" class="panel-hoge">
    <h4 class="panel-title">
      <a data-toggle="collapse" data-parent="#accordion" href="#collapse<%= warehouse.id %>"<%= warehouse.name %></a>
    </h4>
  </div>
  <div id="collapse<%= warehouse.id %>" role="fuga" aria-labelledby="hoge<%= warehouse.id %>" class="panel-collapse collapse">
    <div class="table-responsive">
      <table class="table table-hover">
        <tbody>
          <% warehouse.qrs.each do |qr| %>
            <tr>
              <td><%= qr.staff.name %></td>
              <% if qr.shape.present? %><%= qr.shape.name %><% end %>
              <% if qr.size.present? %><%= qr.size.name %><% end %>
            </tr>
          <% end %>
        </tbody>
      </table>
    </div>
  </div>
</div>

# controller側にも追記した通り、本題から逸れる部分を含めていないので、ちょっとありえない形になっています。

見ての通り、明らかにcontrollerの処理が悪かった。
①余計なeager_loadを利用している（全てpreloaddで良かった）
②レコードの取り方が変則的（わざわざ遠いレコードから取り始める必要性がない）
③viewsで二重にeachを回すために、クエリキャッシュが多くなりすぎている（load時間自体が結構長い）

controllerをリファクタリング

class QrsController < ApplicationController
  def index
    @item = Item.find(params[:item_id])
    default_qrs = Qr.preload({staff: :warehouse}).where(item_id: @item.id)
    @q = default_qrs.ransack(params[:q])
    searched_qrs = @q.result.preload(:shape, :size).sort_by{|qr| qr.staff.warehouse.position}
    hash_warehouses = searched_qrs.group_by{|qr| qr.staff.warehouse}
    @qrs_per_warehouses = Kaminari.paginate_array(hash_warehouses.to_a).page(params[:page]).per(10)
  end
end

リファクタのポイントは以下。
①クエリキャッシュする際は全てpreloadで実施
②最短でレコードを取れるようにQrからActiverRecordをスタート
③余計なクエリキャッシュを除き、最低限の（クエリキャッシュ済）変数が出来た時点で、ハッシュ化してやり、そのままviews側に投下
　→views側で分解してやることで、紐付け関係が余計なクエリキャッシュで崩れることを防いだ。

views側をcontrollerに合わせて、arrayではなくhashに対応させる

# app/views/qrs/index.html.erb

<% @warehouses.to_h.each do |warehouse, qrs| %>
  <div id="hoge<%= warehouse.id %>" role="tab" class="panel-hoge">
    <h4 class="panel-title">
      <a data-toggle="collapse" data-parent="#accordion" href="#collapse<%= warehouse.id %>"<%= warehouse.name %></a>
    </h4>
  </div>
  <div id="collapse<%= warehouse.id %>" role="fuga" aria-labelledby="hoge<%= warehouse.id %>" class="panel-collapse collapse">
    <div class="table-responsive">
      <table class="table table-hover">
        <tbody>
          <% qrs.each do |qr| %>
            <tr>
              <td><%= qr.staff.name %></td>
              <% if qr.shape.present? %><%= qr.shape.name %><% end %>
              <% if qr.size.present? %><%= qr.size.name %><% end %>
            </tr>
          <% end %>
        </tbody>
      </table>
    </div>
  </div>

まとめ

ビジネスサイドの担当者から、ページの描画速度が遅いという問合せがあり、今回のリファクタリングに取り組むことになった経緯でした。
いくら効率的なクエリキャッシュを行っても、大幅な速度改善には至らず、悩んでいたところ、先輩エンジニアからハッシュ化せよとの助言をもらい、上記の実装を行いました。
目から鱗だったので、同じ悩みを持つエンジニアがいるのではと思い、今回は記事にしてみました。

ちなみに、とにかくページ速度自体は改善したので、これで終わりにしたのですが、なぜ既存のクエリキャッシュ効率化で速度改善に至らないのか、原因は調べきれておりません。
このハッシュ化させる方法が最適解だったのかは、自分でも未だ確信を持つには至っていない為、「いや、これはアンチパターンである」ということであればご指摘いただけるとありがたいですm(_ _)m