Performance on Translated Models

[gertfindel]

I have been using this extension on some projects and the results in terms of performance are very poor, due to n+1 queries and similar.

One option would be to replace Globalize and a second, maybe easier solution for the short term, would be forcing early loading of the translations for the current locale.

I would like to implement this, but accessing the locale (a business logic thing) from active record, or similar does not sound as a good practice. Opinions?

@huoxito wrote most of the current Globalize usage in this extension, so he might know better.

[huoxito]

Hey @gertfindel I can imagine this not performing very well sorry. The initial project that motivated the globalize work here didn't move on so I didn't have the pleasure to see this running in production myself. We've seen a lot of bug reports and feedback from community since then though.

I suggest you start by looking into globalize first before trying anything else. Globalize is a great project and I'd rather first try hard to improve it.

I would like to implement this, but accessing the locale (a business logic thing) from active record, or similar does not sound as a good practice.

I'm not sure what you mean there? Can you elaborate a bit more please? I think spree_i18n should only be used for stores that actually translate db records. For stores that only need a couple of locales files spree_i18n is definitely overkill.

[gertfindel]

Given I do need Globalize, it needs some query optimisation. Every time a translated model is needed, the app incurs in a N+1 query in the view.

We should just auto include(:translations_for_model, :specific_locale) in every query where these models are called. So, in order to avoid accessing the view or controllers, we should use some metaprogramming to include the proper translation for every AR select on this tables. I haven't tried this yet, but it seems doable.. the dirty stuff starts when deciding which translation to use... spanish or japanese, because the locale isn't available yet if we would do this in an initialiser, right? Here is where

I would appreciate your advice, @huoxito =)

[wuboy0307]

You can try override https://github.com/spree/spree_i18n/blob/master/app/models/spree_i18n/translatable.rb and add default_scope section on your own

module SpreeI18n
  module Translatable
    extend ActiveSupport::Concern
    included do
      accepts_nested_attributes_for :translations

      table_alias = "t_#{Time.now.to_i}"
      default_scope {
        joins("LEFT OUTER JOIN #{translations_table_name} as #{table_alias} on \
          #{table_alias}.#{translation_class.reflections[:globalized_model].options[:foreign_key]}=#{table_name}.#{primary_key}")
        .where("#{table_alias}.locale=?", I18n.locale)
      }
    end
  end
end

(just a example, I didn't handle i18n fallback and also no test)

However, it will affect all queries to your translated table since it's default scope.
IMHO, default_scope {includes(:translations)} is enough.

[huoxito]

hey @gertfindel sorry for the late response. I'm kind of focused in some other stuff right now so I don't really know what to say.You could start by trying what @wuboy0307 said above, even though I'm definitely not a fan of default_scope and would probably go some other direction just don't know which one right now.

will leave this open as I'm looking forward for performance patches to improve things around here.

[radar]

I wonder if it would be possible to use IdentityCache (as I do in spree/spree#4887) for Products, and then have the product+variant data along with translations cached within the cache backend, and retrieving those from the database.

Just a thought.

[gertfindel]

Exactly, it has to be a mix of default_scope and some cache

[huoxito]

default_scope is evil. There's a lot of places in Spree where you wont care about translations at all. I think it's worth it to think of some other approach

[wuboy0307]

I agree with huoxito. Even I suggest gertfindel to add default_scope base on his question. I personally add a "with_translations" scope and only append it when I need translation. It would be better you can decide to append it or not instead of "Evil default_scope". default_scope solution is for lazy people. :)

[JDutil]

This should be resolved by f38be5f