Replace Code.ensure_loaded/1 with faster counterpart

[timbuchwaldt]

Our benchmarks showed that Code.ensure_loaded/1 becomes the bottleneck of our system when validating data with Vex.

I built a cached version of Code.ensure_loaded/1 that utilises FastGlobal to store a list of modules loaded in such a way.

This change brought a ~10x performance improvement on a real world application.

[benwilson512]

It's been a very long time since I've looked at Vex code but yeah there definitely shouldn't be any Code.ensure_loaded calls at runtime. A faster / cached version is nice, but I think it makes more sense to try to design this such that the call doesn't happen at runtime at all.

[timbuchwaldt]

Hey Ben. True that, not a beautiful solution, especially since in a normal production system Code.ensure_loaded/1 is not needed, as all code is loaded anyway.

We contemplated making Vex an application that is actually startable instead of a plain library, then ensuring loading all validators during startup - how does that sound to you?

[benwilson512]

What are the validators? Why might they not be loaded?

[benwilson512]

The normal use for Code.ensure_loaded? is something like

if Code.ensure_loaded?(Ecto) do
  def list_foos() do
    # if ecto is also around, make this function do stuff
  end
else
  def list_foos, do: raise "only available if ecto is also a dependency"
end

This is a common pattern if you have library A that has an optional dependency on library B. For people who have B also installed, they get certain functions in A. All of this happens at compile time though, so no runtime loading is necessary either at application start or any other time.

[timbuchwaldt]

The problem is the usage of function_exported?/3. It returns false if the module isn't currently loaded, which can happen in dev/test if it hasn't been called before.

Thinking about it the implementation kind of looks like a Protocol in disguise, but have to look a bit deeper.

[benwilson512]

Yeah I was getting the same "this looks like protocols" vibe. The original commits for the ensure loaded lines come from when Elixir would have been ~v0.10 so things have come a long way since then hahaha.

[timbuchwaldt]

Damn - and this is how a quick fix turns into a quest of refactoring :D

[benwilson512]

In looking at this more closely, the ensure loaded stuff may be completely unnecessary. I think it hails from a time when protocol implementation loading worked differently.

Can you try a branch where you just simply remove the Code.ensure_loaded calls? Do you run into any issues?

[uberbrodt]

I spent some time looking at this, and the Code.ensure_loaded/1 calls are still necessary when the VM isn't running in embedded mode. I'm not sure there's a way to fix it that preserves the existing API. Turning it into an OTP application wouldn't really help us, because Vex constructs module names at runtime based on the name used to call a validator.

I think that requiring users to load the validators in their Application callback is the least "tricky" way of removing the calls to Code.ensure_loaded/1. Something like Vex.sources/1 or Vex.set_validators/1, where you pass in a list of validator modules.

[jfornoff]

You can probably do it very similarly to how Plug.Router works, but this would require substantial effort.

…

On Mon, Jun 18, 2018, 14:53 Chris Brodt ***@***.***> wrote: I spent some time looking at this, and the Code.ensure_loaded/1 calls are still necessary when the VM isn't running in embedded mode. I'm not sure there's a way to fix it that preserves the existing API. Turning it into an OTP application wouldn't really help us, because Vex constructs module names at runtime based on the name used to call a validator. I think that requiring users to load the validators in their Application callback is the least "tricky" way of removing the calls to Code.ensure_loaded/1. Something like Vex.sources/1 or Vex.set_validators/1, where you pass in a list of validator modules. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#52 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQjioGe8t5a4FE1sS5-TNQvzKBEOVLoks5t96KvgaJpZM4SMeyV> .

[uberbrodt]

Yes, I thought of that as well. Might be a much cleaner way of doing it, especially if we generated some of the necessary code for the user. I don't believe there would be any process bottlenecks with reworking it as an OTP application in this fashion.

…

On Mon, Jun 18, 2018 at 05:58:44AM -0700, Jan Fornoff wrote: You can probably do it very similarly to how Plug.Router works, but this would require substantial effort. On Mon, Jun 18, 2018, 14:53 Chris Brodt ***@***.***> wrote: > I spent some time looking at this, and the Code.ensure_loaded/1 calls are > still necessary when the VM isn't running in embedded mode. I'm not sure > there's a way to fix it that preserves the existing API. Turning it into an > OTP application wouldn't really help us, because Vex constructs module > names at runtime based on the name used to call a validator. > > I think that requiring users to load the validators in their Application > callback is the least "tricky" way of removing the calls to > Code.ensure_loaded/1. Something like Vex.sources/1 or Vex.set_validators/1, > where you pass in a list of validator modules. > > — > You are receiving this because you are subscribed to this thread. > > Reply to this email directly, view it on GitHub > <#52 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/ABQjioGe8t5a4FE1sS5-TNQvzKBEOVLoks5t96KvgaJpZM4SMeyV> > . > -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #52 (comment)

[jfornoff]

I'm not entirely sure if a rewrite towards an OTP application would be necessary with the Plug.Router approach. You shouldn't have to load code at runtime (I don't think Plug does that).

…

On Mon, Jun 18, 2018, 15:40 Chris Brodt ***@***.***> wrote: Yes, I thought of that as well. Might be a much cleaner way of doing it, especially if we generated some of the necessary code for the user. I don't believe there would be any process bottlenecks with reworking it as an OTP application in this fashion. On Mon, Jun 18, 2018 at 05:58:44AM -0700, Jan Fornoff wrote: > You can probably do it very similarly to how Plug.Router works, but this > would require substantial effort. > > On Mon, Jun 18, 2018, 14:53 Chris Brodt ***@***.***> wrote: > > > I spent some time looking at this, and the Code.ensure_loaded/1 calls are > > still necessary when the VM isn't running in embedded mode. I'm not sure > > there's a way to fix it that preserves the existing API. Turning it into an > > OTP application wouldn't really help us, because Vex constructs module > > names at runtime based on the name used to call a validator. > > > > I think that requiring users to load the validators in their Application > > callback is the least "tricky" way of removing the calls to > > Code.ensure_loaded/1. Something like Vex.sources/1 or Vex.set_validators/1, > > where you pass in a list of validator modules. > > > > — > > You are receiving this because you are subscribed to this thread. > > > > Reply to this email directly, view it on GitHub > > <#52 (comment)>, or mute > > the thread > > < https://github.com/notifications/unsubscribe-auth/ABQjioGe8t5a4FE1sS5-TNQvzKBEOVLoks5t96KvgaJpZM4SMeyV > > > . > > > > > -- > You are receiving this because you commented. > Reply to this email directly or view it on GitHub: > #52 (comment) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#52 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQjimiEPOhYAYeFqH5lsQZsSM70RCTYks5t963agaJpZM4SMeyV> .