From 9b5401fcc9624be9bd60331d59169267ae2f7bac Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Tue, 9 Apr 2019 11:06:44 +0200 Subject: [PATCH] depend on Zeitwerk 2.1.0 --- Gemfile.lock | 4 +- activesupport/CHANGELOG.md | 8 ++ activesupport/activesupport.gemspec | 2 +- .../dependencies/zeitwerk_integration.rb | 13 +--- .../lib/active_support/descendants_tracker.rb | 11 ++- .../application/zeitwerk_integration_test.rb | 78 +++++++++++++++---- 6 files changed, 87 insertions(+), 29 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index b1f9991adc..8e219f3f31 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -71,7 +71,7 @@ PATH i18n (>= 0.7, < 2) minitest (~> 5.1) tzinfo (~> 1.1) - zeitwerk (~> 2.0) + zeitwerk (~> 2.1) rails (6.0.0.beta3) actioncable (= 6.0.0.beta3) actionmailbox (= 6.0.0.beta3) @@ -526,7 +526,7 @@ GEM websocket-extensions (0.1.3) xpath (3.2.0) nokogiri (~> 1.8) - zeitwerk (2.0.0) + zeitwerk (2.1.0) PLATFORMS java diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index cf72673dbb..57e03b5e12 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,11 @@ +* The Zeitwerk compatibility interface for `ActiveSupport::Dependencies` no + longer implements `autoloaded_constants` or `autoloaded?` (undocumented, + anyway). Experience shows introspection does not have many use cases, and + troubleshooting is done by logging. With this design trade-off we are able + to use even less memory in all environments. + + *Xavier Noria* + * Depends on Zeitwerk 2, which stores less metadata if reloading is disabled and hence uses less memory when `config.cache_classes` is `true`, a standard setup in production. diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index e719fc6176..546cc1797a 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -34,5 +34,5 @@ Gem::Specification.new do |s| s.add_dependency "tzinfo", "~> 1.1" s.add_dependency "minitest", "~> 5.1" s.add_dependency "concurrent-ruby", "~> 1.0", ">= 1.0.2" - s.add_dependency "zeitwerk", "~> 2.0" + s.add_dependency "zeitwerk", "~> 2.1" end diff --git a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb index faf9edef27..b0f7a3f9cc 100644 --- a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb +++ b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb @@ -21,17 +21,8 @@ module ActiveSupport ActiveSupport::Inflector.safe_constantize(cpath) end - def autoloaded_constants - cpaths = [] - Rails.autoloaders.each do |autoloader| - cpaths.concat(autoloader.loaded_cpaths.to_a) - end - cpaths - end - - def autoloaded?(object) - cpath = object.is_a?(Module) ? object.name : object.to_s - Rails.autoloaders.any? { |autoloader| autoloader.loaded?(cpath) } + def to_unload?(cpath) + Rails.autoloaders.main.to_unload?(cpath) end def verbose=(verbose) diff --git a/activesupport/lib/active_support/descendants_tracker.rb b/activesupport/lib/active_support/descendants_tracker.rb index 2dca990712..fe0c6991aa 100644 --- a/activesupport/lib/active_support/descendants_tracker.rb +++ b/activesupport/lib/active_support/descendants_tracker.rb @@ -22,11 +22,18 @@ module ActiveSupport def clear if defined? ActiveSupport::Dependencies + # to_unload? is only defined in Zeitwerk mode. + to_unload = if Dependencies.respond_to?(:to_unload?) + ->(klass) { Dependencies.to_unload?(klass.name) } + else + ->(klass) { Dependencies.autoloaded?(klass) } + end + @@direct_descendants.each do |klass, descendants| - if ActiveSupport::Dependencies.autoloaded?(klass) + if to_unload[klass] @@direct_descendants.delete(klass) else - descendants.reject! { |v| ActiveSupport::Dependencies.autoloaded?(v) } + descendants.reject! { |v| to_unload[v] } end end else diff --git a/railties/test/application/zeitwerk_integration_test.rb b/railties/test/application/zeitwerk_integration_test.rb index a9da060347..b461d82450 100644 --- a/railties/test/application/zeitwerk_integration_test.rb +++ b/railties/test/application/zeitwerk_integration_test.rb @@ -98,24 +98,35 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase assert_nil deps.safe_constantize("Admin") end - test "autoloaded_constants returns autoloaded constant paths" do - app_file "app/models/admin/user.rb", "class Admin::User; end" - app_file "app/models/post.rb", "class Post; end" - boot - - assert Admin::User - assert_equal ["Admin", "Admin::User"], deps.autoloaded_constants - end - - test "autoloaded? says if a constant has been autoloaded" do + test "to_unload? says if a constant would be unloaded (main)" do app_file "app/models/user.rb", "class User; end" app_file "app/models/post.rb", "class Post; end" boot assert Post - assert deps.autoloaded?("Post") - assert deps.autoloaded?(Post) - assert_not deps.autoloaded?("User") + assert deps.to_unload?("Post") + assert_not deps.to_unload?("User") + end + + test "to_unload? says if a constant would be unloaded (once)" do + add_to_config 'config.autoload_once_paths << "#{Rails.root}/extras"' + app_file "extras/foo.rb", "class Foo; end" + app_file "extras/bar.rb", "class Bar; end" + boot + + assert Foo + assert_not deps.to_unload?("Foo") + assert_not deps.to_unload?("Bar") + end + + test "to_unload? says if a constant would be unloaded (reloading disabled)" do + app_file "app/models/user.rb", "class User; end" + app_file "app/models/post.rb", "class Post; end" + boot("production") + + assert Post + assert_not deps.to_unload?("Post") + assert_not deps.to_unload?("User") end test "eager loading loads the application code" do @@ -315,4 +326,45 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase assert_nil autoloader.logger end end + + # This is here because to guarantee classic mode works as always, Zeitwerk + # integration does not touch anything in classic. The descendants tracker is a + # very small one-liner exception. We leave its main test suite untouched, and + # add some minimal safety net here. + # + # When time passes, things are going to be reorganized (famous last words). + test "descendants tracker" do + class ::ZeitwerkDTIntegrationTestRoot + extend ActiveSupport::DescendantsTracker + end + class ::ZeitwerkDTIntegrationTestChild < ::ZeitwerkDTIntegrationTestRoot; end + class ::ZeitwerkDTIntegrationTestGrandchild < ::ZeitwerkDTIntegrationTestChild; end + + begin + app_file "app/models/user.rb", "class User < ZeitwerkDTIntegrationTestRoot; end" + app_file "app/models/post.rb", "class Post < ZeitwerkDTIntegrationTestRoot; end" + app_file "app/models/tutorial.rb", "class Tutorial < Post; end" + boot + + assert User + assert Tutorial + + direct_descendants = [ZeitwerkDTIntegrationTestChild, User, Post].to_set + assert_equal direct_descendants, ZeitwerkDTIntegrationTestRoot.direct_descendants.to_set + + descendants = direct_descendants.merge([ZeitwerkDTIntegrationTestGrandchild, Tutorial]) + assert_equal descendants, ZeitwerkDTIntegrationTestRoot.descendants.to_set + + ActiveSupport::DescendantsTracker.clear + + direct_descendants = [ZeitwerkDTIntegrationTestChild].to_set + assert_equal direct_descendants, ZeitwerkDTIntegrationTestRoot.direct_descendants.to_set + + descendants = direct_descendants.merge([ZeitwerkDTIntegrationTestGrandchild]) + assert_equal descendants, ZeitwerkDTIntegrationTestRoot.descendants.to_set + ensure + Object.send(:remove_const, :ZeitwerkDTIntegrationTestRoot) + Object.send(:remove_const, :ZeitwerkDTIntegrationTestChild) + end + end end