Support preloading of applications modules in Mix (or at least mix test)

[josevalim]

We have recently added the --slowest N feature to mix test but the first tests to run will always be the slowest because they also need to load the code they are invoking.

One possible solution for this problem is to preload the modules in the loaded applications. We can make it either generic (a flag on app.start) or specific to mix test --slowest N.

[sorentwo]

Mind if I help with this one? I helped implement --slowest support initially, so I'm invested in it.

Regarding the flags, I think we should start by automatically enabling it when --slowest is used. You shouldn't need to remember to use both --preload (or whatever it would be called) and --slowest flags to get accurate timings.

[josevalim]

@sorentwo definitely. Let's wait for a couple more people to jump in though because I am not yet sure if that's the best way to go.

And yes, preload should be done automatically if you are calling --slowest.

This may also be helpful to @benwilson512.

[bschaeffer]

Just weighing in, but if this is needed solely to accurately measure the first test when using --slowest, I'm 👍 on doing it automatically when using the --slowest flag.

[benwilson512]

In my view it's needed more generally, in that tests using assert_receive can fail if they are the first test to call out to a large library. As a simple example:

test "should be fine" do
  test_pid = self()
  spawn(fn -> SomeBigLibrary.fun(); send(test_pid, :done) end)
  assert_receive(:done)
end

Even if fun is normally very fast, if it involves loading a ton of modules within SomeBigLibrary the test may fail if it's one of the first to run, which can be incredibly hard to debug.

[sorentwo]

@benwilson512 That's a sensible use case for having a separate flag to enable preloading. Are you proposing that the default behavior when running mix test is to preload, or that it should be exposed as a flag?

[josevalim]

@sorentwo so let's go with an implementation that adds --preload to app.start and --slowest N enables this flag by default. :) But generally speaking, --preload won't be enabled.

[fishcakez]

in that tests using assert_receive can fail if they are the first test to call out to a large library.

It's not the only time this issue may occur, and becomes more likely on slower CI systems (not naming any names travis). It's possible to control the default assert receive timeout globally, rather than per call using assert_receive_timeout in the ExUnit configuration, for example:

ExUnit.start(assert_receive_timeout: 400)

This approach is recommended generally for assert receive because it always you to scale the timeout with the performance of the system being used and how that affects scheduling.

I'm not saying this resolves this issue, rather a general point on how to resolve the comment in general (when it may or may not be because of module loading).

[josevalim]

@sorentwo if you haven't started yet, please hold. @fishcakez brought some new concerns we need to discuss and it is likely that --slowest will need its own version of preloading anyway.

[sorentwo]

@josevalim Thanks for the heads up, I'll try and follow the discussion in #elixir-lang.

[josevalim]

@fishcakez I wonder if we can use code:ensure_modules_loaded/1 for improving the performance of loading those apps.

[fishcakez]

@josevalim that function got added in OTP 19 (there are a couple more too then) but we could try it out and fallback on OTP 18.

[josevalim]

@fishcakez master is OTP19+. :)

[fishcakez]

😱 I thought we were going to support last 3 OTP releases.

[josevalim]

@fishcakez the plan is to support at least 2 releases in common with the previous version, that's why we went from 18&19&20 to 19&20. I would like to eventually set on the last 3 OTP releases but we are still getting large implements between releases. For example, the gap between 19 and 20 is a big one (unicode atoms, debug info, new guards, string unicode, etc).