mobility icon indicating copy to clipboard operation
mobility copied to clipboard

WIP of initializing fallbacks on start

Open f1sherman opened this issue 5 years ago • 9 comments

As pointed out in #481, I18n fallbacks are lazy-initialized by default. This can be problematic in production environments, particularly with forking appservers, since:

  1. The first request pays the initialization cost
  2. Memory usage is duplicated across appserver processes rather than taking advantage of copy-on-write

This is a draft of a way to mitigate these problems. Readability isn't great since the i18n library doesn't seem to have an API to explicitly initialize fallbacks. Instead we're just looping through each locale and referencing it so that i18n will initialize it.

Considerations:

  1. We probably don't want this behavior in development, how should we account for that?
  2. Ideally this behavior would be configurable, what is the right place for that configuration to live? This could potentially address (1).
  3. Would this be considered a breaking change to have turned on by default?

f1sherman avatar Dec 23 '20 19:12 f1sherman

Sorry for the very late reply. The problem here is that fallbacks is not actually dependent on Rails, so you can't really assume there's a Rails configuration or mode to refer to.

Ideally this behavior would be configurable, what is the right place for that configuration to live?

This is a longstanting problem. One solution would be to define a custom plugin:

module Mobility
  module Plugins
    module PreloadFallbacks
      extend Mobility::Plugin

      requires :fallbacks, include: :before

      private

      def generate_fallbacks(fallbacks)
        super.tap do |fallbacks_instance|
          fallbacks.each_key do |locale|
            # Initialize fallbacks
            fallbacks_instance[locale]
          end
        end
      end
    end

    register_plugin(:preload_fallbacks, PreloadFallbacks)
  end
end

Then enable it with:

Mobility.configure do
  fallbacks ...

  preload_fallbacks
end

Other than this, to make this part of the default fallbacks plugin we'd need something like if defined? Rails which I really don't like... or a Rails-specific implementation.

We don't actually even have a Rails plugin, just an ActiveRecord one, so maybe this would be a place to create one. In that case we'd define a "Rails" plugin which would include Rails-specific patches for things like fallbacks. Same idea as above but just renamed Mobility::Plugins::Rails, which would I guess require ActiveRecord :thinking:

shioyama avatar Nov 05 '21 04:11 shioyama

Thanks @shioyama! Sorry for taking so long to get to this. Your plugin code does indeed seem to do the trick (though I had to add a register_plugin :preload_fallbacks, PreloadFallbacks at the end).

One great thing about your plugin is that it's not actually Rails specific at all. Or were you saying that it's really only useful in a Rails context?

I'm happy to contribute this back but I could really use a little guidance on how you'd like it done. Thank you!

f1sherman avatar Dec 07 '21 20:12 f1sherman

One great thing about your plugin is that it's not actually Rails specific at all.

Yes indeed :smile:

I'm happy to contribute this back but I could really use a little guidance on how you'd like it done. Thank you!

Thanks for offering! I think for now just having that code above is enough for folks to find this issue and if they need it just copy paste. It's just a dozen lines of code after all. If enough people want it it could be added to the gem maybe, or as an option to the fallbacks plugin itself.

shioyama avatar Dec 08 '21 09:12 shioyama

though I had to add a register_plugin :preload_fallbacks, PreloadFallbacks at the end)

Thanks! Updated my comment so it works in case anyone else uses it.

shioyama avatar Dec 08 '21 09:12 shioyama

Ok sounds good!

I think for now just having that code above is enough for folks to find this issue and if they need it just copy paste.

In lieu of adding this to the gem right now, would it be crazy for me to submit a PR to add a note to the README? I feel like it's really easy to not know you have an initialization performance issue in your app, and it can take a bit of work to track that performance issue down to this gem. It took me at least a few hours, anyway.

f1sherman avatar Dec 08 '21 11:12 f1sherman

Wel, I suppose we could make an option to fallbacks, something like preload_translations, defaulting to false, which if true would do what the plugin above does.

Then in a Rails app you could set it to Rails.env.production? so it only preloads in production.

WDYT?

Since it's not dependent on Rails I don't have an issue with adding it.

shioyama avatar Dec 11 '21 12:12 shioyama

@shioyama yes that would be amazing and I think it would save folks a lot of trouble!

f1sherman avatar Dec 11 '21 16:12 f1sherman

Ok, it's a bit tricky to pass a preload_fallbacks option to the fallbacks plugin. Currently the option is the fallbacks hash itself (or true/nil/false). It's possible to do but I need to think a bit because I'm not sure the pattern is one we'd want to repeat (it's currently only used in the Backend plugin which is a very special one).

shioyama avatar Dec 15 '21 13:12 shioyama

@shioyama ok sounds good! Let me know if there is anything I can do to help!

f1sherman avatar Dec 15 '21 14:12 f1sherman

I need to close this since I don't have time to implement new features. Anyone who wants to do this please feel free to give it a shot.

shioyama avatar Mar 20 '24 08:03 shioyama