その他
    ホーム技術発信DoRubyメモ化トラップ

    メモ化トラップ

    この記事はアピリッツの技術ブログ「DoRuby」から移行した記事です。情報が古い可能性がありますのでご注意ください。

    🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢🍢

     みなさーん! 快適Railsライフ送ってますかー!!?? 私は快適ではありません……。先日も訳の分からないエラーに引っかかって四苦八苦しました……。
     メモ化という事柄に関してです。

    メモ化とは

     メモ化って便利だよなー。マスタを一々データベースから参照せずに取ってこれるから高速に取ってこれるんだぜ! 
     使いやすいgemもあるし。

    /app/models/oden_master.rb
    
    class OdenMaster < ActiveRecord::Base
      extend Memoist
    
      class << self
        def oden_data_memo(oden_code)
          find_by(code: oden_code)
        end
        memoize :oden_data_memo
      end
    end
    

    こんな感じにしちゃえば、同じ名前のおでんマスタは1度参照して来てしまえばキャッシュされるからもうSQLを呼ばずに取って来れる!

    発端と実装

    某日:
    くらいあんと「おでん情報にユーザーの好みくっつけて送ってくれない?」
    ぼく「わかりました!」
    ぼく「うーん、どうしようかな。インスタンスに属性付けちゃえば楽だな」

    /app/models/oden_master.rb
    
    class OdenMaster < ActiveRecord::Base
      extend Memoist
    
      attr_accessor :preference_data
    
      class << self
        def oden_data_memo(oden_code)
          find_by(code: oden_code)
        end
        memoize :oden_data_memo
      end
    
      def set_user_preference_data(user_data)
        self.preference_data = user_data
      end
    end
    

    で、コントローラはこんな感じにして、後はviewをいい感じに作れば完了!(結構簡略化してます)

    app/controllers/oden_controller.rb
    
      def current_oden
        user = User.find_by(code: params[:user_code])
        raise InvalidUser unless user
        oden_code = params[:oden_code]
        @oden_master = OdenMaster.oden_data_memo(oden_code)
        raise InvalidOden unless oden_master
    
        user_oden_data = user.oden_preferences.find_by(oden_code: oden_code)
        @oden_master.set_user_preference_data(user_oden_data) if user_oden_data
      end
    end
    

    テストも簡単に作って、通ったし問題無いな! メモ化して一度呼んできたマスタはもう取ってきてあるし、大丈夫!
    コミットしてマージしてプッシュして開発環境にデプロイして(要するにクライアントが開発環境で試せる環境を作る)。
    くらいあんと「大丈夫そうですね」
    ぼく「お仕事完了!」
    ……しかしこのコードには致命的なミスがあるのがまだ、この時点では分からなかった。

    発覚

    数日後
    てすたー「なんか別のユーザーのデータ入ってきてるよ」
    ぼく「えっ」
    ぼく「なんでだろう……。コンソール上で実際に動かしても何の問題も無いし……。うーん、うーん……」
    てすたー「まだ、このおでんマスタに対するおでんデータ作ってないのに、入っちゃってるんだよね。それに私、こんなに辛子を大根に付けないんですよ」
    ぼく「そうですか……。何でだろう。因みに私はたっぷり付けてゲホゲホ言いながら食べるのが好きです。」
    てすたー「変態かよ。まあ、おでんデータ入れればちゃんと自分の入ってくるんだけど」
    ぼく「むせる感覚が最高なんですよ。なんでこんな短いコードでそんな事が起きるんだろうなぁ……。調べてみます」

     多分悩んだのは30分~1時間位だった。
     問題は、memoizeをちゃんと読み込んでいなかった事にありました。
     以下が、memoizeのソースになります。

      def memoize(*method_names)
        if method_names.last.is_a?(Hash)
          identifier = method_names.pop[:identifier]
        end
    
        method_names.each do |method_name|
          unmemoized_method = Memoist.unmemoized_method_for(method_name, identifier)
          memoized_ivar = Memoist.memoized_ivar_for(method_name, identifier)
    
          Memoist.memoist_eval(self) do
            include InstanceMethods
    
            if method_defined?(unmemoized_method)
              warn "Already memoized #{method_name}"
              return
            end
            alias_method unmemoized_method, method_name
    
            if instance_method(method_name).arity == 0
    
              # define a method like this;
    
              # def mime_type(reload=true)
              #   skip_cache = reload || !instance_variable_defined?("@_memoized_mime_type")
              #   set_cache = skip_cache && !frozen?
              #
              #   if skip_cache
              #     value = _unmemoized_mime_type
              #   else
              #     value = @_memoized_mime_type
              #   end
              #
              #   if set_cache
              #     @_memoized_mime_type = value
              #   end
              #
              #   value
              # end
    
              module_eval <<-EOS, __FILE__, __LINE__ + 1
                def #{method_name}(reload = false)
                  skip_cache = reload || !instance_variable_defined?("#{memoized_ivar}")
                  set_cache = skip_cache && !frozen?
    
                  if skip_cache
                    value = #{unmemoized_method}
                  else
                    value = #{memoized_ivar}
                  end
    
                  if set_cache
                    #{memoized_ivar} = value
                  end
    
                  value
                end
              EOS
            else
    
              # define a method like this;
    
              # def mime_type(*args)
              #   reload = Memoist.extract_reload!(method(:_unmemoized_mime_type), args)
              #
              #   skip_cache = reload || !memoized_with_args?(:mime_type, args)
              #   set_cache = skip_cache && !frozen
              #
              #   if skip_cache
              #     value = _unmemoized_mime_type(*args)
              #   else
              #     value = @_memoized_mime_type[args]
              #   end
              #
              #   if set_cache
              #     @_memoized_mime_type ||= {}
              #     @_memoized_mime_type[args] = value
              #   end
              #
              #   value
              # end
    
              module_eval <<-EOS, __FILE__, __LINE__ + 1
                def #{method_name}(*args)
                  reload = Memoist.extract_reload!(method(#{unmemoized_method.inspect}), args)
    
                  skip_cache = reload || !(instance_variable_defined?(#{memoized_ivar.inspect}) && #{memoized_ivar} && #{memoized_ivar}.has_key?(args))
                  set_cache = skip_cache && !frozen?
    
                  if skip_cache
                    value = #{unmemoized_method}(*args)
                  else
                    value = #{memoized_ivar}[args]
                  end
    
                  if set_cache
                    #{memoized_ivar} ||= {}
                    #{memoized_ivar}[args] = value
                  end
    
                  value
                end
              EOS
            end
    
            if private_method_defined?(unmemoized_method)
              private method_name
            elsif protected_method_defined?(unmemoized_method)
              protected method_name
            end
          end
        end
        # return a chainable method_name symbol if we can
        method_names.length == 1 ? method_names.first : method_names
      end
    
    

     まあ、コードの解説は抜きにして、端的に言えば、メモ化で参照されるデータは単一のものである、という事です。
     その単一のものに対して破壊的変更をしてしまえば、そのメモ化で参照されるデータにも勿論影響が及びます。

     上記のコードだと、

    ユーザーA: おでんコードXに対するユーザーデータを持つ
    ユーザーB: おでんコードXに対するユーザーデータは持たない
    

     と言う条件の下で、

    1. ユーザーAがおでんコードXに対する情報を取得
    メモ化されたおでんコードXのマスタの属性にデータが入る。
    
    2. ユーザーBがおでんコードXに対する情報を取得
    メモ化されたおでんコードXのマスタの属性はユーザーAのものが入ったままデータが送られてしまう。
    

     と言う事が起きてしまう訳です。カラムに変更を掛けてしまっても同じ事が起こります。

    解決

     まあ、策としては色々あります。
    ・メモ化して取ってきてくる時に属性を初期化する
    ・データが無くてもnilなどを明示的に入れる
    ・属性を使わない。メモ化したデータを弄る事をそもそもしないように心掛ける
     等々。
     自分は一番上の、メモ化して取ってきてくる時に属性を初期化する、を選びました。下のような感じにメソッドを作ったりしてそれを取ってくるようにして。

    def attr_initialized_oden_data_memo(oden_code)
      oden_data = oden_data_memo(oden_code)
      oden_data.preference_data = nil
      oden_data
    end
    

     理由としては、まあ実装上の設計の問題みたいな感じですが。
     そこ辺りは設計と相談にもなってきます。

     それではより良いおでんライフを。