Autoload schema constants

[gmcgibbon]

👋 I noticed an app I was working on was taking some time to load dry-schema:

This PR adds autoloading to schema constants so that on load, less files are required and apps using this gem can boot faster. Using a simple benchmark

require "benchmark"

x = Benchmark.realtime do
  require "dry/schema"
end

puts "Loaded in #{x * 1000}ms"

We can observe the time it takes to load the gem. Autoload changes this number from ~90ms to 2ms.

The downside to this, is we loose the ability to eager-load autoloaded constants into memory. Rails accounts for this by using Zeitwerk to load autoloaded constants early, which helps in environments like production where we want to do as much loading as possible upfront. Since dry-rb doesn't use Zeitwerk, I'm not sure how we can address this. Any ideas?

[solnic]

Thanks for opening this PR. We're happy to improve lib loading time but this should be done via Zeitwerk and it should be unified across all dry-rb gems. We could probably have some common API for it in dry-core and re-use it in many gems. I'm gonna close this because it may cause all kinds of constant-related issues and unexpected problems.

I should mention that lib load time has been on my radar for some time now. We're introducing Zeitwerk integration with Hanami 2.0 and rom 6.0 already and I suspect dry-rb will follow soon.

[rafaelfranca]

👍🏽 for the direction, but could you explain what those "constant-related issues and unexpected problems." are that would be caused by those changes but not by applying zeitwerk? This is pretty much what zeitwerk would do for you if you started using it on this project right now.

[solnic]

@rafaelfranca changing how lib's code is being loaded comes with a risk that it'll break something, like for example one of the tests that started to fail in this PR https://github.com/dry-rb/dry-schema/runs/6382845899?check_suite_focus=true#step:5:14

If we had a coordinated, planned effort to introduce autoloading via Zeitwerk across dry-rb gems, we'd reduce this risk. That's why we can't just accept adding autoloading in one of the gems and hope that it's not gonna break anything.

Maybe we'd manage to introduce autoloading/zeitwerk in dry-rb gems for hanami 2.0 release. WDYT @timriley?

[timriley]

Wow, that's a huge load time improvement you've shared, @gmcgibbon — thanks so much for looking into this!

To add to @solnic's comments — and especially after seeing the performance improvements from @gmcgibbon's work — I'd be very interested in us looking at adding autoloading across dry-rb gems in general, and if we could do it in a single coordinated effort, it'd probably help us do it most effectively.

However, I'd also be open to having one of our gems be a frontrunner in this effort, and giving us an early learning opportunity.

To that end, @rafaelfranca and @gmcgibbon — given your work in moving to autoloading here, would you have any idea why that spec @solnic pointed out is failing? If we can have our specs green here, it'd certainly make it more feasible for us to consider a switch here first.

[timriley]

And yes, I should note that I'd also like for our gems to move to Zeitwerk, as early as is practical. Right now my personal focus is on finishing Hanami 2.0 development, but I agree with @solnic that it could be a good opportunity for us to move all dry-rb gems to Zeitwerk right before the Hanami 2.0 final release.

[solnic]

However, I'd also be open to having one of our gems be a frontrunner in this effort, and giving us an early learning opportunity.

Yes but maybe something less popular than schema/validation/types/configurable 😅

[gmcgibbon]

given your work in moving to autoloading here, would you have any idea why that spec @solnic pointed out is failing? If we can have our specs green here, it'd certainly make it more feasible for us to consider a switch here first.

It is failing because I only added autoload definitions for the requires that were present. These requires seem to require other constants defined on Dry::Schema (like Result). The fix would be to add autoload definitions for all constants under Dry::Schema (eg. autoload(:Result, "dry/schema/result")).

With the current behaviour, you would have to reference Dry::Schema::DSL first before Dry::Schema::Result was available. I would fix this, but I assume Zeitwerk is what you'd rather go with. TL;DR is you just need to add more autoload definitions to the library to ensure constants are loadable in any order, but Zeitwerk will manage this for you.

[rafaelfranca]

@gmcgibbon do you have some time to try starting with a less popular gem? @solnic which one do you recommend? dry-monads?

[gmcgibbon]

Happy to get something started. Let me know what repo I can hack on! 😄

[solnic]

I reckon dry-monitor would be a good candidate. It's a gem where in many cases you'd cherry-pick its components so it's expected that the gem wouldn't be loaded as-a-whole. We use it in Hanami 2.0 so we could easily verify if autoloading isn't breaking anything. If we used Zeitwerk with dry-monitor, it'd be also good to see if it doesn't conflict with Hanami's own Zeitwerk setup.