Lazy instantiation and memoization with container auto-registration

Support dry-web
1

I’ve got a pretty simple setup like this:

class App < Dry::Component::Container
  ROOT = Pathname.new(__FILE__).dirname.dirname

  load_paths! 'app'
  configure do |config|
    config.root = ROOT
    config.auto_register = ['app/controllers', 'app/queries', 'app/views']
  end
end

It’s working fine, but I noticed that App['views.sign_in'] is instantiating a new object every time it’s called. I assumed that it would only instantiate one object, and memoize it. The objects I’m working with are immutable and some are expensive to instantiate (e.g. template loading and precompilation), so I really only want to instantiate one of them.

I know I can do App.register('views.sign_in', Views::SignIn.new), but is there a way do the same thing through auto-registration? It would be nice if it was lazy, so I can run a single test without booting up everything.

1 Like
2

I’ve been aware of this since day 1. There’s a PR which adds support for singletons. I’m not sure yet if we want to support OOTB memoization though. We’ve had some brief discussions about it and decided not to memoize as this can be handled at a class level and you can control it. We have support for custom loaders so maybe it’d be worth to at least experiment with a loader that can be configured to memoize and see which strategy is better.

3

I had a look through that PR, and it’s a bit different to what I was expecting. I dug into the code a bit, and came up with this, which gives me the behaviour that I’m after:

class MemoizingResolver < Dry::Container::Resolver
  def initialize(*)
    super
    @cache = {}
  end

  def call(container, key)
    @cache.fetch(key) do
      @cache[key] = super
    end
  end
end

class App < Dry::Component::Container
  configure do |config|
    config.resolver = MemoizingResolver.new
  end
end

This memoizes everything, so it’s not ideal in terms of flexibility, but it’s good enough for my purposes at the moment. To enable/disable it on a per-key basis, I’d need to get into Dry::Container::Item, probably changing the behaviour of #call based on a new key in the options hash.

4

That works too, although using a custom loader should be simpler. And btw - I recommend using Concurrent::Map#fetch_or_store in such cases.

1 Like
5

A (thread)safer version is

class MemoizingResolver < Dry::Container::Resolver
  def initialize
    @_mutex = Mutex.new
    @_key_mutexes = Hash.new { |h, k| h[k] = Mutex.new }
    @cache = {}
  end

  def call(container, k)
    key = k.to_s

    if @cache.key?(key)
      @cache[key]
    else
      synchronize(key) do
        if @cache.key?(key)
          @cache[key]
        else
          @cache[key] = super
        end
      end
    end
  end

  def synchronize(key)
    mutex(key).synchronize { yield }
  end

  private

  def mutex(key)
    @_mutex.synchronize { @_key_mutexes[key] }
  end
end

We need @_key_mutexes here and can’t use a single mutex for everything because it would end up with a deadlock.

@solnic fetch_or_store does not guarantee your block will be evaluated once and only once, this can lead to some unexpected consequences in this case.

6

Hello! I was hoping to revisit this conversation.

I would like to know if the team would be amenable to accepting a PR to change the AutoRegistrar or the Container to accept a configuration value to memoize resolved objects. Any objections?

7

I’m sure we’d be opening to exploring this, yes :slight_smile:

8

What is the best course of action then? Should I be providing code snippets in this forum or start with a pull request and take the conversation from there?

9

Probably should have replied in a thread. What is the best course of action then? Should I be providing code snippets in this forum or start with a pull request and take the conversation from there?

10

Happy to look at this in a PR, @arielvalentin :slight_smile:

11

@timriley I did start working on this but I am not entirely sure the best way to move forward.

I wanted to set a default value for registry_options e.g.

setting :registry_options, {}

And then override them in auto_load like so:

auto_register!('lib') do |config|
   config.registry_options[:memoize] = false
end

And then change AutoRegistrar to use the registry_options when it loads components:

container.require_component(component) do
    register(component.identifier, config.registry_options) { registration_config.instance.(component) }
end

The challenge with this solution is that Dry::AutoRegistrar::Configuration does not use dry-configurable. Would there be an objection to changing this class to use dry-configurable instead of its own implementation of the setting DSL?

1 Like
12

Getting Dry::AutoRegistrar::Configuration into line with dry-configurable strikes me as the right move on the march toward 1.0 for sure; it’s also in-pattern with already-1.0’d siblings.